Skip to content

Commit 6ecdadb

Browse files
author
epriestley
committed
After "Request Review", move revisions with voided "Accepts" into "Ready to Review", not "Waiting on Other Reviewers"
Summary: Depends on D18756. Fixes T12539. See PHI190. Currently, when this occurs: - Alice accepts. - Bailey requests review. - Alice views her dashboard. ...the revision appears in "Waiting on Other Reviewers" (regardless of whether other reviewers actually exist or not). Instead, ignore these voided/non-current accepts and let the revisions appear in "Ready to Review", which is more natural. Test Plan: Went through the steps above. On `master`, saw revision in "Waiting on Other Reviewers". After patch, saw it in "Ready to Review". Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T12539 Differential Revision: https://secure.phabricator.com/D18757
1 parent 6d36eb9 commit 6ecdadb

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,22 @@ private function filterShouldReview(array $phids) {
123123
$reviewing = array(
124124
DifferentialReviewerStatus::STATUS_ADDED,
125125
DifferentialReviewerStatus::STATUS_COMMENTED,
126+
127+
// If an author has used "Request Review" to put an accepted revision
128+
// back into the "Needs Review" state, include "Accepted" reviewers
129+
// whose reviews have been voided in the "Should Review" bucket.
130+
131+
// If we don't do this, they end up in "Waiting on Other Reviewers",
132+
// even if there are no other reviewers.
133+
DifferentialReviewerStatus::STATUS_ACCEPTED,
126134
);
127135
$reviewing = array_fuse($reviewing);
128136

129137
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
130138

131139
$results = array();
132140
foreach ($objects as $key => $object) {
133-
if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) {
141+
if (!$this->hasReviewersWithStatus($object, $phids, $reviewing, false)) {
134142
continue;
135143
}
136144

src/applications/differential/query/DifferentialRevisionResultBucket.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ protected function getRevisionsNotAuthored(array $objects, array $phids) {
5353
protected function hasReviewersWithStatus(
5454
DifferentialRevision $revision,
5555
array $phids,
56-
array $statuses) {
56+
array $statuses,
57+
$current = null) {
5758

5859
foreach ($revision->getReviewers() as $reviewer) {
5960
$reviewer_phid = $reviewer->getReviewerPHID();
@@ -66,6 +67,16 @@ protected function hasReviewersWithStatus(
6667
continue;
6768
}
6869

70+
if ($current !== null) {
71+
if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) {
72+
$diff_phid = $revision->getActiveDiffPHID();
73+
$is_current = $reviewer->isAccepted($diff_phid);
74+
if ($is_current !== $current) {
75+
continue;
76+
}
77+
}
78+
}
79+
6980
return true;
7081
}
7182

0 commit comments

Comments
 (0)