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

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;
+  }
+
+
 }

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() &&
+        if (!$this->getIsCloseByCommit()) {
-            (($object->getStatus() == $status_revision) ||
+          switch ($object->getStatus()) {
-             ($object->getStatus() == $status_plan))) {
+            case $status_revision:
+            case $status_plan:
+            case $status_abandoned:
               $object->setStatus($status_review);
+              break;
+          }
         }
 
         $diff = $this->requireDiff($xaction->getNewValue());