Modularize Dashboard Panel transactionns

Summary: Depends on D20369. Ref T13272. Move toward a world where we can edit panels with just one controller, instead of separate "Edit" and "Editpro" controllers.

Test Plan: Created and edited panels. This will get vetted more thoroughly after additional changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

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

src/__phutil_library_map__.php

@@ -2936,6 +2936,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardPanelEditproController' => 'applications/dashboard/controller/panel/PhabricatorDashboardPanelEditproController.php',
     'PhabricatorDashboardPanelHasDashboardEdgeType' => 'applications/dashboard/edge/PhabricatorDashboardPanelHasDashboardEdgeType.php',
     'PhabricatorDashboardPanelListController' => 'applications/dashboard/controller/panel/PhabricatorDashboardPanelListController.php',
+    'PhabricatorDashboardPanelNameTransaction' => 'applications/dashboard/xaction/panel/PhabricatorDashboardPanelNameTransaction.php',
     'PhabricatorDashboardPanelNgrams' => 'applications/dashboard/storage/PhabricatorDashboardPanelNgrams.php',
     'PhabricatorDashboardPanelPHIDType' => 'applications/dashboard/phid/PhabricatorDashboardPanelPHIDType.php',
     'PhabricatorDashboardPanelQuery' => 'applications/dashboard/query/PhabricatorDashboardPanelQuery.php',
