From 1c4405d8adb970fe29a7a1ecf3100c33bcc39549 Mon Sep 17 00:00:00 2001 From: Zeid Date: Tue, 22 Jul 2025 14:14:12 -0400 Subject: [PATCH] PhabricatorRepositoryCommitPublishWorker: move relevance logic (1963406) - move repository matching check to PhabricatorRepositoryCommitPublishWorker Prior to this change, a commit that matches a revision was still going through code in both PhabricatorRepositoryCommitPublishWorker and DiffusionUpdateObjectAfterCommitWorker that was making changes to the revision other than changin the status. With this change, the revisions will still show a link to the commit, however none of the code in DiffusionUpdateObjectAfterCommitWorker will be triggered. --- ...DiffusionUpdateObjectAfterCommitWorker.php | 43 ++++++------------- ...abricatorRepositoryCommitPublishWorker.php | 19 ++++++++ 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/applications/diffusion/worker/DiffusionUpdateObjectAfterCommitWorker.php b/src/applications/diffusion/worker/DiffusionUpdateObjectAfterCommitWorker.php index 9b33447340..81cbdcaed2 100644 --- a/src/applications/diffusion/worker/DiffusionUpdateObjectAfterCommitWorker.php +++ b/src/applications/diffusion/worker/DiffusionUpdateObjectAfterCommitWorker.php @@ -183,37 +183,20 @@ private function updateRevision( $xactions = array(); - $revisionRepositoryCallsign = $revision->getRepository()->getCallsign(); - $commitRepositoryCallsign = $commit->getRepository()->getCallsign(); - - $mustCloseRevision = false; - if ($revisionRepositoryCallsign === $commitRepositoryCallsign) { - $mustCloseRevision = true; - } else { - $config = PhabricatorEnv::getEnvConfig('diffusion.legacy-repos-mapping'); - if (array_key_exists($revisionRepositoryCallsign, $config)) { - if ($config[$revisionRepositoryCallsign] === $commitRepositoryCallsign) { - $mustCloseRevision = true; - } - } - } + $xactions[] = $this->newEdgeTransaction( + $revision, + $commit, + DifferentialRevisionHasCommitEdgeType::EDGECONST); - if ($mustCloseRevision) { - $xactions[] = $this->newEdgeTransaction( - $revision, - $commit, - DifferentialRevisionHasCommitEdgeType::EDGECONST); - - $match_data = $this->getUpdateProperty('revisionMatchData'); - - $type_close = DifferentialRevisionCloseTransaction::TRANSACTIONTYPE; - $xactions[] = $revision->getApplicationTransactionTemplate() - ->setTransactionType($type_close) - ->setNewValue(true) - ->setMetadataValue('isCommitClose', true) - ->setMetadataValue('revisionMatchData', $match_data) - ->setMetadataValue('commitPHID', $commit->getPHID()); - } + $match_data = $this->getUpdateProperty('revisionMatchData'); + + $type_close = DifferentialRevisionCloseTransaction::TRANSACTIONTYPE; + $xactions[] = $revision->getApplicationTransactionTemplate() + ->setTransactionType($type_close) + ->setNewValue(true) + ->setMetadataValue('isCommitClose', true) + ->setMetadataValue('revisionMatchData', $match_data) + ->setMetadataValue('commitPHID', $commit->getPHID()); $extraction_engine = id(new DifferentialDiffExtractionEngine()) ->setViewer($viewer) diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php index 9d70e3db1c..9af88cb7b9 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php @@ -456,6 +456,25 @@ private function closeRevisions( return; } + $revisionRepositoryCallsign = $revision->getRepository()->getCallsign(); + $commitRepositoryCallsign = $commit->getRepository()->getCallsign(); + + $mustCloseRevision = false; + if ($revisionRepositoryCallsign === $commitRepositoryCallsign) { + $mustCloseRevision = true; + } else { + $config = PhabricatorEnv::getEnvConfig('diffusion.legacy-repos-mapping'); + if (array_key_exists($revisionRepositoryCallsign, $config)) { + if ($config[$revisionRepositoryCallsign] === $commitRepositoryCallsign) { + $mustCloseRevision = true; + } + } + } + + if (!$mustCloseRevision) { + return; + } + // NOTE: This is very old code from when revisions had a single reviewer. // It still powers the "Reviewer (Deprecated)" field in Herald, but should // be removed.