Maniphest - move subpriority edits to be transaction powered

Summary:
...this was nice to do for boards, since this diff also starts calling this code in the board move case. The big trick is to use the new expandTransactions hook to expand the subpriority transaction into a priority transaction if pertinent. The other stuff is just about hiding these new subpriority extractions.

...also removes the "edit" UI from the default board since we can't actually edit anything and it thus is buggy.

Ref T4422. Next step is to move board edits into the editor with their own little transaction.

Test Plan: re-orded tasks on a maniphest query, reloaded, and noted re-order stuck.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4422

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

src/applications/maniphest/controller/ManiphestSubpriorityController.php

@@ -41,28 +41,18 @@ final class ManiphestSubpriorityController extends ManiphestController {
       $after_sub = null;
     }
 
-    $new_sub = ManiphestTransactionEditor::getNextSubpriority(
-      $after_pri,
-      $after_sub);
+    $xactions = array(id(new ManiphestTransaction())
+      ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY)
+      ->setNewValue(array(
+        'newPriority' => $after_pri,
+        'newSubpriorityBase' => $after_sub)));
+    $editor = id(new ManiphestTransactionEditor())
+      ->setActor($user)
+      ->setContinueOnMissingFields(true)
+      ->setContinueOnNoEffect(true)
+      ->setContentSourceFromRequest($request);
 
-    $task->setSubpriority($new_sub);
-
-    if ($after_pri != $task->getPriority()) {
-      $xactions = array();
-      $xactions[] = id(new ManiphestTransaction())
-        ->setTransactionType(ManiphestTransaction::TYPE_PRIORITY)
-        ->setNewValue($after_pri);
-
-      $editor = id(new ManiphestTransactionEditor())
-        ->setActor($user)
-        ->setContinueOnMissingFields(true)
-        ->setContinueOnNoEffect(true)
-        ->setContentSourceFromRequest($request);
-
-      $editor->applyTransactions($task, $xactions);
-    } else {
-      $task->save();
-    }
+    $editor->applyTransactions($task, $xactions);
 
     return id(new AphrontAjaxResponse())->setContent(
       array(

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -16,6 +16,7 @@ final class ManiphestTransactionEditor
     $types[] = ManiphestTransaction::TYPE_PROJECTS;
     $types[] = ManiphestTransaction::TYPE_ATTACH;
     $types[] = ManiphestTransaction::TYPE_EDGE;
+    $types[] = ManiphestTransaction::TYPE_SUBPRIORITY;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 
@@ -58,6 +59,8 @@ final class ManiphestTransactionEditor
       case ManiphestTransaction::TYPE_EDGE:
         // These are pre-populated.
         return $xaction->getOldValue();
+      case ManiphestTransaction::TYPE_SUBPRIORITY:
+        return $object->getSubpriority();
     }
 
   }
@@ -79,6 +82,7 @@ final class ManiphestTransactionEditor
       case ManiphestTransaction::TYPE_DESCRIPTION:
       case ManiphestTransaction::TYPE_ATTACH:
       case ManiphestTransaction::TYPE_EDGE:
+      case ManiphestTransaction::TYPE_SUBPRIORITY:
         return $xaction->getNewValue();
     }
   }
@@ -102,7 +106,6 @@ final class ManiphestTransactionEditor
     return parent::transactionHasEffect($object, $xaction);
   }
 
-
   protected function applyCustomInternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
@@ -147,10 +150,39 @@ final class ManiphestTransactionEditor
         // These are a weird, funky mess and are already being applied by the
         // time we reach this.
         return;
+      case ManiphestTransaction::TYPE_SUBPRIORITY:
+        $data = $xaction->getNewValue();
+        $new_sub = $this->getNextSubpriority(
+          $data['newPriority'],
+          $data['newSubpriorityBase']);
+        $object->setSubpriority($new_sub);
+        return;
     }
 
   }
 
+  protected function expandTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    $xactions = parent::expandTransaction($object, $xaction);
+    switch ($xaction->getTransactionType()) {
+      case ManiphestTransaction::TYPE_SUBPRIORITY:
+        $data = $xaction->getNewValue();
+        $new_pri = $data['newPriority'];
+        if ($new_pri != $object->getPriority()) {
+          $xactions[] = id(new ManiphestTransaction())
+            ->setTransactionType(ManiphestTransaction::TYPE_PRIORITY)
+            ->setNewValue($new_pri);
+        }
+        break;
+      default:
+        break;
+    }
+
+    return $xactions;
+  }
+
   protected function applyCustomExternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
