From d0a2e3c54f9ae254c7158965480c892d1c03d70c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Feb 2018 09:39:27 -0800 Subject: [PATCH] Fix an issue where some Differential edit pathways may not have reviewers attached Summary: Depends on D19021. Ref T13053. When you "Subscribe", or make some other types of edits, we don't necessarily have reviewer data, but may now need it to do the new recipient list logic. I don't have a totally clean way to deal with this in the general case in mind, but just load it for now so that things don't fatal. Test Plan: Subscribed to a revision with the "Subscribe" action. Reviewers: amckinley Maniphest Tasks: T13053 Differential Revision: https://secure.phabricator.com/D19022 --- .../editor/DifferentialTransactionEditor.php | 25 +++++++++++++++++++ .../storage/DifferentialRevision.php | 4 +++ 2 files changed, 29 insertions(+) diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index cc29d69fe0..063df6c602 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -632,6 +632,8 @@ final class DifferentialTransactionEditor } protected function getMailTo(PhabricatorLiskDAO $object) { + $this->requireReviewers($object); + $phids = array(); $phids[] = $object->getAuthorPHID(); foreach ($object->getReviewers() as $reviewer) { @@ -645,6 +647,8 @@ final class DifferentialTransactionEditor } protected function newMailUnexpandablePHIDs(PhabricatorLiskDAO $object) { + $this->requireReviewers($object); + $phids = array(); foreach ($object->getReviewers() as $reviewer) { @@ -1737,4 +1741,25 @@ final class DifferentialTransactionEditor } } + private function requireReviewers(DifferentialRevision $revision) { + if ($revision->hasAttachedReviewers()) { + return; + } + + $with_reviewers = id(new DifferentialRevisionQuery()) + ->setViewer($this->getActor()) + ->needReviewers(true) + ->withPHIDs(array($revision->getPHID())) + ->executeOne(); + if (!$with_reviewers) { + throw new Exception( + pht( + 'Failed to reload revision ("%s").', + $revision->getPHID())); + } + + $revision->attachReviewers($with_reviewers->getReviewers()); + } + + } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 4591315207..e8fdf7e514 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -583,6 +583,10 @@ final class DifferentialRevision extends DifferentialDAO return $this; } + public function hasAttachedReviewers() { + return ($this->reviewerStatus !== self::ATTACHABLE); + } + public function getReviewerPHIDs() { $reviewers = $this->getReviewers(); return mpull($reviewers, 'getReviewerPHID');