Summary:
`RepositoryStatusMessage` is basically a key/value table associated with a repository that I'm using to let the daemons store the most recent event of a given type, so we can easily show it on the status dashboard. I think this will be a lot easier for users to figure out than digging through logfiles.
I'm also going to write the "this needs a pull" status here eventually, for reducing the time lapse between pushes and discovery.
- Add storage for these messages.
- Have the pull engine populate the INIT phase. I'll do the FETCH phase next.
- Update the status readout to show all the various states.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7461
Summary: I'm planning to add more detailed info to Diffusion itself, but catch the big issue here.
Test Plan: Hit config issue locally, then resolved it.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7439
Summary: Ref T2230. This cleans up D7442, by using `git for-each-ref` everywhere we can, in a basically reasonable way.
Test Plan:
In bare and non-bare repositories:
- Ran discovery with `bin/repository discover`;
- listed branches on `/diffusion/X/`;
- listed tags on `/diffusion/X/`;
- listed tags, branches and refs on `/diffusion/rXnnnn`.
Reviewers: btrahan, avivey
Reviewed By: avivey
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7447
Summary:
Fixes T3619. These URIs are valid:
git@domain.com:/path (Git SCP-style implicit SSH)
ssh://git@domain.com/path (Explicit SSH)
This URI, arrived at by adding "ssh://" to the front of an SCP-style URI, is not:
ssh://git@domain.com:/path
Detect URIs in this form and reject them. See T3619.
Test Plan:
{F75486}
Also set some valid URIs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3619
Differential Revision: https://secure.phabricator.com/D7431
Summary: Looks like this is pretty straightforward; same as the reads except mark it as needing PUSH.
Test Plan: Ran `git push`, pushed over SSH to a hosted repo.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7425
Summary: Mostly ripped from D7391. No writes yet.
Test Plan: Ran `git clone` against a local over HTTP, got a clone.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7423
Summary:
- Add web UI for configuring SSH hosting.
- Route git reads (`git-upload-pack` over SSH).
Test Plan:
>>> orbital ~ $ git clone ssh://127.0.0.1/
Cloning into '127.0.0.1'...
Exception: Unrecognized repository path "/". Expected a path like "/diffusion/X/".
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/X/
Cloning into 'X'...
Exception: No repository "X" exists!
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/MT/
Cloning into 'MT'...
Exception: This repository is not available over SSH.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/P/
Cloning into 'P'...
Exception: TODO: Implement serve over SSH.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7421
Summary:
Mostly ripped from D7391, with some changes:
- Serve repositories at `/diffusion/X/`, with no special `/git/` or `/serve/` URI component.
- This requires a little bit of magic, but I got the magic working for Git, Mercurial and SVN, and it seems reasonable.
- I think having one URI for everything will make it easier for users to understand.
- One downside is that git will clone into `X` by default, but I think that's not a big deal, and we can work around that in the future easily enough.
- Accept HTTP requests for Git, SVN and Mercurial repositories.
- Auth logic is a little different in order to be more consistent with how other things work.
- Instead of AphrontBasicAuthResponse, added "VCSResponse". Mercurial can print strings we send it on the CLI if we're careful, so support that. I did a fair amount of digging and didn't have any luck with git or svn.
- Commands we don't know about are assumed to require "Push" capability by default.
No actual VCS data going over the wire yet.
Test Plan:
Ran a bunch of stuff like this:
$ hg clone http://local.aphront.com:8080/diffusion/P/
abort: HTTP Error 403: This repository is not available over HTTP.
...and got pretty reasonable-seeming errors in all cases. All this can do is produce errors for now.
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7417
Summary:
Basically straight from D7391. The differences are basically:
- Policy stuff is all application-scope instead of global-scope.
- Made a few strings a little nicer.
- Deleted a bit of dead code.
- Added a big "THIS DOESN'T WORK YET" warning.
Test Plan: See screenshots.
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7416
Summary: No editing or view yet, just adds the schema and a policy default. Part of D7391.
Test Plan: `bin/storage upgrade`
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7415
Summary:
Ref T2231. This:
- Activates the new multi-step workflow, and exposes it in the UI.
- Adds "can create", "default view" and "default edit" capabilities.
- Provides a default value for `repository.default-local-path` and forces repositories into it by default. It's still editable, but Phabricator gets it correct (for some definition of correct) by default now.
Test Plan: Created some new repositories with the new workflow.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1286, T2231
Differential Revision: https://secure.phabricator.com/D7413
Summary: Ref T2231. This just moves the "Delete" dialog from Repositories to Diffusion. This dialog just shows instructions and isn't interesting.
Test Plan: {F75093}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7412
Summary: Fixes T1286. Ref T2231. See previous diffs; same as the others but does "Local Path".
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1286, T2231
Differential Revision: https://secure.phabricator.com/D7409
Summary: Ref T2231. Crumbs in the Diffusion edit workflow are a bit wonky, with stuff like "rP (master)" which isn't very useful and no link back to the main "Edit" page. Make them consistent across all the screens.
Test Plan: Loaded a bunch of these screens and saw sane crumbs on all of them.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7407
Summary:
@chad is hitting an issue described in P961, which I think is this bug in PHP: https://bugs.php.net/bug.php?id=43200
Work around it by defining a "PHIDInterface" and having both "Flaggable" and "Policy" extend it, so that there is only one `getPHID()` declaration.
Test Plan: shrug~
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D7408
Summary: Ref T2231. Brings "Notify/Publish" and "Autoclose" to the new UI.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7402
Summary: See title. Fixes T1809.
Test Plan:
verified each type that has flaggable interface still can be flagged
verified that new custom query filter works
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T1809
Differential Revision: https://secure.phabricator.com/D7392
Summary: First cut of an 'info panel' for phabricator. Basic concept is for display a list of items with a bit more info and depth and an object item list. Projects could be a good first example.
Test Plan: UIExamples
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7398
Summary:
Gets rid of some old Differential-specific nonsense and replaces it with general runtime-pluggable Remarkup rules.
Facebook: This removes two options which may be in use. Have any classes being added via config here just subclass the new abstract bases instead. This should take 5 seconds to fix. You can adjust order by overriding `getPriority()` on the rules, if necessary.
Test Plan: See comments.
Reviewers: btrahan
Reviewed By: btrahan
CC: FacebookPOC, andrewjcg, aran
Differential Revision: https://secure.phabricator.com/D7393
Summary:
Ref T1049. I don't really want to sink too much time into this right now, but a seemingly reasonable architecture came to me in a dream. Here's a high-level overview of how things fit together:
- **"Build"**: In Harbormaster, "build" means any process we want to run against a working copy. It might actually be building an executable, but it might also be running lint, running unit tests, generating documentation, generating symbols, running a deploy, setting up a sandcastle, etc.
- `HarbormasterBuildable`: A "buildable" is some piece of code which build operations can run on. Generally, this is either a Differential diff or a Diffusion commit. The Buildable class just wraps those objects and provides a layer of abstraction. Currently, you can manually create a buildable from a commit. In the future, this will be done automatically.
- `HarbormasterBuildStep`: A "build step" is an individual build operation, like "run lint", "run unit", "build docs", etc. The step defines how to perform the operation (for example, "run unit tests by executing 'arc unit'"). In this diff, this barely exists.
- `HarbormasterBuildPlan`: This glues together build steps into groups or sequences. For example, you might want to "run unit", and then "deploy" if the tests pass. You can create a build plan which says "run step "unit tests", then run step "deploy" on success" or whatever. In the future, these will also contain triggers/conditions ("Automatically run this build plan against every commit") and probably be able to define failure actions ("If this plan fails, send someone an email"). Because build plans will run commands, only administrators can manage them.
- `HarbormasterBuild`: This is the concrete result of running a `BuildPlan` against a `Buildable`. It tracks the build status and collects results, so you can see if the build is running/successful/failed. A `Buildable` may have several `Build`s, because you can execute more than one `BuildPlan` against it. For example, you might have a "documentation" build plan which you run continuously against HEAD, but a "unit" build plan which you want to run against every commit.
- `HarbormasterBuildTarget`: This is the concrete result of running a `BuildStep` against a `Buildable`. These are children of `Build`. A step might be able to produce multiple targets, but generally this is something like "Unit Tests" or "Lint" and has an overall status, so you can see at a glance that unit tests were fine but lint had some issues.
- `HarbormasterBuildItem`: An optional subitem for a target. For lint, this might be an individual file. For unit tests, an individual test. For normal builds, an executable. For deploys, a server. For documentation generation, there might just not be subitems.
- `HarbormasterBuildLog`: Provides extra information, like command/execution transcripts. This is where stdout/stderr will get dumped, and general details and other messages.
- `HarbormasterBuildArtifact`: Stores side effects or results from build steps. For example, something which builds a binary might put the binary in "Files" and then put its PHID here. Unit tests might put coverage information here. Generally, any build step which produces some high-level output object can use this table to record its existence.
This diff implements almost nothing and does nothing useful, but puts most of these object relationships in place. The two major things you can't easily do with these objects are:
1) Run arbitrary cron jobs. Jenkins does this, but it feels tacked on and I don't know of anyone using it for that. We could create fake Buildables to get a similar effect, but if we need to do this I'd rather do it elsewhere in general. Build and cron/service/monitoring feel like pretty different problems to me.
2) Run parameterized/matrix steps (maybe?). Bamboo has this plan/stage/task/job breakdown where a build step can generate a zillion actual jobs, like "build client on x86", "build server on x86", "build client on ARM", "build server on ARM", etc. We can sort of do this by having a Step map to multiple Targets, but I haven't really thought about it too much and it may end up being not-great. I'd guess we have like an 80% chance of getting a clean implementation if/when we get there. I suspect no one actually needs this, or when they do they'll just implement a custom Step and it can be parameterized at that level. I'm not too worried about this overall.
The major difference between this and Jenkins/Bamboo/TravisCI is that all three of those are **plan-centric**: the primary object in the system is a build plan, and the dashboard shows you all your build plans and the current status. I don't think this is the right model. One disadvantage is that you basically end up with top-level messaging that says "Trunk is broken", not "Trunk was broken by commit af32f392f". Harbormaster is **buildable-centric**: the primary object in the system is stuff you can run build operations against (commits/branches/revisions), and actual build plans are secondary. The main view will be "recent commits on this branch, and whether they're good or not" -- which I think is what's most important in a larger/more complex product -- not the pass/fail status of all jobs. This also makes it easier and more natural to integrate with Differential and Diffusion, which both care about the overall status of the commit/revision, not the current status of jobs.
Test Plan: Poked around, but this doesn't really do anything yet.
Reviewers: btrahan
Reviewed By: btrahan
CC: zeeg, chad, aran, seporaitis
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D7368
Summary: Ref T4010. Adds a history page and restores the transaction title strings, which previously sort-of existed in the defunct feed story class.
Test Plan: See screenshots.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T4010
Differential Revision: https://secure.phabricator.com/D7371
Summary:
Ref T4010. Projects have a weird proto-version of ApplicationTransactions which is very similar but not quite the same.
Move the storage to a modern format, but keep all the other code for now.
Test Plan: Migrated project transactions; edited projects.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4010
Differential Revision: https://secure.phabricator.com/D7370
Summary:
Ref T1344. This is //very// rough. Some UI issues:
- Empty states for the board and columns are junky.
- Column widths are crazy. I think we need to set them to fixed-width, since we may have an arbitrarily large number of columns?
- I don't think we have the header UI elements in M10 yet and that mock is pretty old, so I sort of very roughly approximated it.
- What should we do when you click a task title? Popping the whole task in a dialog is possible but needs a bunch of work to actually work. Might need to build "sheets" or something.
- Icons are slightly clipped for some reason.
- All the backend stuff is totally faked.
Generally, my plan is just to use these to implement all of T390. Specifically:
- "Kanban" projects will have "Backlog" on the left. You'll drag them toward the right as you make progress.
- "Milestone" projects will have "No Milestone" on the left, then "Milestone 9", "Milestone 8", etc.
- "Sprint" projects will have "Backlog" on the left, then "Sprint 31", "Sprint 30", etc.
So all of these things end up being pretty much exactly the same, with some minor text changes and new columns showing up on the left vs the right or whatever.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran, sascha-egerer
Maniphest Tasks: T1344
Differential Revision: https://secure.phabricator.com/D7374
Summary:
Fixes T3675.
- Maniphest had a couple of old non-event listeners; move them to events.
- Make most of the similar listeners a little more similar.
- Add checks for access to the application.
Test Plan:
- Viewed profile, project, task, revision.
- Clicked all the actions.
- Blocked access to various applications and verified the actions vanished.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7365
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:
- Introduce `PhabricatorEventListener`, which has an application.
- Populate this for event listeners installed by applications.
- Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.
This doesn't actually change any behaviors.
Test Plan: Viewed Maniphest, Differntial, People.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7364
Summary:
This is primarily a client request, and a little bit use-case specific, but policies seem to be holding up well and I'm getting more comfortable about maintaining this. Much if it can run through ApplicationTransactions.
Allow the ability to edit status, policies, priorities, assignees and projects of a task to be restricted to some subset of users. Also allow bulk edit to be locked. This affects the editor itself and the edit, view and list interfaces.
Test Plan: As a restricted user, created, edited and commented on tasks. Tried to drag them around.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7357
Summary: Drop the "Pro" bit.
Test Plan: Created/edited tasks, moved tasks around, generally made a mess. Nothing burned down.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7352
Summary:
I'll hold this for a couple weeks.
These classes are now only used to render legacy feed stories. I don't plan to migrate the stories since I don't think they're particularly valuable, and migrating them would be complex and time consuming.
With these classes removed, legacy Maniphest feed stories simply vanish from feed.
Test Plan: `grep`, viewed feed, verified it worked but omitted old-style stories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7114
Summary:
Fixes T2146. This is a really simple approach, you just do:
!print .rule {
whatever: blah;
}
And it transforms it into:
.printable .rule {
whatever: blah;
}
@media print {
.rule {
whatever: blah;
}
}
So we end up with these rules twice, but they should compress well and we shouldn't need that many of them, and this fix is way way simpler than all the nonsense I discussed in T2146.
Test Plan:
- Added a unit test.
- Added a simple rule to throw away the menubar when printing.
- Checked the latter with `/?__print__=1`.
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T2146
Differential Revision: https://secure.phabricator.com/D7363
Summary:
Ref T3569. Fixes T3567. When figuring out how much time has been spent on an object, subtract "preemptive" events which interrupted the object.
Also, make the UI look vaguely sane:
{F72773}
Test Plan: Added a bunch of unit tests, mucked around in the UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, skyronic, aran
Maniphest Tasks: T3567, T3569
Differential Revision: https://secure.phabricator.com/D7349
Summary: Makes it easy to choose distinctive icons for projects.
Test Plan:
{F71018}
{F71020}
{F71019}
{F71021}
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D7333
Summary: Ref T603. Give countdowns proper UI-level policy controls, and an application-level default policy. Put policy information in the header.
Test Plan:
- Adjusted default policy.
- Created new countdowns.
- Edited countdowns.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7322
Summary: Ref T603. Gives the create/edit interface a policy control, and adds an application-level default.
Test Plan: Created and edited polls.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7321
Summary:
Ref T603. Fixes T2823. This updates Paste and Macro.
- **Paste**
- Added default view policy.
- I didn't add a "create" policy, since I can't come up with any realistic scenario where you'd give users access to pastes but not let them create them.
- **Macro**
- Added a "manage" policy, which covers creating and editing macros. This lets an install only allow "People With An Approved Sense of Humor" or whatever to create macros.
- Removed the "edit" policy, since giving individual users access to specific macros doesn't make much sense to me.
- Changed the view policy to the "most public" policy the install allows.
- Added view policy information to the header.
Also fix a couple of minor things in Maniphest.
Test Plan:
- Set Paste policy, created pastes via web and Conduit, saw they got the right default policies.
- Set Macro policy, tried to create/edit macros with valid and unauthorized users.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2823, T603
Differential Revision: https://secure.phabricator.com/D7317
Summary: Ref T3958. Adds a provider for Mozilla's Persona auth.
Test Plan:
- Created a Persona provider.
- Registered a new account with Persona.
- Logged in with Persona.
- Linked an account with Persona.
- Dissolved an account link with Persona.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3958
Differential Revision: https://secure.phabricator.com/D7313
Summary: Ref T603. Allow the endpoint to take an existing policy PHID to populate the editor and return a useful datastructure.
Test Plan: In the next revision, actually hooked this up.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7299
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary: UX on this could probably be better 'disabled' crumbs don't appear to have any visible difference, and the policy error has to load the /create page rather than being a modal - not sure on the way to fix these.
Test Plan: Tried to create a project with and without access, saw suitable error.
Reviewers: epriestley, #blessed_reviewers
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7279
Summary:
Ref T603. This isn't remotely usable yet, but I wanted to get any feedback before I build it out anymore.
I think this is a reasonable interface for defining custom policies? It's basically similar to Herald, although it's a bit simpler.
I imagine users will rarely interact with this, but this will service the high end of policy complexity (and allow the definition of things like "is member of LDAP group" or whatever).
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, asherkin
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7217
Summary:
Ref T603. Allows the Differential view policy to be configured with a default.
I've omitted "edit" because I want to wait and see how comment/comment-action policies work out. I could imagine locking "edit" down to only the owner at some point, and providing a wider "interact" capability, or something like that, which would cover accept/reject/commandeer. Users in this group could still edit indirectly by commandeering first.
Test Plan: Created new revisions from the CLI and conduit.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7269
Summary:
Ref T603. In thinking about this, I think I went mad with power in creating this capability. I can't imagine any reason to give users access to Herald but not let them create rules.
We can restore this later if some install comes up with a good reason to have it, but in the interest of keeping policies as simple as possible, I think we're better off without it. In particular, if you don't want a group of users creating rules, just lock them out of the application entirely.
The "Manage Global Rules" capability is still around, I think that one's super good.
Test Plan: Edited Herald policies, created a rule.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7268
Summary: Ref T603. Allow global default policies to be configured for tasks.
Test Plan:
- Created task via web UI.
- Created task via Conduit.
- Created task via email.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7267
Summary: Ref T603. Use the new hotness.
Test Plan: Edited Herald in Applications, tried to create rules / global rules without capabilities, got reasonable error messages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7263
Summary:
Ref T603. I want to let applications define new capabilities (like "can manage global rules" in Herald) and get full support for them, including reasonable error strings in the UI.
Currently, this is difficult for a couple of reasons. Partly this is just a code organization issue, which is easy to fix. The bigger thing is that we have a bunch of strings which depend on both the policy and capability, like: "You must be an administrator to view this object." "Administrator" is the policy, and "view" is the capability.
That means every new capability has to add a string for each policy, and every new policy (should we introduce any) needs to add a string for each capability. And we can't do any piecemeal "You must be a {$role} to {$action} this object" becuase it's impossible to translate.
Instead, make all the strings depend on //only// the policy, //only// the capability, or //only// the object type. This makes the dialogs read a little more strangely, but I think it's still pretty easy to understand, and it makes adding new stuff way way easier.
Also provide more context, and more useful exception messages.
Test Plan:
- See screenshots.
- Also triggered a policy exception and verified it was dramatically more useful than it used to be.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7260
