@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

+86
+4
src/__phutil_library_map__.php
··· 474 474 'DiffusionCommitRef' => 'applications/diffusion/data/DiffusionCommitRef.php', 475 475 'DiffusionCommitRemarkupRule' => 'applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php', 476 476 'DiffusionCommitRemarkupRuleTestCase' => 'applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php', 477 + 'DiffusionCommitRevertedByCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php', 478 + 'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php', 477 479 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', 478 480 'DiffusionConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionConduitAPIMethod.php', 479 481 'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php', ··· 3536 3538 'DiffusionCommitRef' => 'Phobject', 3537 3539 'DiffusionCommitRemarkupRule' => 'PhabricatorObjectRemarkupRule', 3538 3540 'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase', 3541 + 'DiffusionCommitRevertedByCommitEdgeType' => 'PhabricatorEdgeType', 3542 + 'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType', 3539 3543 'DiffusionCommitTagsController' => 'DiffusionController', 3540 3544 'DiffusionConduitAPIMethod' => 'ConduitAPIMethod', 3541 3545 'DiffusionController' => 'PhabricatorController',
+18
src/applications/diffusion/controller/DiffusionCommitController.php
··· 419 419 ->withEdgeTypes(array( 420 420 DiffusionCommitHasTaskEdgeType::EDGECONST, 421 421 DiffusionCommitHasRevisionEdgeType::EDGECONST, 422 + DiffusionCommitRevertsCommitEdgeType::EDGECONST, 423 + DiffusionCommitRevertedByCommitEdgeType::EDGECONST, 422 424 )); 423 425 424 426 $edges = $edge_query->execute(); ··· 427 429 $edges[$commit_phid][DiffusionCommitHasTaskEdgeType::EDGECONST]); 428 430 $revision_phid = key( 429 431 $edges[$commit_phid][DiffusionCommitHasRevisionEdgeType::EDGECONST]); 432 + 433 + $reverts_phids = array_keys( 434 + $edges[$commit_phid][DiffusionCommitRevertsCommitEdgeType::EDGECONST]); 435 + $reverted_by_phids = array_keys( 436 + $edges[$commit_phid][DiffusionCommitRevertedByCommitEdgeType::EDGECONST]); 430 437 431 438 $phids = $edge_query->getDestinationPHIDs(array($commit_phid)); 432 439 ··· 613 620 $refs = $this->buildRefs($drequest); 614 621 if ($refs) { 615 622 $props['References'] = $refs; 623 + } 624 + 625 + if ($reverts_phids) { 626 + $this->loadHandles($reverts_phids); 627 + $props[pht('Reverts')] = $this->renderHandlesForPHIDs($reverts_phids); 628 + } 629 + 630 + if ($reverted_by_phids) { 631 + $this->loadHandles($reverted_by_phids); 632 + $props[pht('Reverted By')] = $this->renderHandlesForPHIDs( 633 + $reverted_by_phids); 616 634 } 617 635 618 636 if ($task_phids) {
+16
src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php
··· 1 + <?php 2 + 3 + final class DiffusionCommitRevertedByCommitEdgeType 4 + extends PhabricatorEdgeType { 5 + 6 + const EDGECONST = 56; 7 + 8 + public function getInverseEdgeConstant() { 9 + return DiffusionCommitRevertsCommitEdgeType::EDGECONST; 10 + } 11 + 12 + public function shouldWriteInverseTransactions() { 13 + return true; 14 + } 15 + 16 + }
+19
src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php
··· 1 + <?php 2 + 3 + final class DiffusionCommitRevertsCommitEdgeType extends PhabricatorEdgeType { 4 + 5 + const EDGECONST = 55; 6 + 7 + public function getInverseEdgeConstant() { 8 + return DiffusionCommitRevertedByCommitEdgeType::EDGECONST; 9 + } 10 + 11 + public function shouldWriteInverseTransactions() { 12 + return true; 13 + } 14 + 15 + public function shouldPreventCycles() { 16 + return true; 17 + } 18 + 19 + }
+29
src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
··· 102 102 103 103 $conn_w = id(new DifferentialRevision())->establishConnection('w'); 104 104 105 + $reverts_refs = id(new DifferentialCustomFieldRevertsParser()) 106 + ->parseCorpus($message); 107 + $reverts = array_mergev(ipull($reverts_refs, 'monograms')); 108 + 109 + if ($reverts) { 110 + $reverted_commits = id(new DiffusionCommitQuery()) 111 + ->setViewer($actor) 112 + ->withIdentifiers($reverts) 113 + ->execute(); 114 + $reverted_commit_phids = mpull($reverted_commits, 'getPHID', 'getPHID'); 115 + 116 + // NOTE: Skip any write attempts if a user cleverly implies a commit 117 + // reverts itself. 118 + unset($reverted_commit_phids[$commit->getPHID()]); 119 + 120 + $editor = new PhabricatorEdgeEditor(); 121 + foreach ($reverted_commit_phids as $commit_phid) { 122 + try { 123 + $editor->addEdge( 124 + $commit->getPHID(), 125 + DiffusionCommitRevertsCommitEdgeType::EDGECONST, 126 + $commit_phid); 127 + } catch (PhabricatorEdgeCycleException $ex) { 128 + continue; 129 + } 130 + } 131 + $editor->save(); 132 + } 133 + 105 134 // NOTE: The `differential_commit` table has a unique ID on `commitPHID`, 106 135 // preventing more than one revision from being associated with a commit. 107 136 // Generally this is good and desirable, but with the advent of hash