From b21b73b8dd35301e083080a5ed7c7c83ae366ca3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 9 Jul 2020 12:23:31 -0700
Subject: [PATCH] Expand Revision transaction API to allow actions to vary more
 broadly based on the viewer and revision state

Summary:
See PHI1810. Build toward support for "Request Review" by non-authors on drafts, to forcefully pull a revision out of draft.

Currently, some action strings can't vary based on revision state or the current viewer, so this "pull out of draft" action would have to either: say "Request Review"; or be a totally separate action.

Neither seem great, so allow the labels and messages to vary based on the viewer and revision state.

Test Plan: Grepped for affected symbols, see followup changes.

Differential Revision: https://secure.phabricator.com/D21401
---
 .../DifferentialRevisionAbandonTransaction.php |  7 +++++--
 .../DifferentialRevisionAcceptTransaction.php  |  7 +++++--
 .../DifferentialRevisionActionTransaction.php  | 18 ++++++++++++------
 .../DifferentialRevisionCloseTransaction.php   |  7 +++++--
 ...fferentialRevisionCommandeerTransaction.php |  7 +++++--
 ...ferentialRevisionPlanChangesTransaction.php |  7 +++++--
 .../DifferentialRevisionReclaimTransaction.php |  7 +++++--
 .../DifferentialRevisionRejectTransaction.php  |  7 +++++--
 .../DifferentialRevisionReopenTransaction.php  |  7 +++++--
 ...rentialRevisionRequestReviewTransaction.php | 10 +++++++---
 .../DifferentialRevisionResignTransaction.php  |  7 +++++--
 11 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php
index c2baef034f..c793663b89 100644
--- a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionAbandonTransaction
   const TRANSACTIONTYPE = 'differential.revision.abandon';
   const ACTIONKEY = 'abandon';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Abandon Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('This revision will be abandoned and closed.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
index 3d2df14899..12596d8693 100644
--- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionAcceptTransaction
   const TRANSACTIONTYPE = 'differential.revision.accept';
   const ACTIONKEY = 'accept';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Accept Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('These changes will be approved.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
index 338fde99b2..241678487a 100644
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
@@ -17,7 +17,9 @@ abstract class DifferentialRevisionActionTransaction
   }
 
   abstract protected function validateAction($object, PhabricatorUser $viewer);
-  abstract protected function getRevisionActionLabel();
+  abstract protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer);
 
   protected function validateOptionValue($object, $actor, array $value) {
     return null;
@@ -53,12 +55,14 @@ abstract class DifferentialRevisionActionTransaction
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return null;
   }
 
   protected function getRevisionActionSubmitButtonText(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return null;
   }
 
@@ -105,17 +109,19 @@ abstract class DifferentialRevisionActionTransaction
       ->setValue(true);
 
     if ($this->isActionAvailable($revision, $viewer)) {
-      $label = $this->getRevisionActionLabel();
+      $label = $this->getRevisionActionLabel($revision, $viewer);
       if ($label !== null) {
         $field->setCommentActionLabel($label);
 
-        $description = $this->getRevisionActionDescription($revision);
+        $description = $this->getRevisionActionDescription($revision, $viewer);
         $field->setActionDescription($description);
 
         $group_key = $this->getRevisionActionGroupKey();
         $field->setCommentActionGroupKey($group_key);
 
-        $button_text = $this->getRevisionActionSubmitButtonText($revision);
+        $button_text = $this->getRevisionActionSubmitButtonText(
+          $revision,
+          $viewer);
         $field->setActionSubmitButtonText($button_text);
 
         // Currently, every revision action conflicts with every other
diff --git a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php
index 1a6017a02f..b1ad04ba77 100644
--- a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionCloseTransaction
   const TRANSACTIONTYPE = 'differential.revision.close';
   const ACTIONKEY = 'close';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Close Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('This revision will be closed.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php
index a296597bc7..88f6a7258b 100644
--- a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionCommandeerTransaction
   const TRANSACTIONTYPE = 'differential.revision.commandeer';
   const ACTIONKEY = 'commandeer';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Commandeer Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('You will take control of this revision and become its author.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
index f4fb0a3eb1..144152526d 100644
--- a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionPlanChangesTransaction
   const TRANSACTIONTYPE = 'differential.revision.plan';
   const ACTIONKEY = 'plan-changes';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Plan Changes');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht(
       'This revision will be removed from review queues until it is revised.');
   }
diff --git a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php
index e023dda0a1..6f9d7b79bf 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionReclaimTransaction
   const TRANSACTIONTYPE = 'differential.revision.reclaim';
   const ACTIONKEY = 'reclaim';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Reclaim Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('This revision will be reclaimed and reopened.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
index 0001ce09ca..cd3e815d26 100644
--- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionRejectTransaction
   const TRANSACTIONTYPE = 'differential.revision.reject';
   const ACTIONKEY = 'reject';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Request Changes');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('This revision will be returned to the author for updates.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php
index a2d25287bf..2e7a18fb22 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionReopenTransaction
   const TRANSACTIONTYPE = 'differential.revision.reopen';
   const ACTIONKEY = 'reopen';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Reopen Revision');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('This revision will be reopened for review.');
   }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
index 169e41dec5..026b57b55c 100644
--- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionRequestReviewTransaction
   const TRANSACTIONTYPE = 'differential.revision.request';
   const ACTIONKEY = 'request-review';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Request Review');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     if ($revision->isDraft()) {
       return pht('This revision will be submitted to reviewers for feedback.');
     } else {
@@ -20,7 +23,8 @@ final class DifferentialRevisionRequestReviewTransaction
   }
 
   protected function getRevisionActionSubmitButtonText(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
 
     // See PHI975. When the action stack will promote the revision out of
     // draft, change the button text from "Submit Quietly".
diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
index a2b4fd4337..faab748cdd 100644
--- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
@@ -6,12 +6,15 @@ final class DifferentialRevisionResignTransaction
   const TRANSACTIONTYPE = 'differential.revision.resign';
   const ACTIONKEY = 'resign';
 
-  protected function getRevisionActionLabel() {
+  protected function getRevisionActionLabel(
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('Resign as Reviewer');
   }
 
   protected function getRevisionActionDescription(
-    DifferentialRevision $revision) {
+    DifferentialRevision $revision,
+    PhabricatorUser $viewer) {
     return pht('You will resign as a reviewer for this change.');
   }