From 415ad784844e5fd05239dcd34b186c7561fc8860 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 28 Mar 2017 06:07:08 -0700
Subject: [PATCH] Remove old code for "Request Review" action from Differential

Summary: Ref T10967. This moves all remaining "request review" pathways (just `differential.createcomment`) to the new code, and removes the old action.

Test Plan: Requested review on a revision, `grep`'d for the action constant.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17567
---
 ...ferentialCreateCommentConduitAPIMethod.php |  2 +
 .../editor/DifferentialTransactionEditor.php  | 53 -------------------
 .../storage/DifferentialTransaction.php       |  2 -
 3 files changed, 2 insertions(+), 55 deletions(-)

diff --git a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
index ee70644537..52736d6f3b 100644
--- a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
@@ -60,6 +60,8 @@ final class DifferentialCreateCommentConduitAPIMethod
       'accept' => DifferentialRevisionAcceptTransaction::TRANSACTIONTYPE,
       'reject' => DifferentialRevisionRejectTransaction::TRANSACTIONTYPE,
       'resign' => DifferentialRevisionResignTransaction::TRANSACTIONTYPE,
+      'request_review' =>
+        DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE,
     );
 
     $action = $request->getValue('action');
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 9b837876fb..e35257ff9b 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -140,8 +140,6 @@ final class DifferentialTransactionEditor
             return ($object->getStatus() == $status_closed);
           case DifferentialAction::ACTION_RETHINK:
             return ($object->getStatus() != $status_plan);
-          case DifferentialAction::ACTION_REQUEST:
-            return ($object->getStatus() != $status_review);
           case DifferentialAction::ACTION_CLAIM:
             return ($actor_phid != $object->getAuthorPHID());
         }
@@ -200,9 +198,6 @@ final class DifferentialTransactionEditor
           case DifferentialAction::ACTION_REOPEN:
             $object->setStatus($status_review);
             return;
-          case DifferentialAction::ACTION_REQUEST:
-            $object->setStatus($status_review);
-            return;
           case DifferentialAction::ACTION_CLOSE:
             $old_status = $object->getStatus();
             $object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED);
@@ -294,19 +289,6 @@ final class DifferentialTransactionEditor
             $downgrade_accepts = true;
           }
           break;
-
-        // TODO: Remove this, obsoleted by ModularTransactions above.
-        case DifferentialTransaction::TYPE_ACTION:
-          switch ($xaction->getNewValue()) {
-            case DifferentialAction::ACTION_REQUEST:
-              $downgrade_rejects = true;
-              if ((!$is_sticky_accept) ||
-                  ($object->getStatus() != $status_plan)) {
-                $downgrade_accepts = true;
-              }
-              break;
-          }
-          break;
       }
     }
 
@@ -952,41 +934,6 @@ final class DifferentialTransactionEditor
         }
         break;
 
-      case DifferentialAction::ACTION_REQUEST:
-        if (!$actor_is_author) {
-          return pht(
-            'You can not request review of this revision because you do '.
-            'not own it. To request review of a revision, you must be its '.
-            'owner.');
-        }
-
-        switch ($revision_status) {
-          case ArcanistDifferentialRevisionStatus::ACCEPTED:
-          case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
-          case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED:
-            // These are OK.
-            break;
-          case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
-            // This will be caught as "no effect" later on.
-            break;
-          case ArcanistDifferentialRevisionStatus::ABANDONED:
-            return pht(
-              'You can not request review of this revision because it has '.
-              'been abandoned. Instead, reclaim it.');
-          case ArcanistDifferentialRevisionStatus::CLOSED:
-            return pht(
-              'You can not request review of this revision because it has '.
-              'already been closed.');
-          default:
-            throw new Exception(
-              pht(
-                'Encountered unexpected revision status ("%s") when '.
-                'validating "%s" action.',
-                $revision_status,
-                $action));
-        }
-        break;
-
       case DifferentialAction::ACTION_CLOSE:
         // We force revisions closed when we discover a corresponding commit.
         // In this case, revisions are allowed to transition to closed from
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
index 868a24ebb0..c6c55e0cdd 100644
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -607,8 +607,6 @@ final class DifferentialTransaction
               'not closed.');
           case DifferentialAction::ACTION_RETHINK:
             return pht('This revision already requires changes.');
-          case DifferentialAction::ACTION_REQUEST:
-            return pht('Review is already requested for this revision.');
           case DifferentialAction::ACTION_CLAIM:
             return pht(
               'You can not commandeer this revision because you already own '.