See D17585, might just be a transient state bug related to downgrading / voiding stuff. This is mostly so I don't forget about it.
See also PHI190.
Description
Description
Revisions and Commits
Revisions and Commits
| Restricted Differential Revision | Restricted Diffusion Commit | |
| Restricted Differential Revision | Restricted Diffusion Commit |
Event Timeline
Comment Actions
This is a legitimate bug but I think it's very long-standing and fixing it requires doing a lot of extra data loading to get the current diff PHID. Until someone else notices, I'd rather wait for diff PHIDs to be denormalized onto revisions.
Fix is something like this, but needs a needActiveDiffs(...):
diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php index 6d83d97a47..c70377874e 100644 --- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -115,6 +115,11 @@ final class DifferentialRevisionRequiredActionResultBucket $reviewing = array( DifferentialReviewerStatus::STATUS_ADDED, DifferentialReviewerStatus::STATUS_COMMENTED, + + // If an author has used "Request Review" to put an accepted revision + // back into the "needs review" state, include "Accepted" reviewers + // whose reviews have been voided. + DifferentialReviewerStatus::STATUS_ACCEPTED, ); $reviewing = array_fuse($reviewing); @@ -122,7 +127,7 @@ final class DifferentialRevisionRequiredActionResultBucket $results = array(); foreach ($objects as $key => $object) { - if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) { + if (!$this->hasReviewersWithStatus($object, $phids, $reviewing, false)) { continue; } diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php index 762e2d97f4..34a17aaaf6 100644 --- a/src/applications/differential/query/DifferentialRevisionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php @@ -54,7 +54,8 @@ abstract class DifferentialRevisionResultBucket protected function hasReviewersWithStatus( DifferentialRevision $revision, array $phids, - array $statuses) { + array $statuses, + $current = null) { foreach ($revision->getReviewers() as $reviewer) { $reviewer_phid = $reviewer->getReviewerPHID(); @@ -67,6 +68,17 @@ abstract class DifferentialRevisionResultBucket continue; } + if ($current !== null) { + if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) { + $diff_phid = $revision->getActiveDiff()->getPHID(); + $is_current = $reviewer->isAccepted($diff_phid); + } + + if ($is_current !== $current) { + continue; + } + } + return true; }