@@ -2944,10 +2945,12 @@ phutil_register_library_map(array(
     'PhabricatorDashboardPanelSearchApplicationCustomField' => 'applications/dashboard/customfield/PhabricatorDashboardPanelSearchApplicationCustomField.php',
     'PhabricatorDashboardPanelSearchEngine' => 'applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php',
     'PhabricatorDashboardPanelSearchQueryCustomField' => 'applications/dashboard/customfield/PhabricatorDashboardPanelSearchQueryCustomField.php',
+    'PhabricatorDashboardPanelStatusTransaction' => 'applications/dashboard/xaction/panel/PhabricatorDashboardPanelStatusTransaction.php',
     'PhabricatorDashboardPanelTabsCustomField' => 'applications/dashboard/customfield/PhabricatorDashboardPanelTabsCustomField.php',
     'PhabricatorDashboardPanelTransaction' => 'applications/dashboard/storage/PhabricatorDashboardPanelTransaction.php',
     'PhabricatorDashboardPanelTransactionEditor' => 'applications/dashboard/editor/PhabricatorDashboardPanelTransactionEditor.php',
     'PhabricatorDashboardPanelTransactionQuery' => 'applications/dashboard/query/PhabricatorDashboardPanelTransactionQuery.php',
+    'PhabricatorDashboardPanelTransactionType' => 'applications/dashboard/xaction/panel/PhabricatorDashboardPanelTransactionType.php',
     'PhabricatorDashboardPanelType' => 'applications/dashboard/paneltype/PhabricatorDashboardPanelType.php',
     'PhabricatorDashboardPanelViewController' => 'applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php',
     'PhabricatorDashboardPortal' => 'applications/dashboard/storage/PhabricatorDashboardPortal.php',
@@ -8915,6 +8918,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardPanelEditproController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardPanelHasDashboardEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorDashboardPanelListController' => 'PhabricatorDashboardController',
+    'PhabricatorDashboardPanelNameTransaction' => 'PhabricatorDashboardPanelTransactionType',
     'PhabricatorDashboardPanelNgrams' => 'PhabricatorSearchNgrams',
     'PhabricatorDashboardPanelPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorDashboardPanelQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
@@ -8923,10 +8927,12 @@ phutil_register_library_map(array(
     'PhabricatorDashboardPanelSearchApplicationCustomField' => 'PhabricatorStandardCustomField',
     'PhabricatorDashboardPanelSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorDashboardPanelSearchQueryCustomField' => 'PhabricatorStandardCustomField',
+    'PhabricatorDashboardPanelStatusTransaction' => 'PhabricatorDashboardPanelTransactionType',
     'PhabricatorDashboardPanelTabsCustomField' => 'PhabricatorStandardCustomField',
-    'PhabricatorDashboardPanelTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorDashboardPanelTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorDashboardPanelTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhabricatorDashboardPanelTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorDashboardPanelTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorDashboardPanelType' => 'Phobject',
     'PhabricatorDashboardPanelViewController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardPortal' => array(

src/applications/dashboard/controller/PhabricatorDashboardQueryPanelInstallController.php

@@ -98,7 +98,8 @@ final class PhabricatorDashboardQueryPanelInstallController
         $xactions = array();
 
         $xactions[] = id(new PhabricatorDashboardPanelTransaction())
-          ->setTransactionType(PhabricatorDashboardPanelTransaction::TYPE_NAME)
+          ->setTransactionType(
+            PhabricatorDashboardPanelNameTransaction::TRANSACTIONTYPE)
           ->setNewValue($v_name);
 
         $xactions[] = id(new PhabricatorDashboardPanelTransaction())

src/applications/dashboard/controller/dashboard/PhabricatorDashboardEditController.php

@@ -198,7 +198,8 @@ final class PhabricatorDashboardEditController
     $xactions = array();
 
     $xactions[] = id(new PhabricatorDashboardPanelTransaction())
-      ->setTransactionType(PhabricatorDashboardPanelTransaction::TYPE_NAME)
+      ->setTransactionType(
+        PhabricatorDashboardPanelNameTransaction::TRANSACTIONTYPE)
       ->setNewValue($name);
 
     $editor = id(new PhabricatorDashboardPanelTransactionEditor())

src/applications/dashboard/controller/panel/PhabricatorDashboardPanelArchiveController.php

@@ -25,7 +25,8 @@ final class PhabricatorDashboardPanelArchiveController
     if ($request->isFormPost()) {
       $xactions = array();
       $xactions[] = id(new PhabricatorDashboardPanelTransaction())
-        ->setTransactionType(PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE)
+        ->setTransactionType(
+          PhabricatorDashboardPanelStatusTransaction::TRANSACTIONTYPE)
         ->setNewValue((int)!$panel->getIsArchived());
 
       id(new PhabricatorDashboardPanelTransactionEditor())

src/applications/dashboard/controller/panel/PhabricatorDashboardPanelEditController.php

@@ -112,7 +112,7 @@ final class PhabricatorDashboardPanelEditController
       $v_view_policy = $request->getStr('viewPolicy');
       $v_edit_policy = $request->getStr('editPolicy');
 
-      $type_name = PhabricatorDashboardPanelTransaction::TYPE_NAME;
+      $type_name = PhabricatorDashboardPanelNameTransaction::TRANSACTIONTYPE;
       $type_view_policy = PhabricatorTransactions::TYPE_VIEW_POLICY;
       $type_edit_policy = PhabricatorTransactions::TYPE_EDIT_POLICY;
 

src/applications/dashboard/editor/PhabricatorDashboardPanelEditEngine.php

@@ -91,7 +91,8 @@ final class PhabricatorDashboardPanelEditEngine
         ->setDescription(pht('Name of the panel.'))
         ->setConduitDescription(pht('Rename the panel.'))
         ->setConduitTypeDescription(pht('New panel name.'))
-        ->setTransactionType(PhabricatorDashboardPanelTransaction::TYPE_NAME)
+        ->setTransactionType(
+          PhabricatorDashboardPanelNameTransaction::TRANSACTIONTYPE)
         ->setIsRequired(true)
         ->setValue($object->getName()),
     );

src/applications/dashboard/editor/PhabricatorDashboardPanelTransactionEditor.php

@@ -18,96 +18,7 @@ final class PhabricatorDashboardPanelTransactionEditor
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDGE;
 
-    $types[] = PhabricatorDashboardPanelTransaction::TYPE_NAME;
-    $types[] = PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE;
-
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorDashboardPanelTransaction::TYPE_NAME:
-        if ($this->getIsNewObject()) {
-          return null;
-        }
-        return $object->getName();
-      case PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE:
-        return (int)$object->getIsArchived();
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorDashboardPanelTransaction::TYPE_NAME:
-        return $xaction->getNewValue();
-      case PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE:
-        return (int)$xaction->getNewValue();
-    }
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorDashboardPanelTransaction::TYPE_NAME:
-        $object->setName($xaction->getNewValue());
-        return;
-      case PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE:
-        $object->setIsArchived((int)$xaction->getNewValue());
-        return;
-    }
-
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorDashboardPanelTransaction::TYPE_NAME:
-      case PhabricatorDashboardPanelTransaction::TYPE_ARCHIVE:
-        return;
-    }
-
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorDashboardPanelTransaction::TYPE_NAME:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getName(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('Panel name is required.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-        break;
-    }
-
-    return $errors;
-  }
-
-
 }

src/applications/dashboard/storage/PhabricatorDashboardPanelTransaction.php

@@ -1,10 +1,7 @@
 <?php
 
 final class PhabricatorDashboardPanelTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
-
-  const TYPE_NAME = 'dashpanel:name';
-  const TYPE_ARCHIVE = 'dashboard:archive';
 
   public function getApplicationName() {
     return 'dashboard';
@@ -14,99 +11,8 @@ final class PhabricatorDashboardPanelTransaction
     return PhabricatorDashboardPanelPHIDType::TYPECONST;
   }
 
-  public function getTitle() {
+  public function getBaseTransactionClass() {
-    $author_phid = $this->getAuthorPHID();
+    return 'PhabricatorDashboardPanelTransactionType';
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $author_link = $this->renderHandleLink($author_phid);
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        if (!strlen($old)) {
-          return pht(
-            '%s created this panel.',
-            $author_link);
-        } else {
-          return pht(
-            '%s renamed this panel from "%s" to "%s".',
-            $author_link,
-            $old,
-            $new);
-        }
-      case self::TYPE_ARCHIVE:
-        if ($new) {
-          return pht(
-            '%s archived this panel.',
-            $author_link);
-        } else {
-          return pht(
-            '%s activated this panel.',
-            $author_link);
-        }
-    }
-
-    return parent::getTitle();
   }
 
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $author_link = $this->renderHandleLink($author_phid);
-    $object_link = $this->renderHandleLink($object_phid);
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        if (!strlen($old)) {
-          return pht(
-            '%s created dashboard panel %s.',
-            $author_link,
-            $object_link);
-        } else {
-          return pht(
-            '%s renamed dashboard panel %s from "%s" to "%s".',
-            $author_link,
-            $object_link,
-            $old,
-            $new);
-        }
-      case self::TYPE_ARCHIVE:
-        if ($new) {
-          return pht(
-            '%s archived dashboard panel %s.',
-            $author_link,
-            $object_link);
-        } else {
-          return pht(
-            '%s activated dashboard panel %s.',
-            $author_link,
-            $object_link);
-        }
-    }
-
-    return parent::getTitleForFeed();
-  }
-
-  public function getColor() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        if (!strlen($old)) {
-          return PhabricatorTransactions::COLOR_GREEN;
-        }
-        break;
-    }
-
-    return parent::getColor();
-  }
 }

