Move Harbormaster Build Plans to modular transactions

Summary: Depends on D20216. Ref T13258. Bland infrastructure update to prepare for bigger things.

Test Plan: Created and edited a build plan.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13258

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

src/__phutil_library_map__.php

@@ -1334,12 +1334,15 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanEditEngine' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php',
     'HarbormasterBuildPlanEditor' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditor.php',
     'HarbormasterBuildPlanNameNgrams' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanNameNgrams.php',
+    'HarbormasterBuildPlanNameTransaction' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php',
     'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php',
     'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php',
     'HarbormasterBuildPlanSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildPlanSearchAPIMethod.php',
     'HarbormasterBuildPlanSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildPlanSearchEngine.php',
+    'HarbormasterBuildPlanStatusTransaction' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php',
     'HarbormasterBuildPlanTransaction' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php',
     'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php',
+    'HarbormasterBuildPlanTransactionType' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php',
     'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php',
     'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php',
     'HarbormasterBuildSearchConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php',
@@ -6943,12 +6946,15 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanEditEngine' => 'PhabricatorEditEngine',
     'HarbormasterBuildPlanEditor' => 'PhabricatorApplicationTransactionEditor',
     'HarbormasterBuildPlanNameNgrams' => 'PhabricatorSearchNgrams',
+    'HarbormasterBuildPlanNameTransaction' => 'HarbormasterBuildPlanTransactionType',
     'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType',
     'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HarbormasterBuildPlanSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'HarbormasterBuildPlanSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'HarbormasterBuildPlanTransaction' => 'PhabricatorApplicationTransaction',
+    'HarbormasterBuildPlanStatusTransaction' => 'HarbormasterBuildPlanTransactionType',
+    'HarbormasterBuildPlanTransaction' => 'PhabricatorModularTransaction',
     'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'HarbormasterBuildPlanTransactionType' => 'PhabricatorModularTransactionType',
     'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HarbormasterBuildRequest' => 'Phobject',
     'HarbormasterBuildSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',

src/applications/harbormaster/controller/HarbormasterPlanDisableController.php

@@ -19,11 +19,11 @@ final class HarbormasterPlanDisableController
       return new Aphront404Response();
     }
 
-    $plan_uri = $this->getApplicationURI('plan/'.$plan->getID().'/');
+    $plan_uri = $plan->getURI();
 
     if ($request->isFormPost()) {
 
-      $type_status = HarbormasterBuildPlanTransaction::TYPE_STATUS;
+      $type_status = HarbormasterBuildPlanStatusTransaction::TRANSACTIONTYPE;
 
       $v_status = $plan->isDisabled()
         ? HarbormasterBuildPlan::STATUS_ACTIVE

src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php

@@ -82,7 +82,8 @@ final class HarbormasterBuildPlanEditEngine
         ->setKey('name')
         ->setLabel(pht('Name'))
         ->setIsRequired(true)
-        ->setTransactionType(HarbormasterBuildPlanTransaction::TYPE_NAME)
+        ->setTransactionType(
+          HarbormasterBuildPlanNameTransaction::TRANSACTIONTYPE)
         ->setDescription(pht('The build plan name.'))
         ->setConduitDescription(pht('Rename the plan.'))
         ->setConduitTypeDescription(pht('New plan name.'))

src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php

@@ -11,100 +11,23 @@ final class HarbormasterBuildPlanEditor
     return pht('Harbormaster Build Plans');
   }
 
+  public function getCreateObjectTitle($author, $object) {
+    return pht('%s created this build plan.', $author);
+  }
+
+  public function getCreateObjectTitleForFeed($author, $object) {
+    return pht('%s created %s.', $author, $object);
+  }
+
   protected function supportsSearch() {
     return true;
   }
 
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
-    $types[] = HarbormasterBuildPlanTransaction::TYPE_NAME;
-    $types[] = HarbormasterBuildPlanTransaction::TYPE_STATUS;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case HarbormasterBuildPlanTransaction::TYPE_NAME:
-        if ($this->getIsNewObject()) {
-          return null;
-        }
-        return $object->getName();
-      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
-        return $object->getPlanStatus();
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case HarbormasterBuildPlanTransaction::TYPE_NAME:
-        return $xaction->getNewValue();
-      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
-        return $xaction->getNewValue();
-    }
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case HarbormasterBuildPlanTransaction::TYPE_NAME:
-        $object->setName($xaction->getNewValue());
-        return;
-      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
-        $object->setPlanStatus($xaction->getNewValue());
-        return;
-    }
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case HarbormasterBuildPlanTransaction::TYPE_NAME:
-      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
-        return;
-    }
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case HarbormasterBuildPlanTransaction::TYPE_NAME:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getName(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('You must choose a name for your build plan.'),
-            last($xactions));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-        break;
-    }
-
-    return $errors;
-  }
-
-
 }

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php