@@ -159,7 +191,19 @@ final class ManiphestTransactionEditor
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-    return true;
+    $should_mail = true;
+    if (count($xactions) == 1) {
+      $xaction = head($xactions);
+      switch ($xaction->getTransactionType()) {
+        case ManiphestTransaction::TYPE_SUBPRIORITY:
+          $should_mail = false;
+          break;
+        default:
+          $should_mail = true;
+          break;
+      }
+    }
+    return $should_mail;
   }
 
   protected function getMailSubjectPrefix() {
@@ -305,10 +349,7 @@ final class ManiphestTransactionEditor
   }
 
 
-  public static function getNextSubpriority($pri, $sub) {
-
-    // TODO: T603 Figure out what the policies here should be once this gets
-    // cleaned up.
+  private function getNextSubpriority($pri, $sub) {
 
     if ($sub === null) {
       $next = id(new ManiphestTask())->loadOneWhere(

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -12,6 +12,7 @@ final class ManiphestTransaction
   const TYPE_PRIORITY = 'priority';
   const TYPE_EDGE = 'edge';
   const TYPE_ATTACH = 'attach';
+  const TYPE_SUBPRIORITY = 'subpriority';
 
   public function getApplicationName() {
     return 'maniphest';
@@ -85,6 +86,8 @@ final class ManiphestTransaction
           return false;
         }
         break;
+      case self::TYPE_SUBPRIORITY:
+        return true;
     }
 
     return false;

src/applications/project/controller/PhabricatorProjectBoardController.php

@@ -96,8 +96,10 @@ final class PhabricatorProjectBoardController
     foreach ($columns as $column) {
       $panel = id(new PHUIWorkpanelView())
         ->setHeader($column->getDisplayName())
-        ->setHeaderColor($column->getHeaderColor())
-        ->setEditURI('edit/'.$column->getID().'/');
+        ->setHeaderColor($column->getHeaderColor());
+      if (!$column->isDefaultColumn()) {
+        $panel->setEditURI('edit/'.$column->getID().'/');
+      }
 
       $cards = id(new PHUIObjectItemListView())
         ->setUser($viewer)

src/applications/project/controller/PhabricatorProjectMoveController.php

@@ -72,8 +72,31 @@ final class PhabricatorProjectMoveController
       $column_phid,
       $edge_phids);
 
-    // TODO: We also need to deal with priorities, so far this only gets stuff
-    // in the correct column.
+    if ($after_phid) {
+      $after_task = id(new ManiphestTaskQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($after_phid))
+        ->requireCapabilities(array(PhabricatorPolicyCapability::CAN_EDIT))
+        ->executeOne();
+      if (!$after_task) {
+        return new Aphront404Response();
+      }
+      $after_pri = $after_task->getPriority();
+      $after_sub = $after_task->getSubpriority();
+
+      $xactions = array(id(new ManiphestTransaction())
+        ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY)
+        ->setNewValue(array(
+          'newPriority' => $after_pri,
+          'newSubpriorityBase' => $after_sub)));
+      $editor = id(new ManiphestTransactionEditor())
+        ->setActor($viewer)
+        ->setContinueOnMissingFields(true)
+        ->setContinueOnNoEffect(true)
+        ->setContentSourceFromRequest($request);
+
+      $editor->applyTransactions($object, $xactions);
+    }
 
     return id(new AphrontAjaxResponse())->setContent(array());
   }

src/view/phui/PHUIWorkpanelView.php

@@ -53,15 +53,20 @@ final class PHUIWorkpanelView extends AphrontTagView {
           $footer_tag);
     }
 
-    $header_edit = id(new PHUIIconView())
-      ->setSpriteSheet(PHUIIconView::SPRITE_ACTIONS)
-      ->setSpriteIcon('settings-grey')
-      ->setHref($this->editURI);
+    $header_edit = null;
+    if ($this->editURI) {
+      $header_edit = id(new PHUIIconView())
+        ->setSpriteSheet(PHUIIconView::SPRITE_ACTIONS)
+        ->setSpriteIcon('settings-grey')
+        ->setHref($this->editURI);
+    }
 
     $header = id(new PhabricatorActionHeaderView())
       ->setHeaderTitle($this->header)
-      ->setHeaderColor($this->headerColor)
-      ->addAction($header_edit);
+      ->setHeaderColor($this->headerColor);
+    if ($header_edit) {
+      $header->addAction($header_edit);
+    }
 
     $body = phutil_tag(
       'div',