Summary: Select a similar or better FontAwesome icon to represent each application
Test Plan: Visual inspection
Reviewers: epriestley, btrahan
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11489
Summary:
Ref T2783. I think this served two purposes:
- Improving performance in cases where we "know" a repository is local.
- Preventing loops.
It is now obsolete:
- After D11476, refs can almost always resolve on a fast path.
- As T2783 moves forward, we can usually no longer know when a repository is local without actually looking it up -- almost everything is allowed to run anywhere.
- The cluster behavior in D11475 now prevents loops.
Test Plan: `grep`, browsed around. This didn't really do much of anything anymore.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11477
Summary:
Ref T2783. With service-oriented calls, we take a larger performacne hit than necessary resolving refs.
Instead of resolving refs over the wire, try to resolve them from the database first. This can resolve almost all refs (commit hashes, branch and tag names).
This can't resolve weird refs like `master~50`, and obviously can't resolve invalid refs. In those cases we'll go back to the old logic, call `diffusion.resolverefs`, and end up with the right result.
Test Plan:
- Browsed repositories in Diffusion.
- Verified that service repositories no longer make unnecessary `diffusion.resolverefs` calls for common refs (branch names, commit hashes).
- Resolved refs like `master~50`, saw call to underlying VCS and correct result.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11476
Summary:
Fixes T7020. When an external user makes a Conduit request to Diffusion but the repository isn't hosted locally, we need to proxy it.
This also adds a guard layer to prevent requests from getting infinitely proxied inside the cluster.
In "trivial" configurations (where the repository is a service repository, but the service is on the local device) I'm making us always proxy anyway. This basically makes it reasonable to test this stuff (otherwise you'd have to set up two different installs) and this configuration doesn't make much sense in real life (if you're using multiple machines, making one a dedicating daemons+repo box is almost certainly the most reasonable configuration, even for a cluster size of 2).
Test Plan:
- With a service-hosted repository, made Diffusion conduit calls and browsed the UI. Verified requests got proxied once, then resovled.
- With a non-service repository, made Diffusion conduit calls and browsed UI. Verified requests were handled in-process immediately.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7020
Differential Revision: https://secure.phabricator.com/D11475
Summary: Ref T7020. I need this elsewhere, and it's relatively internal anyway.
Test Plan: Browsed around my local, cluster-configured install and saw everything working fine.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7020
Differential Revision: https://secure.phabricator.com/D11474
Summary: Fixes T7021. When I moved around all the timeline stuff I guess I didn't find this "corner" case, which is wildly common in the post-commit review workflow that we don't use.
Test Plan: pre-patch I could reproduce the issue and post patch I could not. The reproduction case is to have a commit with inline comments and then enough subsequent comments to have a "show older" UI. clicking "show older" now works!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7021
Differential Revision: https://secure.phabricator.com/D11479
Summary: Fixes T7011. Recent refactoring here caused us to begin ignoring URI parameters like `commit`. Most controllers take parameters as a `dblob`, which was still parsed properly.
Test Plan:
- Editing different commits actually edits the desired commits.
- Browsed around some `dblob` pages and verified they still work properly.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7011
Differential Revision: https://secure.phabricator.com/D11473
Summary: Ref T6822.
Test Plan: `grep`. This method is only called from within `PhutilArgumentWorkflow::__construct`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11415
Summary: Third time lucky... the filename should match the class name now.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11362
Summary: Fixes T5646. Makes diffusion a much better user experience. Users now see a 404 exception page when they have a bad URI. Previously, they saw a developer-facing raw exception.
Test Plan: played around in diffusion a bunch. most of these changes were fairly mechanical at the end of the day.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5646
Differential Revision: https://secure.phabricator.com/D11299
Summary: Ref T6822.
Test Plan: Visual inspection. These methods are only called from within `PhabricatorController` subclasses.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11241
Summary:
Fixes T5966. Accomplishes a few things
- see title
- adds a force-autoclose flag and the plumbing for it
- removes references to some HarborMaster thing that used to key off commits and seems long dead, but forgotten :/
Test Plan:
ran a few commands. These first three had great success:
`./repository reparse --all FIRSTREPO --message --change --herald --owners`
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date yesterday`
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date yesterday --force-autoclose`
...and these next two showed me some errors as expected:
`./repository reparse --all FIRSTREPO --message --change --herald --owners --min-date garbagedata`
`./repository reparse --all GARBAGEREPO --message --change --herald --owners`
Also, made a diff in a repository with autoclose disabled and commited the diff. Later, reparse the diff with force-autoclose. Verified the diff closed and that the reason "why" had the proper message text.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: joshuaspence, epriestley, Korvin
Maniphest Tasks: T5966
Differential Revision: https://secure.phabricator.com/D10492
Summary: Fixes T6880. If matching commits have no visible/loadable repository, we shouldn't keep going forward in the loop.
Test Plan: Havne't built a repro locally yet so not 100% sure this fixes it.
Reviewers: btrahan, mbishopim3, fabe
Reviewed By: mbishopim3, fabe
Subscribers: mbishopim3, epriestley
Maniphest Tasks: T6880
Differential Revision: https://secure.phabricator.com/D11251
Summary: Use `PhabricatorAuditEditor` instead of `PhabricatorEdgeEditor` when writing reverts edges. This ensures that a transaction is created in addition to the edge.
Test Plan: Reverted a commit and pushed to remote. Saw a row created in `phabricator_audit.audit_transaction_comment`. Interestingly, I can't actually see the transaction at http://phabricator.local/r${CALLSIGN}${REVERTED_COMMIT_HASH}.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11212
Summary:
Ref T1751. When a commit reverts another commit:
- Add an edge linking them;
- Show the edge in Diffusion.
Next steps are:
- If the reverted commit is associated with a Differential revision, leave a comment;
- Also leave a comment on the commit (no API yet);
- Also trigger an audit by the original commit's author.
Test Plan: Used `scripts/repository/reparse.php --message ...` to parse commits with revert language. Verified they appear correctly in Diffusion, and update Differential.
Reviewers: btrahan, epriestley
Reviewed By: btrahan, epriestley
Subscribers: Korvin, epriestley, cburroughs, joshuaspence, sascha-egerer, aran
Maniphest Tasks: T4896, T1751
Differential Revision: https://secure.phabricator.com/D5846
Summary:
Ref T2783.
This updates PhabricatorRepositoryManagementLookupUsersWorkflow to use ConduitCall to retrieve information about the commit.
Test Plan:
Ran `bin/repository lookup-users rTESTe9683b64d3283f0b2d355fdbf231bc918b5ac0ab --trace` and saw the information returned (by making a request to `diffusion.querycommits` as the omnipotent user, signed with the device key).
Mucked with `cluster.addresses` and saw requests rejected.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
Subscribers: Krenair, epriestley, Korvin
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D10403
Summary:
Ref T2783. This is basically a more refined version of D10400, which churned a bit on things like SSH key storage, the actual way the signing protocol shook out, etc.
- When Phabricator tries to make an intra-cluster service call as the omnipotent user, sign it with the host's device key.
- Add `bin/almanac register` to say "this host is X device, identified by private key Y". This stores the keypair locally, adds the public key to Almanac, and trusts it.
Net effect is that once a host has been registered, the daemons can make calls to other nodes as the omnipotent user. This is primarily necessary so they can access repository API methods on remote hosts.
Test Plan:
- Ran `bin/almanac register` with various valid and invalid inputs.
- Verified keys get generated/added/stored properly.
- Made a device-signed cluster Conduit call.
- Made a normal old user-signed cluster Conduit call.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11158
Summary:
Ref T4245 Make repo callsigns optional
This is far from done and still very ugly. I'm just submitting it to check if i'm solving this in the right places.
Right now there's three places with duplicate code and building the identifierMap in the CommitQuery is very ugly.
If we only want to support this in the user frontend then i could hack it into the Markup rule itself and not touch the CommitQuery. Even uglier but more limited in scope...
Generally this approach will need a lot of "check this first and then try the other" in a few places.
I could move the Repository queries into a specialised PhabricatorRepositoryQuery method (withCallsignOrID) but i'm not sure about that.
Test Plan:
- phid.lookup works with R1 and rTEST (which is the same repo)
- R1 and rTEST euqally work in remarkup (tested in comments).
- Reviewed the following syntax also all works:
rTEST
rTESTd773137a7cb9
rTEST:d773137a7cb9
R1
R1:d773137a7cb9
d773137a7cb9
{rTEST}
{rTESTd773137a7cb9}
{rTEST:d773137a7cb9}
{R1}
{R1:d773137a7cb9}
{d773137a7cb9}
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D11050
Summary: Modernize Differential edges to subclass `PhabricatorEdgeType`. Largely based on D11045.
Test Plan: From previous experience, these changes are fairly trivial and safe. I poked around a little to make sure things looked reasonably okay.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, Krenair, epriestley
Differential Revision: https://secure.phabricator.com/D11074
Summary:
Ref T2783. This method is kind of goofballs:
- We send a big list of paths to it.
- It sends back a giant blob of HTML.
Instead, just figure out the path we want locally, then fetch the content with `diffusion.filecontentquery`.
Test Plan:
- Viewed main view and directory view, saw a README.
- See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11099
Summary:
Ref T2783. Currently, the repository edit page does some checks agaisnt the local system to look for binaries and files on disk. These checks don't make sense in a cluster environment.
Ideally, we could make a Conduit call to the host (e.g., add something like `diffusion.querysetupstatus`) to do these checks, but since they're pretty basic config things and cluster installs are advanced, it doesn't seem super worthwhile for now.
Test Plan: Saw fewer checks in a cluster repo.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11102
Summary: This is a fake hash of many 0s which ends up being a bad link. Detect the fake hash and don't print a link. Fixes T6826.
Test Plan: looked at push log and no longer saw a many 0 entry for the first old ref.
Reviewers: epriestley, chad
Reviewed By: chad
Subscribers: Korvin, epriestley
Maniphest Tasks: T6826
Differential Revision: https://secure.phabricator.com/D11096
Summary: Ref T5402. This more or less "fixes" it but there's probably some polish to do?
Test Plan:
stopped and started daemons. error logs look good.
ran bin/storage upgrade. noted that `adjust` added the appropriate indices for active and archive task.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5402
Differential Revision: https://secure.phabricator.com/D11044
Summary: Fixes T6790. Turn the old method into "new" (old signature) and "newEphemeral". Deploy "newEphemeral" as many places as possible; basically places we are not in the Differential application *and* have no intentions of ever saving the diff. These callsites are also all places we are just trying to get some changesets at the end of the day.
Test Plan: set differential application policy to 'administrators only'. viewed a commit in diffusion and it worked without any errors! i'm just using my thinkin' noodle on the other code paths.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6790
Differential Revision: https://secure.phabricator.com/D11020
Summary:
Ref T2783. When creating a new repository, test for cluster services. If cluster services are available, allocate on a random open service.
Show the service that repositories are allocated on.
Test Plan: Created a new repository, saw it allocate onto an available cluster service.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11003
Summary: Ref T2783. In Diffusion -> Edit Repository, we currently have a section called "Local" with options about where the repository is stored. The current name is misleading in a cluster environment, where storage may not actually be local. Shortly, this will also have an option for cluster storage. Call this "Storage" instead.
Test Plan: Edited a repository and poked around.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11001
Summary:
Ref T5833. This allows services to be typed, to distinguish between different kinds of services. This makes a few things easier:
- It's easier for clients to select the services they're interested in (see note in T5873 about Phacility). This isn't a full-power solution, but gets is some of the way there.
- It's easier to set appropriate permissions around when modifications to the Phabricator cluster are allowed. These service nodes need to be demarcated as special in some way no matter what (see T6741). This also defines a new policy for users who are permitted to create services.
- It's easier to browse/review/understand services.
- Future diffs will allow ServiceTypes to specify more service structure (for example, default properties) to make it easier to configure services correctly. Instead of a free-for-all, you'll get a useful list of things that consumers of the service expect to read.
The "custom" service type allows unstructured/freeform services to be created.
Test Plan:
- Created a new service (and hit error cases).
- Edited an existing service.
- Saw service types on list and detail views.
- Poked around new permission stuff.
- Ran `almanac.queryservices` with service class specification.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10995
Summary:
Ref T5955. Ref T2783.
- Removes the "temporary" type. I was going to use this for T3628 but it started taking more time than I wanted to spend on it.
- Add a "cluster" type, which is an internal-only token type used within a cluster. This token value is never shown to the user.
- Automatically generate, use, and cycle cluster tokens.
Test Plan:
- Diffusion (mostly) works with a repository configured to use a remote service.
- Saw cluster tokens generate; terminated a cluster token and saw it regenerate.
- Viewed cluster token in settings panel and saw nice explanatory text instead, as expected (we might just hide these eventually).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783, T5955
Differential Revision: https://secure.phabricator.com/D10990
Summary:
Ref T2783. This is primarily exploratory and just figuring out what we're blocked on:
- Allow a Repository to be bound to a Service. The Service may eventually define multiple read/write nodes, etc.
- There's no UI to do this binding yet, you have to touch the database manually.
- If a repository is bound to a Service, effect Conduit calls via calls to the remote service instead of executing them in-process.
- These don't actually work yet since there's no authentication (see T5955).
Test Plan:
- Made a nice Service with a nice Binding to a nice Interface on a nice Device.
- Force-associated a repository with the service using a raw MySQL query.
- Saw Phabricator try to make a remote call to the service (on localhost) and fail because of missing auth stuff.
- Also ran `almanac.queryservices`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D10982
Summary:
Ref T4712. Specifically...
- Differential
- needed getApplicationTransactionViewObject() implemented
- Audit
- needed getApplicationTransactionViewObject() implemented
- Repository
- one object needed PhabricatorApplicationTransactionInterface implemented
- setShouldTerminate(true)
- Ponder
- BONUS BUG FIX - leaving a comment on an answer had a bad redirect URI
- both PonderQuestion and PonderAnswer needed PhabricatorApplicationTransactionInterface implemented
- setShouldTerminate(true) on both "history" controllers
- left a "TODO" on buildAnswers on the question view controller, which is non-standard and should be re-written eventually
- Phortune
- BONUS BUG FIX - fix new user "createNewAccount" code to not fatal
- PhortuneAccount, PhortuneMerchant, and PhortuneCart needed PhabricatorApplicationTransactionInterface implemented
- setShouldTerminate(true) on Account view, merchant view, and cart view controller
- Fund
- Legalpad
- Nuance
- NuanceSource needed PhabricatorApplicationTransactionInterface implemented
- Releeph (this product is kind of a mess...)
- HACKQUEST - had to manually create an arcanist project to even be able to make a "product" and get started...!
- BONUS BUG FIX - make sure to "setName" on product edit
- ReleephProject (should be ReleepProduct...?), ReleephBranch, and ReleepRequest needed PhabricatorApplicationTransactionInterface implemented
- Harbormaster
- HarbormasterBuildable, HarbormasterBuild, HarbormasterBuildPlan, and HarbormasterBuildStep all needed PhabricatorApplicationTransactionInterface implemented
- setShouldTerminate(true) all over the place
Test Plan: foreach application, viewed the timeline(s) and made sure they still rendered
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4712
Differential Revision: https://secure.phabricator.com/D10925
Summary: These have all been modernized.
Test Plan: Browse Diffusion on a narrow screen.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10920
Summary:
Fixes T6619. In `{Xnnn key=value, key=value}` we did not require a separator between the object and the key-value part. This could lead to `{rX11aaa}` being parsed as `{rX11 aaa}`, i.e. a reference to `rX11` with parameter `aaa` set.
Instead, require a space or comma before we'll parse key-value parts of embedded objects.
Test Plan:
Added and executed unit tests.
{F242002}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6619
Differential Revision: https://secure.phabricator.com/D10915
Summary: See <https://phabricator.wikimedia.org/T906>. This behavior is a bug; we should remove the button if the user can't use the application.
Test Plan:
- With Macro uninstalled, did these things verifying the button vanished:
- Sent a user a message.
- Edited a revision.
- Edited repository basic information.
- Edited an initiative.
- Edited a Harbormaster build step.
- Added task comments.
- Edited profile blurb.
- Edited blog description.
- Commented on Pholio mock.
- Uploaded Pholio image.
- Edited Phortune merchant.
- Edited Phriction document.
- Edited Ponder answer.
- Edited Ponder question.
- Edited Slowvote poll.
- Edited a comment.
- Reinstalled Macro and saw button come back.
- Used button to put silly text on a funny picture.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10900
Summary: Fixes T3189. Now if you say #projects in a commit message they will associate nicely with the commit. Also we record transactions about all this project editing fun.
Test Plan: tested migration by associating some projects with commits and verifying they still showed up post migration. tested adding / removing projects by doing so from the UI, noting transactions written nicely as well
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Projects: #projects
Maniphest Tasks: T3189
Differential Revision: https://secure.phabricator.com/D10877
Summary: Fixes T6152, T6237. This introduces a viewPolicy column to the DifferentialDiff, and re-jiggers the DifferentialDiff policy implementation such that things behave as before once associated with a revision, else use the DifferentialDiff policy column value.
Test Plan: made a diff with a non-standard view policy and noted that policy was still selected in the revision step. arc lint.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237, T6152
Differential Revision: https://secure.phabricator.com/D10875
Summary: ...if pertinent environment variables are set that is... Fixes T4151. This is the last piece in making repository creation somewhat easier.
Test Plan: made a new repo and noted that http serving was on r/w and ssh serving was still off, as expected for my environment configuration
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4151
Differential Revision: https://secure.phabricator.com/D10839
Summary: Fixes T6395. Ref T6350. I guess I missed this code spot in prior testing / I definitely didn't run an empty commit through it. Works now though.
Test Plan: made an empty commit and observed stuck importing status and errors in phd log. applied patch and commit successfully imported with no errors. made another empty commit and it imported as well
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6350, T6395
Differential Revision: https://secure.phabricator.com/D10746
Summary: Ref T6345, This adds more consistent color choices to match how Phabricator generally works across Differential/Diffusion per user statuses.
Test Plan: Review a few Audits in my sandbox.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Maniphest Tasks: T6345
Differential Revision: https://secure.phabricator.com/D10726
Summary:
Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:
- Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
- `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
- Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature.
- Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`.
Test Plan:
- Browsed around in general.
- Hit special controllers (redirect, 404).
- Hit AuditList controller (uses new style).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5702
Differential Revision: https://secure.phabricator.com/D10698
Summary:
I am not sure how valuable this is *as is* - I think it needs different explanations for what happened in mercurial or subversion? I do not know what those explanations are.
Made an error in D10485 - the $hashes that were saved is an array of objects, so it ends up turning into garbage via the wonders of serialization and de-serialization. Fix that by explicitly saving the tree hash.
I would like to make this work for the other VCS types we support, add the "undo / nope" button and call it fixed.
Ref T3686.
Test Plan: clicked "explan why" and saw why
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5693, T3686
Differential Revision: https://secure.phabricator.com/D10489
