From 85a36367474370e41024f1294575709a7757f189 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Mon, 5 Jan 2015 06:52:38 +1100 Subject: [PATCH] Write edges for commit reverts 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 --- src/__phutil_library_map__.php | 4 +++ .../controller/DiffusionCommitController.php | 18 ++++++++++++ ...iffusionCommitRevertedByCommitEdgeType.php | 16 ++++++++++ .../DiffusionCommitRevertsCommitEdgeType.php | 19 ++++++++++++ ...torRepositoryCommitMessageParserWorker.php | 29 +++++++++++++++++++ 5 files changed, 86 insertions(+) create mode 100644 src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php create mode 100644 src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0ead3d5515..e50ece14c0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -474,6 +474,8 @@ phutil_register_library_map(array( 'DiffusionCommitRef' => 'applications/diffusion/data/DiffusionCommitRef.php', 'DiffusionCommitRemarkupRule' => 'applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php', 'DiffusionCommitRemarkupRuleTestCase' => 'applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php', + 'DiffusionCommitRevertedByCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php', + 'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php', 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', 'DiffusionConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionConduitAPIMethod.php', 'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php', @@ -3536,6 +3538,8 @@ phutil_register_library_map(array( 'DiffusionCommitRef' => 'Phobject', 'DiffusionCommitRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase', + 'DiffusionCommitRevertedByCommitEdgeType' => 'PhabricatorEdgeType', + 'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType', 'DiffusionCommitTagsController' => 'DiffusionController', 'DiffusionConduitAPIMethod' => 'ConduitAPIMethod', 'DiffusionController' => 'PhabricatorController', diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index f953adb0f2..92fb29e013 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -419,6 +419,8 @@ final class DiffusionCommitController extends DiffusionController { ->withEdgeTypes(array( DiffusionCommitHasTaskEdgeType::EDGECONST, DiffusionCommitHasRevisionEdgeType::EDGECONST, + DiffusionCommitRevertsCommitEdgeType::EDGECONST, + DiffusionCommitRevertedByCommitEdgeType::EDGECONST, )); $edges = $edge_query->execute(); @@ -428,6 +430,11 @@ final class DiffusionCommitController extends DiffusionController { $revision_phid = key( $edges[$commit_phid][DiffusionCommitHasRevisionEdgeType::EDGECONST]); + $reverts_phids = array_keys( + $edges[$commit_phid][DiffusionCommitRevertsCommitEdgeType::EDGECONST]); + $reverted_by_phids = array_keys( + $edges[$commit_phid][DiffusionCommitRevertedByCommitEdgeType::EDGECONST]); + $phids = $edge_query->getDestinationPHIDs(array($commit_phid)); if ($data->getCommitDetail('authorPHID')) { @@ -615,6 +622,17 @@ final class DiffusionCommitController extends DiffusionController { $props['References'] = $refs; } + if ($reverts_phids) { + $this->loadHandles($reverts_phids); + $props[pht('Reverts')] = $this->renderHandlesForPHIDs($reverts_phids); + } + + if ($reverted_by_phids) { + $this->loadHandles($reverted_by_phids); + $props[pht('Reverted By')] = $this->renderHandlesForPHIDs( + $reverted_by_phids); + } + if ($task_phids) { $task_list = array(); foreach ($task_phids as $phid) { diff --git a/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php b/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php new file mode 100644 index 0000000000..c0502478bf --- /dev/null +++ b/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php @@ -0,0 +1,16 @@ +establishConnection('w'); + $reverts_refs = id(new DifferentialCustomFieldRevertsParser()) + ->parseCorpus($message); + $reverts = array_mergev(ipull($reverts_refs, 'monograms')); + + if ($reverts) { + $reverted_commits = id(new DiffusionCommitQuery()) + ->setViewer($actor) + ->withIdentifiers($reverts) + ->execute(); + $reverted_commit_phids = mpull($reverted_commits, 'getPHID', 'getPHID'); + + // NOTE: Skip any write attempts if a user cleverly implies a commit + // reverts itself. + unset($reverted_commit_phids[$commit->getPHID()]); + + $editor = new PhabricatorEdgeEditor(); + foreach ($reverted_commit_phids as $commit_phid) { + try { + $editor->addEdge( + $commit->getPHID(), + DiffusionCommitRevertsCommitEdgeType::EDGECONST, + $commit_phid); + } catch (PhabricatorEdgeCycleException $ex) { + continue; + } + } + $editor->save(); + } + // NOTE: The `differential_commit` table has a unique ID on `commitPHID`, // preventing more than one revision from being associated with a commit. // Generally this is good and desirable, but with the advent of hash