Summary:
Ref T10784. On `secure`, logged-out users currently can't browse repositories when cluster/service mode is enabled because they aren't permitted to make intracluster requests.
We don't allow totally public external requests (they're hard to rate limit and users might write bots that polled `feed.query` or whatever which we'd have no way to easily disable) but it's fine to allow intracluster public requests.
Test Plan: Browsed a clustered repository while logged out locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10784
Differential Revision: https://secure.phabricator.com/D15695
Summary: Fixes T10772, not sure why this fails, but reverting the code back to old dialog call works.
Test Plan:
- Try to add a new credential when importing a repository.
- Also created a new credential normally, via Passphrase.
- Also edited a credential.
Reviewers: chad
Reviewed By: chad
Subscribers: Korvin
Maniphest Tasks: T10772
Differential Revision: https://secure.phabricator.com/D15691
Summary:
Ref T10751. We currently have a placeholder Almanac document, and a fairly-bad-advice section in Daemons.
Pull these into the modern cluster documentation.
Test Plan: 17 phabricator PHDs
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15689
Summary: Fixes T10789. If we aren't configured with a device, we never grabbed a lock in the first place, and should not expect one to be held.
Test Plan: Pushed non-cluster-configured Git SSH repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10789
Differential Revision: https://secure.phabricator.com/D15692
Summary: Changes elsewhere which support spaces before "|" when defining a table so that tables quote properly also accidentally changed these beautiful drawings into remarkup tables.
Test Plan: (( o.O ))
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15690
Summary:
Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.
When we read (e.g., `git fetch`) the logic goes like this:
- Get the read lock (unique to device + repository).
- Read all the versions of the repository on every other device.
- If any node has a newer version:
- Fetch the newer version.
- Increment our version to be the same as the version we fetched.
- Release the read lock.
- Actually do the fetch.
This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.
When we write (e.g., `git push`) the logic goes like this:
- Get the write lock (unique to the repository).
- Do all the read steps so we're up to date.
- Mark a write pending.
- Do the actual write.
- Bump our version and mark our write finished.
- Release the write lock.
This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.
There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.
The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).
(Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)
Test Plan:
- Pushed changes to a cluster-mode repository.
- Viewed web interface, saw "writing" flag and version changes.
- Pulled changes.
- Faked various failures, got sensible states.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15688
Summary:
Ref T4292. This adds some very basic cluster/device data to the new management view. Nothing interesting yet.
Also deal with disabled bindings a little more cleanly.
Test Plan: {F1214619}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15685
Summary:
Ref T4292. This puts a very rough skeleton in place for the new "Manage Repository" UI, somewhat similar to the "Settings" UI.
Right now, it has one panel with no content, and is not reachable from the UI.
Test Plan: {F1214525}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15683
Summary:
Ref T10756. When repositories are properly configured for the cluster (which is hard to set up today), be smart about which repositories are expected to exist on the current host, and only pull them.
This generally allows daemons to pretty much do the right thing no matter how many copies are running, although there may still be some lock contention issues that need to be sorted out.
Test Plan: {F1214483}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10756
Differential Revision: https://secure.phabricator.com/D15682
Summary: Ref T10702
Test Plan: Open a user profile, attempt to award an archived or previously awarded badge, badges dialog should provide a typeahead, and the suggestions should offer details about whether a badge is archived or already awarded.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10702
Differential Revision: https://secure.phabricator.com/D15665
Summary:
Ref T4571. Write more of the missing documentation sections and clarify a few things.
Since the "replicating master" check needs a special permission, imposes a performance penalty, is probably very difficult to misconfigure, and likely not a big deal anyway, just drop the idea of trying to automatically detect + prevent it. We still show if it's an issue on the status page, provided we have permission to check.
When you don't have any cluster databases configured, never stop trying to connect to the default master database. We might want to do this eventually as load reduction, but just don't muddy the waters too much for now while things stabilize.
Test Plan:
- Tested functionality in cluster, non-cluster, and degraded-cluster modes.
- Used status console to monitor a health check cycle.
- Read docs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15679
Summary:
Ref T4571. When a database goes down briefly, we fall back to replicas.
However, this fallback is slow (not good for users) and keeps sending a lot of traffic to the master (might be bad if the root cause is load-related).
Keep track of recent connections and fully degrade into "severed" mode if we see a sequence of failures over a reasonable period of time. In this mode, we send much less traffic to the master (faster for users; less load for the database).
We do send a little bit of traffic still, and if the master recovers we'll recover back into normal mode seeing several connections in a row succeed.
This is similar to what most load balancers do when pulling web servers in and out of pools.
For now, the specific numbers are:
- We do at most one health check every 3 seconds.
- If 5 checks in a row fail or succeed, we sever or un-sever the database (so it takes about 15 seconds to switch modes).
- If the database is currently marked unhealthy, we reduce timeouts and retries when connecting to it.
Test Plan:
- Configured a bad `master`.
- Browsed around for a bit, initially saw "unrechable master" errors.
- After about 15 seconds, saw "major interruption" errors instead.
- Fixed the config for `master`.
- Browsed around for a while longer.
- After about 15 seconds, things recovered.
- Used "Cluster Databases" console to keep an eye on health checks: it now shows how many recent health checks were good:
{F1213397}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15677
Summary:
The way `DateTime` works with epochs is weird, I goofed this by having my server/viewer timezone the same and not noticing.
Also fix an issue where you do `?epoch=...` and then manually fiddle with the control: the control should win.
Test Plan:
- Set viewer and server timezone to different vlaues.
- Created a countdown using `?epoch=...`.
- Created a countdown using `?epoch=...` and fiddling with date controls.
- Created and edited a countdown using date/time control.
- Poked around Calendar to make sure I didn't ruin anything this time (browsed, created event, edited event).
Reviewers: lpriestley, chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15680
Summary:
Ref T4571. If we fail to connect to the master, automatically try to degrade into a temporary read-only mode ("UNREACHABLE") for the remainder of the request, if possible.
If the request was something like "load the homepage", that'll work fine. If it was something like "submit a comment", there's nothing we can do and we just have to fail.
Detecting this condition imposes a performance penalty: every request checks the connection and gives the database a long time to respond, since we don't want to drop writes unless we have to. So the degraded mode works, but it's really slow, and may perpetuate the problem if the root issue is load-related.
This lays the groundwork for improving this case by degrading futher into a "SEVERED" mode which will persist across requests. In the future, if several requests in a short period of time fail, we'll sever the database host and refuse to try to connect to it for a little while, connecting directly to replicas instead (basically, we're "health checking" the master, like a load balancer would health check a web application server). This will give us a better (much faster) degraded mode in a major service disruption, and reduce load on the master if the root cause is load-related, giving it a better chance of recovering on its own.
Test Plan:
- Disabled master in config by changing the host/username, got degraded automatically to UNREACAHBLE mode immediately.
- Faked full SEVERED mode, requests hit replicas and put me in the mode properly.
- Made stuff work, hit some good pages.
- Hit some non-cluster pages.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15674
Summary: Ref T4571. If `cluster.databases` is configured but only has replicas, implicitly drop to read-only mode and send writes to a replica.
Test Plan:
- Disabled the `master`, saw Phabricator automatically degrade into read-only mode against replicas.
- (Also tested: explicit read-only mode, non-cluster mode, properly configured cluster mode).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15672
Summary:
Ref T4571. Allows users to click the "read-only mode" notification to get more information about why an install is in read-only mode.
Installs can be in this mode for several reasons (explicit administrative action, no masters defined, no masters reachable), and it's useful to be able to tell the difference.
Test Plan: {F1212930}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15671
Summary: Fixes T6710. After D15669, we support a proper timeout parameter, so we don't need this hack anymore.
Test Plan: See D15669: forced a MySQL connector, set a low timeout, set a bad database, saw fast failures.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6710
Differential Revision: https://secure.phabricator.com/D15670
Summary:
Ref T4571. Ref T10759. Ref T10758. This isn't complete, but gets most of the job done:
- When `cluster.databases` is set up, most things ignore `mysql.host` now.
- You can `bin/storage upgrade` and stuff works.
- You can browse around in the web UI and stuff works.
There's still a lot of weird tricky stuff to navigate, and this has real no advantages over configuring a single server yet (no automatic failover, etc).
Test Plan:
- Configured `cluster.databases` to point at my `t1.micro` hosts in EC2 (master + replica).
- Ran `bin/storage upgrade`, got a new install setup on them properly.
- Survived setup warnings, browsed around.
- Switched back to local config, ran `bin/storage upgrade`, browsed around, went through setup checks.
- Intentionally broke config (bad hosts, no masters) and things seemed to react reasonably well.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571, T10758, T10759
Differential Revision: https://secure.phabricator.com/D15668
Summary: Ref T4571. The configuration option still doesn't do anything, but add a status panel for basic setup monitoring.
Test Plan:
Here's what a good version looks like:
{F1212291}
Also faked most of the errors it can detect and got helpful diagnostic messages like this:
{F1212292}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15667
Summary:
Ref T4571. This adds a new option which allows you to upgrade your one-host configuration to a multi-host configuration by configuring it.
Doing this currently does nothing. I wrote a lot of words about what it is //supposed// to do in the future, though.
Test Plan:
- Tried to configure the option in all the possible bad ways, got errors.
- Read documentation.
Reviewers: chad
Reviewed By: chad
Subscribers: eadler
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15663
Summary:
Ref T4571. There will be a very long path beyond this, but add a basic read-only mode. You can explicitly enable this to put Phabricator in a sort of "maintenance" mode today if you're swapping databases or something.
In the long term, we'll automatically degrade into this mode if the master database is down.
Test Plan:
- Enabled read-only mode.
- Browsed around.
- Didn't immediately see anything that was totally 100% broken.
Most stuff is 80-90% broken right now. For example:
- Stuff like submitting comments doesn't work, and gives you a confusing, unhelpful error.
- None of the UI really knows that it's read-only. EditEngine stuff should all hide itself and say "you can't add new comments while an install is in read-only mode", for example, but currently does not.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4571
Differential Revision: https://secure.phabricator.com/D15662
Summary: Recurring events will fatal a Calendar with this not set. `newDateTime` requires a date and time to be called property. I think this is correct fix? Fixes T10766
Test Plan: Build a recurring event, pull up /calendar/, see recurring events as expected. Previously, fatal.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: CodeMouse92, Korvin
Maniphest Tasks: T10766
Differential Revision: https://secure.phabricator.com/D15666
Summary: Testing out a new 'nav' layout in Settings / Config. Spent a few days here and couldn't find much better overall.
Test Plan: View each page in Settings and in Config. Save some config options. Test mobile, desktop, tablet.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15659
Summary:
I think this fixes the Mercurial + HTTP cluster issue. PHP adds `HTTP_` but we were not stripping it, so we would convert an `X-Whatever-Zebra` header into an `Http-X-Whatever-Zebra` header.
I don't think this behavior has changed? So maybe it just never worked? Git is more popular than Mercurial and SSH is easier to configure than HTTP, so it's plausible. I'll keep a careful eye on this when it deploys.
Test Plan:
- Set up local service-based Mercurial repository.
- Tried to clone, got similar error to cluster.
- Applied patch, clean clone.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15660
Summary: Fixes T5813, while I'm in here...
Test Plan: Sorted stuff by end date.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5813
Differential Revision: https://secure.phabricator.com/D15657
Summary: Fixes T10684. Fixes T10520. This primarily implements a date/epoch field, and then does a bunch of standard plumbing.
Test Plan:
- Created countdowns.
- Edited countdowns.
- Used HTTP prefilling.
- Created a countdown ending on "Christmas Morning", etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10520, T10684
Differential Revision: https://secure.phabricator.com/D15655
Summary: Closes T10690
Test Plan: Open Badges application, go to Advanced Search, search for a badge by its name and see result.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T10690
Differential Revision: https://secure.phabricator.com/D15656
Summary:
Fixes T8613. This was pretty straightforward, I just never dug into it originally.
`rawResultLimit = 0` just means "no limit", so the fix is to only apply a limit if it is set to some nonzero value.
Also modernize a few pieces of code.
Test Plan: I'm actually not sure this can actually be hit normally? I faked `setGenerateGhosts(true)` into an unrelated query, hit the fatal, then fixed it.
Reviewers: lpriestley, chad
Reviewed By: chad
Maniphest Tasks: T8613
Differential Revision: https://secure.phabricator.com/D15653
Summary: Found another bouncing around.
Test Plan: Review in diff
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15651
Summary: Ref T10262. Instead of dumping an unhelpful 403 "ACCESS DENIED" page on users, explain the most likely cause of the issue and give them a link to return to the file detail page to learn more or get an up-to-date link.
Test Plan: Hit both errors, had a lovely experience with the helpful dialog text.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15650
Summary: Ref T6027. We got a not-very-user-friendly default string before.
Test Plan: Selected "Move", didn't change the dropdown, hit submit. Now, got a nice human-readable description of the issue.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15649
Summary: Bumps to 14px, fixes some on Differential
Test Plan: view various headers in Differential
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15647
Summary:
Ref T10262. Currently, we always render a tag like this when you `{F123}` an image in remarkup:
```
<img src="/xform/preview/abcdef/" />
```
This either generates the preview or redirects to an existing preview. This is a good behavior in general, because the preview may take a while to generate and we don't want to wait for it to generate on the server side.
However, this flickers a lot in Safari. We might be able to cache this, but we really shouldn't, since the preview URI isn't a legitimately stable/permanent one.
Instead, do a (cheap) server-side check to see if the preview already exists. If it does, return a direct URI. This gives us a stable thumbnail in Safari.
Test Plan:
- Dragged a dog picture into comment box.
- Typed text.
- Thing didn't flicker like crazy all the time in Safari.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15646
Summary: Going to render these all normal case instead of all caps, and bump up the font size. Should be more consistent. Yellow if you green anything orange.
Test Plan: grep, lint
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15645
Summary:
Ref T10262. This removes one-time tokens and makes file data responses always-cacheable (for 30 days).
The URI will stop working once any attached object changes its view policy, or the file view policy itself changes.
Files with `canCDN` (totally public data like profile images, CSS, JS, etc) use "cache-control: public" so they can be CDN'd.
Files without `canCDN` use "cache-control: private" so they won't be cached by the CDN. They could still be cached by a misbehaving local cache, but if you don't want your users seeing one anothers' secret files you should configure your local network properly.
Our "Cache-Control" headers were also from 1999 or something, update them to be more modern/sane. I can't find any evidence that any browser has done the wrong thing with this simpler ruleset in the last ~10 years.
Test Plan:
- Configured alternate file domain.
- Viewed site: stuff worked.
- Accessed a file on primary domain, got redirected to alternate domain.
- Verified proper cache headers for `canCDN` (public) and non-`canCDN` (private) files.
- Uploaded a file to a task, edited task policy, verified it scrambled the old URI.
- Reloaded task, new URI generated transparently.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15642
Summary:
Ref T10262. Files have an internal secret key which is partially used to control access to them, and determines part of the URL you need to access them. Scramble (regenerate) the secret when:
- the view policy for the file itself changes (and the new policy is not "public" or "all users"); or
- the view policy or space for an object the file is attached to changes (and the file policy is not "public" or "all users").
This basically means that when you change the visibility of a task, any old URLs for attached files stop working and new ones are implicitly generated.
Test Plan:
- Attached a file to a task, used `SELECT * FROM file WHERE id = ...` to inspect the secret.
- Set view policy to public, same secret.
- Set view policy to me, new secret.
- Changed task view policy, new secret.
- Changed task space, new secret.
- Changed task title, same old secret.
- Added and ran unit tests which cover this behavior.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10262
Differential Revision: https://secure.phabricator.com/D15641
Summary:
This reverts commit 3f50ba90f1.
Fixes T10412. Everyone seems to hate this and I don't feel strongly about it. It's definitely a little weird.
Test Plan: Straight revert.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10412
Differential Revision: https://secure.phabricator.com/D15644
Summary: Fixes T10737. I agree that this is a better behavior than always making them blue (boring).
Test Plan:
what
{F1208333}
wut
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T10737
Differential Revision: https://secure.phabricator.com/D15643
Summary:
Ref T6027. Fixes T10734.
- If one of the projects a task is tagged with isn't visible to the user or doesn't have a board, it won't have columns.
- Don't show options for projects with disabled boards.
Test Plan:
- Viewed task with project with no columns; no fatal.
- Viewed task with project with disabled board; no options to move on that board.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10734, T6027
Differential Revision: https://secure.phabricator.com/D15640
Summary:
Ref T6027. Normally, actions use the same order as the form, but in some cases (like moving stuff on workboards) it makes sense to reorder them explicitly.
Pin "Move on board" near the bottom, and "projects/subscribers" at the bottom. I think these are generally reasonable rules in all cases.
Test Plan: Opened menu, saw slightly better action order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15639
Summary:
Ref T6027. Try this out and see how it feels? Clear issues:
- This definitely shouldn't be at the top.
- You should probably be able to select it multiple times?
- Some of the "which columns show up" rules might need adjustment?
- Diamond marker maybe not great?
Not sure I love this but it doesn't feel //terrible//...
Test Plan: {F1207891}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15638
Summary: Ref T5214. Fixes T10486. Ref T6027. This exposes the `TYPE_COLUMNS` transaction in a usable way via API, and fixes the interactions via prefilling.
Test Plan:
- Created tasks directly into columns via API.
- Moved tasks between columns via API.
- Used `?column=...` to try to create a template task with valid and bogus column PHIDs.
Reviewers: chad
Reviewed By: chad
Subscribers: AmyLewis
Maniphest Tasks: T5214, T6027, T10486
Differential Revision: https://secure.phabricator.com/D15636
Summary: Ref T6027. This adds human-readable rendering for the new `TYPE_COLUMNS` core transactions.
Test Plan: {F1207784}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15635
Summary:
Ref T6027. We currently have two different transaction types:
- `TYPE_PROJECT_COLUMNS` does most of the work, but has a sort of weird structure and isn't really suitable for API use.
- `TYPE_COLUMN` is this weird, junk transaction which mostly just creates the other transaction.
Merge them into a single higher-level `TYPE_COLUMNS` transaction which works properly and has a sensible structure and comprehensive error checking.
Remaining work here:
- I've removed the old rendering logic, but not yet added new logic. I need to migrate the old transaction types and add new rendering logic.
- Although the internal representation is now //suitable// for use in the API, it isn't properly exposed yet.
Test Plan:
- Created tasks into a column.
- Ran unit tests.
- Moved tasks between columns.
- Will perform additional testing in followups.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15634
Summary: Ref T10671
Test Plan: Open Conduit application, open `badges.edit` or `badges.search`, create, edit, or query for a badge.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10671
Differential Revision: https://secure.phabricator.com/D15622
Summary: Modernize Files a bit, use newPage
Test Plan: New file, drag and drop file, view file, edit file
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15631
Summary: Ref T7673. This is really just so I can force admin.phacility.com logout when you log out of an instance, but there are a few other things we could move here eventually, like the WILLREGISTERUSER event.
Test Plan: Logged out of an instance, got logged out of parent (see next change).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7673
Differential Revision: https://secure.phabricator.com/D15629
Summary:
Ref T7303. Currently, our handling of "scope" is fairly rigid and adheres to the spec, but some of these behaviors don't make much sense in practice.
Soften some behaviors and make them more flexible:
**Soft Failure on Unknown Permissions**: If a client asks for a permission we don't know about, just warn that we don't recognize it instead of fataling. In particular, I plan to make `offline_access` and `whoami` implicit. Older clients that request these permissions will still work fine as long as we don't hard-fatal.
**Move `user.whoami` to ALWAYS scope**: Make `whoami` a default permission. We've already done this, in effect; this just formalizes it.
**Tokens no longer expire**: Make `offline_access` (infinite-duration tokens) a default permission. I think the OAuth model doesn't map well to reality. It is common for other providers to issue "temporary" tokens with a duration of multiple years, and the refesh workflow is sort of silly. We can add a "temporary" scope later if we need temporary tokens.
This flow was potentially extra silly with the "log out of Phacility" use case, where we might need to have you log in again before we could log you out, which is bizarre and senseless. Avoid this nonsense.
**Move away from granular permissions**: Users currently get to pick-and-choose which permissions they grant, but this likely rarely/never works in practice and is fairly hostile since applications can't communicate which permissions they need. Applications which can actually operate with only some subset of permissions can make separate requests (e.g., when you activate "cool feature X", it asks for X permission). I think applications that do this are rare; pretty much everything just asks for tons of permissions and everyone grants them.
Making this all-or-nothing is better for well-behaved applications and better for users. It's also slightly better for overzealous applications that ask for more than they need, but whatever. Users can make an informed decision, hopefully, and I plan to let administrators force applications to a subset of permissions once we introduce meaningful scopes.
Test Plan:
- Generated tokens.
- Used tokens.
- Authorized an instance.
- Faked some bogus scopes, got clean authorization.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15621
Summary: Converts over to `newPage`
Test Plan: Pull up Settings panel, test a few.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15625
Summary: Ref T7303. This interaction is very oldschool; modernize it to enable/disable instead of "nuke from orbit".
Test Plan:
- Enabled applications.
- Disabled applications.
- Viewed applications in list view.
- Generated new tokens.
- Tried to use a token from a disabled application (got rebuffed).
- Tried to use a token from an enabled application (worked fine).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15620
Summary: Runs through Releeph to move to new UI and `newPage`
Test Plan: Ran through product, release, branch, everything seems to work.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15623
Summary: Ref T10570. Earlier work on that task made tables activate even if indented so they work in quoted blocks. However, the documentation doesn't explicitly mark them in code blocks, so it turned them into markup.
Test Plan:
Used `bin/diviner generate` to regenerate documentation, verified it now renders properly.
{F1205818}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10570
Differential Revision: https://secure.phabricator.com/D15619
Summary: Swaps over to new hotness
Test Plan: Pull up mail view, see new UI
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15616
Summary: Swaps over to `newPage` and `newDialog`
Test Plan: Unsure how to actually pull these up?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15617
Summary: Updates to use new UI
Test Plan: Save a custom query, edit a custom query
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15618
Summary: Updates Console and Operations page.
Test Plan: Pull up Console, pull up status page
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15615
Summary: Moves these Maniphest pages over to modern UI, components
Test Plan: Batch Edit Tasks, View some reports.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15614
Summary: Ref T7303. This application is currently stone-age tech (no transactions, hard "delete" action). Bring it up to modern specs.
Test Plan:
- Created and edited an OAuth application.
- Viewed transaction record.
- Tried to create something with no name, invalid redirect URI, etc. Was gently rebuffed with detailed explanatory errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15609
Summary: View various conduit pages and update to new UI and add calls to newPage
Test Plan: View list, view method, make a call.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15613
Summary: Uses modern UI, `newPage`, etc. Changes table behavior to always scroll if too large for container, can't find anything this breaks, but be on the lookout.
Test Plan: Pull up help and view pages, search for some people and projects.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15611
Summary: Single callsite, swap to `newPage`
Test Plan: Visit page, see same status message. Also remove device ready flag.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15610
Summary:
Fixes T10721. When trying to load commits by identifier, we would take some bad pathways in Subversion if the repository had no callsign and end up missing the commits.
Fix this logic so it works for either callsigns (e.g., if passed `rXyyy`) or with PHIDs if passed repositories.
Test Plan:
- Viewed SVN commit in a Subversion repository with no callsign.
- Added a callsign, looked at it again.
- Viewed non-SVN commits in callsign and non-callsign repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10721
Differential Revision: https://secure.phabricator.com/D15607
Summary: Ref T7303. Small modernization.
Test Plan:
- Searched by various users.
- Viewed all, reordered, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15606
Summary:
Ref T7303. Ref T7673. This implements an "auth.logout" which:
- terminates all web sessions;
- terminates the current OAuth token if called via OAuth; and
- may always be called via OAuth.
(Since it consumes an OAuth token, even a "malicious" OAuth application can't really be that much of a jerk with this: it can't continuously log you out, since calling the method once kills the token. The application would need to ask your permission again to get a fresh token.)
The primary goal here is to let Phacility instances call this against the Phacility upstream, so that when you log out of an instance it also logs you out of your Phacility account (possibly with a checkbox or something).
This also smooths over the session token code. Before this change, your sessions would get logged out but when you reloaded we'd tell you your session was invalid.
Instead, try to clear the invalid session before telling the user there's an issue. I think that ssentially 100% of invalid sessions are a result of something in this vein (e.g., forced logout via Settings) nowadays, since the session code is generally stable and sane and has been for a long time.
Test Plan:
- Called `auth.logout` via console, got a reasonable logout experience.
- Called `auth.logout` via OAuth.
- Tried to make another call, verified OAuth token had been invalidated.
- Verified web session had been invalidated.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303, T7673
Differential Revision: https://secure.phabricator.com/D15594
Summary:
Ref T7303. OAuth scope handling never got fully modernized and is a bit of a mess.
Also introduce implicit "ALWAYS" and "NEVER" scopes.
Always give tokens access to meta-methods like `conduit.getcapabilities` and `conduit.query`. These do not expose user information.
Test Plan:
- Used a token to call `user.whoami`.
- Used a token to call `conduit.query`.
- Used a token to try to call `user.query`, got rebuffed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15593
Summary:
Ref T7303. This inches toward properly-behaved cluster logout.
- Use IDs instead of PHIDs in URIs.
- Slightly more modern code.
- Fix some crumb stuff.
Test Plan: Created, edited, viewed, deleted, showed secret for, authorized, test-auth'd an application.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7303
Differential Revision: https://secure.phabricator.com/D15592
Summary: Updates various /people/ pages for new UI and newPage
Test Plan: Review creating people, new people, sending invites, editing a profile, setting a new picture, something with LDAP
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15604
Summary: Cleans up Feed Story individual page
Test Plan: View an individual story by clicking on date.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15599
Summary: No UI updates, just swapping over to `newPage`
Test Plan: Pull up each page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, yelirekim
Differential Revision: https://secure.phabricator.com/D15601
Summary: Updates fund for new edit UI
Test Plan: Create Fund, Edit Fund
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15595
Summary: Updates Macro Audit Edit page with new UI and newPage
Test Plan: Edit Audio on macro, see new layout, save file.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15598
Summary: Brings the edit paths page in owners up to new UI
Test Plan: Edit some paths, yo.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15596
Summary: Closes T8940, recipient list in badge view should show awarder and date info. Took a first stab at how we want to make the date look, but not sure. Looks odd as it is.
Test Plan: Open badge that has awards. Each recipient in list should have a subheader such as "Awarded by ... on ..."
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8940
Differential Revision: https://secure.phabricator.com/D15590
Summary: Missed converting this page, scenario. The box was poorly formatted.
Test Plan: Create a new document that needs signed, verify box is correctly spaced and colored.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15591
Summary: Forgot a more efficient way to get badge from award
Test Plan: Badges on user profiles should still show up with awarder handle on the back of the card
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15589
Summary: Ref T8940
Test Plan: Award badge, open recipient profile page, badge should appear in badges list, and flipping the badge card should show who awarded it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: chad, Korvin
Maniphest Tasks: T8940
Differential Revision: https://secure.phabricator.com/D15570
Summary: Converts Config to new UI, updates to `newPage`
Test Plan: Review all pages in Config, setup issues, ignore an issue, edit a config option
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15587
Summary: Updates Legalpad Manage/Edit with new UI layouts.
Test Plan: Wrote a new document with and without a preamble, edit document, sign document
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15576
Summary: Pulls everything over to two column UI and new edit pages. Removed history view and consolidated some pages.
Test Plan: New Panel, Edit Panel. New Dashboard, Edit Dashboard, View Standalone pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15588
Summary: Minor, moves to `newPage`
Test Plan: Test both pages, still work
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15582
Summary: Little straggler here, updates to `newPage`
Test Plan: Review a document, no visibile changes
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15580
Summary: Updates Conpherence pages to use `newPage`
Test Plan: View a Room, view list of joined rooms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15578
Summary: Just clearing these all out.
Test Plan: Visit channel list and log page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15577
Summary:
Ref T10709. Two issues:
- If a user sends an invalid `!command`, we can throw, which means we don't return HTTP 200. This makes Mailgun re-send the mail later.
- We don't parse headers of the modern API correctly, so the "Message-ID" failsafe doesn't work. Parse them correctly. I //believe// Mailgun's API changed at some point.
Test Plan:
This is difficult to test exhaustively in isolation. I used Mailgun's web tools to verify the format of the hook request, and faked some requests locally.
I'll keep an eye on this as it goes to production and make sure the fix is correct there.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10709
Differential Revision: https://secure.phabricator.com/D15575
Summary: Modernize and use newer UI
Test Plan: Bounce around various views.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15574
Summary: Updates various Phriction pages to match new UI
Test Plan: New Document, Edit Document, View History, Revert Change
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15572
Summary: Swap over to modern components, `newPage` and `handleRequest`.
Test Plan: `arc lint` :(
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D15571
Summary: Runs through Meta, cleaned up policies and editing email addresses to new UI
Test Plan: Set a new Email address for Maniphest, edit policies.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15569
Summary: Fixes T10704. This is just bad copy-paste -- "O" for "old" should be "N" for "new".
Test Plan:
- Followed steps on T10704.
- Applied patch.
- Marked inline done, replied, etc. No more JS errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10704
Differential Revision: https://secure.phabricator.com/D15566
Summary: Cleans up Pholio, moves to two column layout, fix some transaction inconsistencies. This moves "Image" to the MainColumn, which feels fine, but I think we'll likely want some sort of "fullscreen" option for Pholio V2 like we have on workboards perhaps.
Test Plan:
New Mock, Edit Mock, View Mock.
{F1200450}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15564
Summary: Ref T9007
Test Plan: Navigate to "Advanced Search" in Badges, order by rarity, then by commonality. Rarest and most common badges should be ordered, respectively.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9007
Differential Revision: https://secure.phabricator.com/D15555
Summary: This doesn't hit the ambiguous case in Diffusion so it seems fine to make it more consistent.
Test Plan: Looked at a little task-o.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15561
Summary: Walks through various object, rule, create forms and transcripts in Herald. Slightly nicer looking.
Test Plan: Make rules, see rules, edit rules, see transcripts.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15559
Summary: Fixes T10672. Cleaning this up myself since I was responsible for the implementation.
Test Plan: Leave a comment, Edit a badge, create a badge.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10672
Differential Revision: https://secure.phabricator.com/D15556
Summary: Adds basic commenting to Fund Initiatives.
Test Plan: Leave a comment, see comment.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15554
Summary: Uses BLUE_PROPERTY on Recipients box, removes redundent properties since we render the badge itself already.
Test Plan: View a badge with and without a description.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15553
Summary: [WIP] Tossing this up for safety and to read through it. Need to test, update some of the other flows. This updates everything in Auth for new UI and modern conventions.
Test Plan: Loooots of random testing, new providers, edit providers, logging out, forgot password... more coming.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15550
Summary:
[WIP] Allows awarding a badge from a user profile. Unsure of the interactions here if a user can't award any badges, or if we should just hide this.
Fixes T10688
Fixes T10318
Test Plan: Award some badges. Steal them back.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10318, T10688
Differential Revision: https://secure.phabricator.com/D15544
Summary: Ref T8941
Test Plan: Create an object and create multiple transactions, some time apart to ensure that time clumping isn't interfering. Make sure that events that are large enough to have a dropdown menu show badges under author pic.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8941
Differential Revision: https://secure.phabricator.com/D15543
Summary:
Ref T10537. Currently, Nuance has a `NuanceRequestor` object, intended to represent the external user who created content (e.g., a GitHub account or a Twitter account or whatever).
This object is currently almost unused, and its design predates Doorkeeper. In D15541, I chose to use doorkeeper objects instead of NuanceRequestor objects to represent requestors.
I don't currently anticipate a need for such an object, given that we have Doorkeeper. If we do need it in the future for some reason, it would be fairly easy to restore it, create a requestor type which wraps a Doorkeeper object, and then migrate. Not super thrilling to do that, but not a huge mess.
`NuanceItem` still has a `requestorPHID`, but this is now a less formal object PHID instead of a more formal Requestor-object PHID, and holds a doorkeeper exeternal object PHID for GitHub events.
Test Plan:
- Grepped for `nuancerequestor`.
- Ran `bin/storage upgrade -f`.
- Grepped for `requestor`, remaining uses of this term seem reasonable/correct.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15546
Summary: This is kinda bad in terms of UI (It just makes a json of the thing and diffs that), but it's a start.
Test Plan: edit rule, create rule, add/remove/edit conditions, actions
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15542
Summary:
Ref T10538. Ref T10537. This creates PHIDs which represent GitHub users, and uses them as the actors for synchronized comments.
I've just made them Doorkeeper objects. There are three major kinds of objects they //could// possibly be:
- Nuance requestor objects.
- External account objects.
- Doorkeeper objects.
I don't think we actually need distinct nuance requestor objects. These don't really do anything right now, and were originally created before Doorkeeper. I think Doorkeeper is a superset of nuance requestor functionality, and better developed and more flexible.
Likewise, doorkeeper objects are much more flexible than external account objects, and it's nice to imagine that we can import from Twootfeed or whatever without needing to build full OAuth for it. I also like less stuff touching auth code, when possible.
Making these separate from external accounts does make it a bit harder to reconcile external users with internal users, but I think that's OK, and that it's generally desirable to show the real source of a piece of content. That is, if I wrote a comment on GitHub but also have a Phabricator account, I think it's good to show "epriestley (GitHub)" (the GitHub user) as the author, not "epriestley" (the Phabricator user). I think this is generally less confusing overall, and we can add more linkage later to make it clearer.
Test Plan:
{F1194104}
{F1194105}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537, T10538
Differential Revision: https://secure.phabricator.com/D15541
Summary: Ref T10538. This probably gets push events where GitHub does not recognize the author wrong, but I don't have any of those yet.
Test Plan: Added and ran unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10538
Differential Revision: https://secure.phabricator.com/D15540
Summary: Ref T10537. Add a new content source for Nuance. Prepare for better author attribution.
Test Plan: {F1194038}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15539
Summary: Fixes T10670, for users with exclusively archived badges, user profile should show "no badges" message instead of blank box
Test Plan: Award badge to user with no badges, archive badge, user profile should show "no badges" message under badges.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10670
Differential Revision: https://secure.phabricator.com/D15538
Summary:
Fixes T10234. This is a more thorough fix.
Root issue is that some time around D13589, we started hitting an object cache for `loadCustomInlineRules()`, but didn't adjust the code to account for that.
So if a page created multiple similar engines, we'd return the same `$rule` object for multiple engines, call `setEngine()` on it with different engines, and then possibly try to render using an already-expired engine the second time through.
Instead, create a separate `$rule` object for each separate `$engine`.
Test Plan:
Repro is something like this:
- Create a custominlinerule which uses an engine.
- Purge the remarkup cache.
- Load a page which uses the rule in two engines (e.g., in a revision description, and also in an inline comment).
- Before change: second one could fatal. After change: clean load.
Reviewers: thoughtpolice, chad
Reviewed By: thoughtpolice, chad
Subscribers: thoughtpolice, eadler
Maniphest Tasks: T10234
Differential Revision: https://secure.phabricator.com/D15535
Summary:
Ref T10537.
- Let nuance items render custom curtain panels.
- Add a custom panel linking to the imported task, if one exists.
- Actually extract comments properly.
Test Plan:
Unit tests, plus:
{F1193800}
{F1193801}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15537
Summary: Ref T10677, Awarding/revoking badge should create a feed story on homepage with badge handle recipient handles
Test Plan: Award/revoke badge, open Feed, should see story with badge link and recipient links.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10677
Differential Revision: https://secure.phabricator.com/D15534
Summary: Ref T10677, awarding/revoking a badge should create timeline entries with titles that are more clear (excludes homepage feed stories)
Test Plan: Award/revoke a badge to single or multiple users. See timeline entries that reflect those actions.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10677
Differential Revision: https://secure.phabricator.com/D15533
Summary: Cleans up EditEngine, adds new layout to EditEngine and descendents
Test Plan: Test creating a new form, reordering, marking and unmarking defaults. View new forms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15531
Summary: Adds headers, new layout to edit panels on Almanac.
Test Plan: Pull up each edit panel in sandbox, save form.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15527
Summary: Ref T8996, Convert badge recipients from Edges to actual BadgeAward objects
Test Plan: Create badge, award it to recipient. Make sure adding/removing recipients works. (Still need to migrate exisiting recipients to new table and need to create activity feed blurbs)
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: chad, Korvin
Maniphest Tasks: T8996
Differential Revision: https://secure.phabricator.com/D15014
Summary:
Fixes T10385. Two issues:
- `$show_blame` and `$show_color` were improperly swapped.
- Code to hide these columns got dropped somewhere, probably in my recent-ish rewrite.
Test Plan:
- Showed/hid blame.
- Showed/hid colors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10385
Differential Revision: https://secure.phabricator.com/D15528
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.
Split ContentSource apart so applications can add new content sources.
Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.
- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.
{F1190182}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15521
Summary: Updates the Harbormaster UI to match the new two column everywhere else.
Test Plan: Did best I could, tested builds, plans, steps, buildables. Unable to test lint/unit locally, I need to set that up. Kick the tires for me pls. :3
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15523
Summary: Ref T10537. Show when an object is bridged to something external.
Test Plan: {F1190099}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15520
Summary:
Fixes T10665. See that task for discussion.
Because `$head_map` is not properly re-initialized for each ref we check, pushes which affect multiple branches (say, "A" and "B") can have information bleed from the first branch check to the second branch.
To trigger a problem behavior, you can push one commit which updates an existing branch, plus one commit which creates a new branch. If they process in the right order, the `$head_map` from the updated branch will bleed into the `$head_map` for the new branch and trigger an incorrect head split detection.
Test Plan:
- Pushed a set of changes which updated `branch-a` and created `branch-b`.
- Before change: improper detection of split heads.
- After change: clean push.
- Pushed a set of changes which split the head of `branch-d`.
- Correct detection of split heads.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10665
Differential Revision: https://secure.phabricator.com/D15522
Summary: Ref T10538. This makes us render better human-readable descriptions of more GitHub event types.
Test Plan: Ran unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10538
Differential Revision: https://secure.phabricator.com/D15516
