Order actions sensibly within Differential revision comment action groups

Summary:
Ref T11114. See D17114 for some discussion.

For review actions: accept, reject, resign.

For revision actions, order is basically least-severe to most-severe action pairs: plan changes, request review, close, reopen, abandon, reclaim, commandeer.

Test Plan: Viewed revisions as an author and a reviewer, saw sensible action order within action groups.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17115

src/applications/differential/editor/DifferentialRevisionEditEngine.php

@@ -229,6 +229,8 @@ final class DifferentialRevisionEditEngine
       ->setValue(array());
 
     $actions = DifferentialRevisionActionTransaction::loadAllActions();
+    $actions = msortv($actions, 'getRevisionActionOrderVector');
+
     foreach ($actions as $key => $action) {
       $fields[] = $action->newEditField($object, $viewer);
     }

src/applications/differential/storage/DifferentialRevision.php

@@ -447,6 +447,11 @@ final class DifferentialRevision extends DifferentialDAO
     return ($this->getStatus() == $status_abandoned);
   }
 
+  public function isAccepted() {
+    $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+    return ($this->getStatus() == $status_accepted);
+  }
+
   public function getStatusIcon() {
     $map = array(
       ArcanistDifferentialRevisionStatus::NEEDS_REVIEW

src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionAbandonTransaction
     return 'indigo';
   }
 
+  protected function getRevisionActionOrder() {
+    return 500;
+  }
+
   public function generateOldValue($object) {
     return $object->isAbandoned();
   }

src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionAcceptTransaction
     return 'green';
   }
 
+  protected function getRevisionActionOrder() {
+    return 500;
+  }
+
   public function generateOldValue($object) {
     $actor = $this->getActor();
     return $this->isViewerAcceptingReviewer($object, $actor);

src/applications/differential/xaction/DifferentialRevisionActionTransaction.php

@@ -19,6 +19,15 @@ abstract class DifferentialRevisionActionTransaction
   abstract protected function validateAction($object, PhabricatorUser $viewer);
   abstract protected function getRevisionActionLabel();
 
+  protected function getRevisionActionOrder() {
+    return 1000;
+  }
+
+  public function getRevisionActionOrderVector() {
+    return id(new PhutilSortVector())
+      ->addInt($this->getRevisionActionOrder());
+  }
+
   protected function getRevisionActionGroupKey() {
     return DifferentialRevisionEditEngine::ACTIONGROUP_REVISION;
   }

src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionCloseTransaction
     return 'indigo';
   }
 
+  protected function getRevisionActionOrder() {
+    return 300;
+  }
+
   public function generateOldValue($object) {
     return $object->isClosed();
   }
@@ -49,6 +53,13 @@ final class DifferentialRevisionCloseTransaction
           'closed. Only open revisions can be closed.'));
     }
 
+    if (!$object->isAccepted()) {
+      throw new Exception(
+        pht(
+          'You can not close this revision because it has not been accepted. '.
+          'Revisions must be accepted before they can be closed.'));
+    }
+
     $config_key = 'differential.always-allow-close';
     if (!PhabricatorEnv::getEnvConfig($config_key)) {
       if (!$this->isViewerRevisionAuthor($object, $viewer)) {

src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionCommandeerTransaction
     return 'sky';
   }
 
+  protected function getRevisionActionOrder() {
+    return 700;
+  }
+
   public function generateOldValue($object) {
     return $object->getAuthorPHID();
   }

src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php

@@ -23,6 +23,10 @@ final class DifferentialRevisionPlanChangesTransaction
     return 'red';
   }
 
+  protected function getRevisionActionOrder() {
+    return 200;
+  }
+
   public function generateOldValue($object) {
     $status_planned = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
     return ($object->getStatus() == $status_planned);

src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionReclaimTransaction
     return 'sky';
   }
 
+  protected function getRevisionActionOrder() {
+    return 600;
+  }
+
   public function generateOldValue($object) {
     return !$object->isAbandoned();
   }

src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionRejectTransaction
     return 'red';
   }
 
+  protected function getRevisionActionOrder() {
+    return 600;
+  }
+
   public function generateOldValue($object) {
     $actor = $this->getActor();
     return $this->isViewerRejectingReviewer($object, $actor);

src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionReopenTransaction
     return 'sky';
   }
 
+  protected function getRevisionActionOrder() {
+    return 400;
+  }
+
   public function generateOldValue($object) {
     return !$object->isClosed();
   }

src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php

@@ -18,6 +18,10 @@ final class DifferentialRevisionRequestReviewTransaction
     return 'sky';
   }
 
+  protected function getRevisionActionOrder() {
+    return 200;
+  }
+
   public function generateOldValue($object) {
     $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
     return ($object->getStatus() == $status_review);

src/applications/differential/xaction/DifferentialRevisionResignTransaction.php

@@ -22,6 +22,10 @@ final class DifferentialRevisionResignTransaction
     return 'orange';
   }
 
+  protected function getRevisionActionOrder() {
+    return 700;
+  }
+
   public function generateOldValue($object) {
     $actor = $this->getActor();
     return !$this->isViewerAnyReviewer($object, $actor);