src/applications/dashboard/xaction/panel/PhabricatorDashboardPanelNameTransaction.php

@@ -0,0 +1,70 @@
+<?php
+
+final class PhabricatorDashboardPanelNameTransaction
+  extends PhabricatorDashboardPanelTransactionType {
+
+  const TRANSACTIONTYPE = 'dashpanel:name';
+
+  public function generateOldValue($object) {
+    return $object->getName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setName($value);
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    return pht(
+      '%s renamed this panel from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $max_length = $object->getColumnMaximumByteLength('name');
+    foreach ($xactions as $xaction) {
+      $new = $xaction->getNewValue();
+      if (!strlen($new)) {
+        $errors[] = $this->newInvalidError(
+          pht('Panels must have a title.'),
+          $xaction);
+        continue;
+      }
+
+      if (strlen($new) > $max_length) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Panel names must not be longer than %s characters.',
+            $max_length));
+        continue;
+      }
+    }
+
+    if (!$errors) {
+      if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+        $errors[] = $this->newRequiredError(
+          pht('Panels must have a title.'));
+      }
+    }
+
+    return $errors;
+  }
+
+  public function getTransactionTypeForConduit($xaction) {
+    return 'name';
+  }
+
+  public function getFieldValuesForConduit($xaction, $data) {
+    return array(
+      'old' => $xaction->getOldValue(),
+      'new' => $xaction->getNewValue(),
+    );
+  }
+
+}

src/applications/dashboard/xaction/panel/PhabricatorDashboardPanelStatusTransaction.php

@@ -0,0 +1,33 @@
+<?php
+
+final class PhabricatorDashboardPanelStatusTransaction
+  extends PhabricatorDashboardPanelTransactionType {
+
+  const TRANSACTIONTYPE = 'dashpanel:archive';
+
+  public function generateOldValue($object) {
+    return (bool)$object->getIsArchived();
+  }
+
+  public function generateNewValue($object, $value) {
+    return (bool)$value;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setIsArchived((int)$value);
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    if ($new) {
+      return pht(
+        '%s archived this panel.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s activated this panel.',
+        $this->renderAuthor());
+    }
+  }
+
+}

src/applications/dashboard/xaction/panel/PhabricatorDashboardPanelTransactionType.php

@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorDashboardPanelTransactionType
+  extends PhabricatorModularTransactionType {}