@@ -84,6 +84,16 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     return ($this->getPlanStatus() == self::STATUS_DISABLED);
   }
 
+  public function getURI() {
+    return urisprintf(
+      '/harbormaster/plan/%s/',
+      $this->getID());
+  }
+
+  public function getObjectName() {
+    return pht('Build Plan %d', $this->getID());
+  }
+
 
 /* -(  Autoplans  )---------------------------------------------------------- */
 

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php

@@ -1,10 +1,7 @@
 <?php
 
 final class HarbormasterBuildPlanTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
-
-  const TYPE_NAME = 'harbormaster:name';
-  const TYPE_STATUS = 'harbormaster:status';
 
   public function getApplicationName() {
     return 'harbormaster';
@@ -14,67 +11,8 @@ final class HarbormasterBuildPlanTransaction
     return HarbormasterBuildPlanPHIDType::TYPECONST;
   }
 
-  public function getIcon() {
+  public function getBaseTransactionClass() {
-    $old = $this->getOldValue();
+    return 'HarbormasterBuildPlanTransactionType';
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return 'fa-plus';
-        }
-        break;
-    }
-
-    return parent::getIcon();
-  }
-
-  public function getColor() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return 'green';
-        }
-        break;
-    }
-
-    return parent::getIcon();
-  }
-
-  public function getTitle() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-    $author_handle = $this->renderHandleLink($this->getAuthorPHID());
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return pht(
-            '%s created this build plan.',
-            $author_handle);
-        } else {
-          return pht(
-            '%s renamed this build plan from "%s" to "%s".',
-            $author_handle,
-            $old,
-            $new);
-        }
-      case self::TYPE_STATUS:
-        if ($new == HarbormasterBuildPlan::STATUS_DISABLED) {
-          return pht(
-            '%s disabled this build plan.',
-            $author_handle);
-        } else {
-          return pht(
-            '%s enabled this build plan.',
-            $author_handle);
-        }
-    }
-
-    return parent::getTitle();
   }
 
 }

src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php

@@ -0,0 +1,46 @@
+<?php
+
+final class HarbormasterBuildPlanNameTransaction
+  extends HarbormasterBuildPlanTransactionType {
+
+  const TRANSACTIONTYPE = 'harbormaster:name';
+
+  public function generateOldValue($object) {
+    return $object->getName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setName($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s renamed this build plan from "%s" to "%s".',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('You must choose a name for your build plan.'));
+    }
+
+    return $errors;
+  }
+
+  public function getTransactionTypeForConduit($xaction) {
+    return 'name';
+  }
+
+  public function getFieldValuesForConduit($xaction, $data) {
+    return array(
+      'old' => $xaction->getOldValue(),
+      'new' => $xaction->getNewValue(),
+    );
+  }
+
+}

src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php

@@ -0,0 +1,67 @@
+<?php
+
+final class HarbormasterBuildPlanStatusTransaction
+  extends HarbormasterBuildPlanTransactionType {
+
+  const TRANSACTIONTYPE = 'harbormaster:status';
+
+  public function generateOldValue($object) {
+    return $object->getPlanStatus();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setPlanStatus($value);
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    if ($new === HarbormasterBuildPlan::STATUS_DISABLED) {
+      return pht(
+        '%s disabled this build plan.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s enabled this build plan.',
+        $this->renderAuthor());
+    }
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $options = array(
+      HarbormasterBuildPlan::STATUS_DISABLED,
+      HarbormasterBuildPlan::STATUS_ACTIVE,
+    );
+    $options = array_fuse($options);
+
+    foreach ($xactions as $xaction) {
+      $new = $xaction->getNewValue();
+
+      if (!isset($options[$new])) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Status "%s" is not a valid build plan status. Valid '.
+            'statuses are: %s.',
+            $new,
+            implode(', ', $options)));
+        continue;
+      }
+
+    }
+
+    return $errors;
+  }
+
+  public function getTransactionTypeForConduit($xaction) {
+    return 'status';
+  }
+
+  public function getFieldValuesForConduit($xaction, $data) {
+    return array(
+      'old' => $xaction->getOldValue(),
+      'new' => $xaction->getNewValue(),
+    );
+  }
+
+}

src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php

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