From 6ecdadb76a955854875fa25515fba266bb34cfa9 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 1 Nov 2017 10:54:23 -0700
Subject: [PATCH] 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
---
 ...fferentialRevisionRequiredActionResultBucket.php | 10 +++++++++-
 .../query/DifferentialRevisionResultBucket.php      | 13 ++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
index f3971f8571..5f5f4008db 100644
--- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
@@ -123,6 +123,14 @@ 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 in the "Should Review" bucket.
+
+      // If we don't do this, they end up in "Waiting on Other Reviewers",
+      // even if there are no other reviewers.
+      DifferentialReviewerStatus::STATUS_ACCEPTED,
     );
     $reviewing = array_fuse($reviewing);
 
@@ -130,7 +138,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 54705649eb..a0769ac349 100644
--- a/src/applications/differential/query/DifferentialRevisionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php
@@ -53,7 +53,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();
@@ -66,6 +67,16 @@ abstract class DifferentialRevisionResultBucket
         continue;
       }
 
+      if ($current !== null) {
+        if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) {
+          $diff_phid = $revision->getActiveDiffPHID();
+          $is_current = $reviewer->isAccepted($diff_phid);
+          if ($is_current !== $current) {
+            continue;
+          }
+        }
+      }
+
       return true;
     }