From cce6d06fa5ec800353747551f27f6d117dafde75 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 26 Mar 2015 11:11:33 -0700
Subject: [PATCH] Move abandoned revisions to "needs review" when updated

Summary:
Fixes T7602. This is similar to the existing behavior for "changes planned" and "needs revision" revisions.

Also fix the "Update Diff" workflow so it correctly selects closed revisions as attachable.

Test Plan: Updated an abandoned revision, saw it change to "Needs Review".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7602

Differential Revision: https://secure.phabricator.com/D12167
---
 .../DifferentialDiffViewController.php        | 54 +++++++++++++++----
 .../editor/DifferentialTransactionEditor.php  | 13 +++--
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php
index 419e6d9ce9..ebbee90e99 100644
--- a/src/applications/differential/controller/DifferentialDiffViewController.php
+++ b/src/applications/differential/controller/DifferentialDiffViewController.php
@@ -38,19 +38,10 @@ final class DifferentialDiffViewController extends DifferentialController {
       pht('Create a new Revision...'));
     $select[] = hsprintf('</optgroup>');
 
-    $revisions = id(new DifferentialRevisionQuery())
-      ->setViewer($viewer)
-      ->withAuthors(array($viewer->getPHID()))
-      ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
-      ->requireCapabilities(
-        array(
-          PhabricatorPolicyCapability::CAN_VIEW,
-          PhabricatorPolicyCapability::CAN_EDIT,
-        ))
-      ->execute();
-
     $selected_id = $request->getInt('revisionID');
 
+    $revisions = $this->loadSelectableRevisions($viewer, $selected_id);
+
     if ($revisions) {
       $select[] = hsprintf(
         '<optgroup label="%s">',
@@ -153,4 +144,45 @@ final class DifferentialDiffViewController extends DifferentialController {
       ));
   }
 
+  private function loadSelectableRevisions(
+    PhabricatorUser $viewer,
+    $selected_id) {
+
+    $revisions = id(new DifferentialRevisionQuery())
+      ->setViewer($viewer)
+      ->withAuthors(array($viewer->getPHID()))
+      ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->execute();
+    $revisions = mpull($revisions, null, 'getID');
+
+    // If a specific revision is selected (for example, because the user is
+    // following the "Update Diff" workflow), but not present in the dropdown,
+    // try to add it to the dropdown even if it is closed. This allows the
+    // workflow to be used to update abandoned revisions.
+
+    if ($selected_id) {
+      if (empty($revisions[$selected_id])) {
+        $selected = id(new DifferentialRevisionQuery())
+          ->setViewer($viewer)
+          ->withAuthors(array($viewer->getPHID()))
+          ->withIDs(array($selected_id))
+          ->requireCapabilities(
+            array(
+              PhabricatorPolicyCapability::CAN_VIEW,
+              PhabricatorPolicyCapability::CAN_EDIT,
+            ))
+          ->execute();
+        $revisions = mpull($selected, null, 'getID') + $revisions;
+      }
+    }
+
+    return $revisions;
+  }
+
+
 }
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index c01862b047..fbac993566 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -187,6 +187,7 @@ final class DifferentialTransactionEditor
     $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
     $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
     $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
+    $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
 
     switch ($xaction->getTransactionType()) {
       case PhabricatorTransactions::TYPE_VIEW_POLICY:
@@ -202,10 +203,14 @@ final class DifferentialTransactionEditor
       case PhabricatorTransactions::TYPE_EDGE:
         return;
       case DifferentialTransaction::TYPE_UPDATE:
-        if (!$this->getIsCloseByCommit() &&
-            (($object->getStatus() == $status_revision) ||
-             ($object->getStatus() == $status_plan))) {
-          $object->setStatus($status_review);
+        if (!$this->getIsCloseByCommit()) {
+          switch ($object->getStatus()) {
+            case $status_revision:
+            case $status_plan:
+            case $status_abandoned:
+              $object->setStatus($status_review);
+              break;
+          }
         }
 
         $diff = $this->requireDiff($xaction->getNewValue());