diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php index d37754227e..6d83d97a47 100644 --- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -29,6 +29,14 @@ final class DifferentialRevisionRequiredActionResultBucket } $phids = array_fuse($phids); + // Before continuing, throw away any revisions which responsible users + // have explicitly resigned from. + + // The goal is to allow users to resign from revisions they don't want to + // review to get these revisions off their dashboard, even if there are + // other project or package reviewers which they have authority over. + $this->filterResigned($phids); + $groups = array(); $groups[] = $this->newGroup() @@ -229,4 +237,25 @@ final class DifferentialRevisionRequiredActionResultBucket return $results; } + private function filterResigned(array $phids) { + $resigned = array( + DifferentialReviewerStatus::STATUS_RESIGNED, + ); + $resigned = array_fuse($resigned); + + $objects = $this->getRevisionsNotAuthored($this->objects, $phids); + + $results = array(); + foreach ($objects as $key => $object) { + if (!$this->hasReviewersWithStatus($object, $phids, $resigned)) { + continue; + } + + $results[$key] = $object; + unset($this->objects[$key]); + } + + return $results; + } + } diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php index 08c9707225..836022c882 100644 --- a/src/applications/differential/storage/DifferentialReviewer.php +++ b/src/applications/differential/storage/DifferentialReviewer.php @@ -42,4 +42,9 @@ final class DifferentialReviewer return $this->assertAttachedKey($this->authority, $cache_fragment); } + public function isResigned() { + $status_resigned = DifferentialReviewerStatus::STATUS_RESIGNED; + return ($this->getReviewerStatus() == $status_resigned); + } + } diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php index 54b7e35257..291f859d08 100644 --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -25,9 +25,23 @@ final class DifferentialReviewersView extends AphrontView { public function render() { $viewer = $this->getUser(); + $reviewers = $this->reviewers; $view = new PHUIStatusListView(); - foreach ($this->reviewers as $reviewer) { + + // Move resigned reviewers to the bottom. + $head = array(); + $tail = array(); + foreach ($reviewers as $key => $reviewer) { + if ($reviewer->isResigned()) { + $tail[$key] = $reviewer; + } else { + $head[$key] = $reviewer; + } + } + + $reviewers = $head + $tail; + foreach ($reviewers as $reviewer) { $phid = $reviewer->getReviewerPHID(); $handle = $this->handles[$phid]; @@ -98,6 +112,13 @@ final class DifferentialReviewersView extends AphrontView { pht('Blocking Review')); break; + case DifferentialReviewerStatus::STATUS_RESIGNED: + $item->setIcon( + 'fa-times', + 'grey', + pht('Resigned')); + break; + default: $item->setIcon( PHUIStatusItemView::ICON_QUESTION, diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php index c393ee51c5..42f644e8d1 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php @@ -178,16 +178,10 @@ abstract class DifferentialRevisionReviewTransaction $reviewer->setLastActionDiffPHID($diff_phid); } - if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) { - if ($reviewer->getID()) { - $reviewer->delete(); - } - } else { - try { - $reviewer->save(); - } catch (AphrontDuplicateKeyQueryException $ex) { - // At least for now, just ignore it if we lost a race. - } + try { + $reviewer->save(); + } catch (AphrontDuplicateKeyQueryException $ex) { + // At least for now, just ignore it if we lost a race. } } }