Reimplement Countdown transactions using Modular Transaction framework

Test Plan: owls

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

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

src/__phutil_library_map__.php

@@ -2413,9 +2413,11 @@ phutil_register_library_map(array(
     'PhabricatorCountdownDAO' => 'applications/countdown/storage/PhabricatorCountdownDAO.php',
     'PhabricatorCountdownDefaultEditCapability' => 'applications/countdown/capability/PhabricatorCountdownDefaultEditCapability.php',
     'PhabricatorCountdownDefaultViewCapability' => 'applications/countdown/capability/PhabricatorCountdownDefaultViewCapability.php',
+    'PhabricatorCountdownDescriptionTransaction' => 'applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php',
     'PhabricatorCountdownEditController' => 'applications/countdown/controller/PhabricatorCountdownEditController.php',
     'PhabricatorCountdownEditEngine' => 'applications/countdown/editor/PhabricatorCountdownEditEngine.php',
     'PhabricatorCountdownEditor' => 'applications/countdown/editor/PhabricatorCountdownEditor.php',
+    'PhabricatorCountdownEpochTransaction' => 'applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php',
     'PhabricatorCountdownListController' => 'applications/countdown/controller/PhabricatorCountdownListController.php',
     'PhabricatorCountdownMailReceiver' => 'applications/countdown/mail/PhabricatorCountdownMailReceiver.php',
     'PhabricatorCountdownQuery' => 'applications/countdown/query/PhabricatorCountdownQuery.php',
@@ -2423,9 +2425,11 @@ phutil_register_library_map(array(
     'PhabricatorCountdownReplyHandler' => 'applications/countdown/mail/PhabricatorCountdownReplyHandler.php',
     'PhabricatorCountdownSchemaSpec' => 'applications/countdown/storage/PhabricatorCountdownSchemaSpec.php',
     'PhabricatorCountdownSearchEngine' => 'applications/countdown/query/PhabricatorCountdownSearchEngine.php',
+    'PhabricatorCountdownTitleTransaction' => 'applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php',
     'PhabricatorCountdownTransaction' => 'applications/countdown/storage/PhabricatorCountdownTransaction.php',
     'PhabricatorCountdownTransactionComment' => 'applications/countdown/storage/PhabricatorCountdownTransactionComment.php',
     'PhabricatorCountdownTransactionQuery' => 'applications/countdown/query/PhabricatorCountdownTransactionQuery.php',
+    'PhabricatorCountdownTransactionType' => 'applications/countdown/xaction/PhabricatorCountdownTransactionType.php',
     'PhabricatorCountdownView' => 'applications/countdown/view/PhabricatorCountdownView.php',
     'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php',
     'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php',
@@ -7516,9 +7520,11 @@ phutil_register_library_map(array(
     'PhabricatorCountdownDAO' => 'PhabricatorLiskDAO',
     'PhabricatorCountdownDefaultEditCapability' => 'PhabricatorPolicyCapability',
     'PhabricatorCountdownDefaultViewCapability' => 'PhabricatorPolicyCapability',
+    'PhabricatorCountdownDescriptionTransaction' => 'PhabricatorCountdownTransactionType',
     'PhabricatorCountdownEditController' => 'PhabricatorCountdownController',
     'PhabricatorCountdownEditEngine' => 'PhabricatorEditEngine',
     'PhabricatorCountdownEditor' => 'PhabricatorApplicationTransactionEditor',
+    'PhabricatorCountdownEpochTransaction' => 'PhabricatorCountdownTransactionType',
     'PhabricatorCountdownListController' => 'PhabricatorCountdownController',
     'PhabricatorCountdownMailReceiver' => 'PhabricatorObjectMailReceiver',
     'PhabricatorCountdownQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
@@ -7526,9 +7532,11 @@ phutil_register_library_map(array(
     'PhabricatorCountdownReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'PhabricatorCountdownSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'PhabricatorCountdownSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'PhabricatorCountdownTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorCountdownTitleTransaction' => 'PhabricatorCountdownTransactionType',
+    'PhabricatorCountdownTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorCountdownTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorCountdownTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorCountdownTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorCountdownView' => 'AphrontView',
     'PhabricatorCountdownViewController' => 'PhabricatorCountdownController',
     'PhabricatorCursorPagedPolicyAwareQuery' => 'PhabricatorPolicyAwareQuery',

src/applications/countdown/editor/PhabricatorCountdownEditEngine.php

@@ -81,7 +81,8 @@ final class PhabricatorCountdownEditEngine
         ->setKey('name')
         ->setLabel(pht('Name'))
         ->setIsRequired(true)
-        ->setTransactionType(PhabricatorCountdownTransaction::TYPE_TITLE)
+        ->setTransactionType(
+            PhabricatorCountdownTitleTransaction::TRANSACTIONTYPE)
         ->setDescription(pht('The countdown name.'))
         ->setConduitDescription(pht('Rename the countdown.'))
         ->setConduitTypeDescription(pht('New countdown name.'))
@@ -89,7 +90,8 @@ final class PhabricatorCountdownEditEngine
       id(new PhabricatorEpochEditField())
         ->setKey('epoch')
         ->setLabel(pht('End Date'))
-        ->setTransactionType(PhabricatorCountdownTransaction::TYPE_EPOCH)
+        ->setTransactionType(
+            PhabricatorCountdownEpochTransaction::TRANSACTIONTYPE)
         ->setDescription(pht('Date when the countdown ends.'))
         ->setConduitDescription(pht('Change the end date of the countdown.'))
         ->setConduitTypeDescription(pht('New countdown end date.'))
@@ -97,7 +99,8 @@ final class PhabricatorCountdownEditEngine
       id(new PhabricatorRemarkupEditField())
         ->setKey('description')
         ->setLabel(pht('Description'))
-        ->setTransactionType(PhabricatorCountdownTransaction::TYPE_DESCRIPTION)
+        ->setTransactionType(
+            PhabricatorCountdownDescriptionTransaction::TRANSACTIONTYPE)
         ->setDescription(pht('Description of the countdown.'))
         ->setConduitDescription(pht('Change the countdown description.'))
         ->setConduitTypeDescription(pht('New description.'))

src/applications/countdown/editor/PhabricatorCountdownEditor.php

@@ -14,10 +14,6 @@ final class PhabricatorCountdownEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhabricatorCountdownTransaction::TYPE_TITLE;
-    $types[] = PhabricatorCountdownTransaction::TYPE_EPOCH;
-    $types[] = PhabricatorCountdownTransaction::TYPE_DESCRIPTION;
-
     $types[] = PhabricatorTransactions::TYPE_EDGE;
     $types[] = PhabricatorTransactions::TYPE_SPACE;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
@@ -27,126 +23,6 @@ final class PhabricatorCountdownEditor
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorCountdownTransaction::TYPE_TITLE:
-        return $object->getTitle();
-      case PhabricatorCountdownTransaction::TYPE_DESCRIPTION:
-        return $object->getDescription();
-      case PhabricatorCountdownTransaction::TYPE_EPOCH:
-        return $object->getEpoch();
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorCountdownTransaction::TYPE_TITLE:
-        return $xaction->getNewValue();
-      case PhabricatorCountdownTransaction::TYPE_DESCRIPTION:
-        return $xaction->getNewValue();
-      case PhabricatorCountdownTransaction::TYPE_EPOCH:
-        return $xaction->getNewValue()->getEpoch();
-    }
-
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $type = $xaction->getTransactionType();
-    switch ($type) {
-      case PhabricatorCountdownTransaction::TYPE_TITLE:
-        $object->setTitle($xaction->getNewValue());
-        return;
-      case PhabricatorCountdownTransaction::TYPE_DESCRIPTION:
-        $object->setDescription($xaction->getNewValue());
-        return;
-      case PhabricatorCountdownTransaction::TYPE_EPOCH:
-        $object->setEpoch($xaction->getNewValue());
-        return;
-    }
-
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $type = $xaction->getTransactionType();
-    switch ($type) {
-      case PhabricatorCountdownTransaction::TYPE_TITLE:
-        return;
-      case PhabricatorCountdownTransaction::TYPE_DESCRIPTION:
-        return;
-      case PhabricatorCountdownTransaction::TYPE_EPOCH:
-        return;
-    }
-
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorCountdownTransaction::TYPE_TITLE:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getTitle(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('You must give the countdown a name.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-      break;
-      case PhabricatorCountdownTransaction::TYPE_EPOCH:
-        if (!$object->getEpoch() && !$xactions) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('You must give the countdown an end date.'),
-            null);
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-
-        foreach ($xactions as $xaction) {
-          $value = $xaction->getNewValue();
-          if (!$value->isValid()) {
-            $error = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht('You must give the countdown a valid end date.'),
-              $xaction);
-            $errors[] = $error;
-          }
-        }
-      break;
-    }
-
-    return $errors;
-  }
-
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {

src/applications/countdown/storage/PhabricatorCountdownTransaction.php

@@ -1,11 +1,7 @@
 <?php
 
 final class PhabricatorCountdownTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
-
-  const TYPE_TITLE = 'countdown:title';
-  const TYPE_EPOCH = 'countdown:epoch';
-  const TYPE_DESCRIPTION = 'countdown:description';
 
   const MAILTAG_DETAILS = 'countdown:details';
   const MAILTAG_COMMENT = 'countdown:comment';
@@ -23,62 +19,8 @@ final class PhabricatorCountdownTransaction
     return new PhabricatorCountdownTransactionComment();
   }
 
-  public function getTitle() {
+  public function getBaseTransactionClass() {
-    $author_phid = $this->getAuthorPHID();
+    return 'PhabricatorCountdownTransactionType';
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_TITLE:
-        return pht(
-          '%s renamed this countdown from "%s" to "%s".',
-          $this->renderHandleLink($author_phid),
-          $old,
-          $new);
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          '%s edited the description of this countdown.',
-          $this->renderHandleLink($author_phid));
-      case self::TYPE_EPOCH:
-        return pht(
-          '%s updated this countdown to end on %s.',
-          $this->renderHandleLink($author_phid),
-          phabricator_datetime($new, $this->getViewer()));
-    }
-
-    return parent::getTitle();
-  }
-
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_TITLE:
-        return pht(
-          '%s renamed %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          '%s edited the description of %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_EPOCH:
-        return pht(
-          '%s edited the end date of %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-    }
-
-    return parent::getTitleForFeed();
   }
 
   public function getMailTags() {
@@ -88,9 +30,9 @@ final class PhabricatorCountdownTransaction
       case PhabricatorTransactions::TYPE_COMMENT:
         $tags[] = self::MAILTAG_COMMENT;
         break;
-      case self::TYPE_TITLE:
+      case PhabricatorCountdownTitleTransaction::TRANSACTIONTYPE:
-      case self::TYPE_EPOCH:
+      case PhabricatorCountdownEpochTransaction::TRANSACTIONTYPE:
-      case self::TYPE_DESCRIPTION:
+      case PhabricatorCountdownDescriptionTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_DETAILS;
         break;
       default:
@@ -100,21 +42,4 @@ final class PhabricatorCountdownTransaction
 
     return $tags;
   }
-
-  public function hasChangeDetails() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($this->getOldValue() !== null);
-    }
-
-    return parent::hasChangeDetails();
-  }
-
-  public function renderChangeDetails(PhabricatorUser $viewer) {
-    return $this->renderTextCorpusChangeDetails(
-      $viewer,
-      $this->getOldValue(),
-      $this->getNewValue());
-  }
-
 }

src/applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php

@@ -0,0 +1,55 @@
+<?php
+
+final class PhabricatorCountdownDescriptionTransaction
+  extends PhabricatorCountdownTransactionType {
+
+  const TRANSACTIONTYPE = 'countdown:description';
+
+  public function generateOldValue($object) {
+    return $object->getDescription();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setDescription($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the countdown description.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the countdown description for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO COUNTDOWN DESCRIPTION');
+  }
+
+  public function newChangeDetailView() {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+      ->setViewer($viewer)
+      ->setOldText($this->getOldValue())
+      ->setNewText($this->getNewValue());
+  }
+
+  public function newRemarkupChanges() {
+    $changes = array();
+
+    $changes[] = $this->newRemarkupChange()
+      ->setOldValue($this->getOldValue())
+      ->setNewValue($this->getNewValue());
+
+    return $changes;
+  }
+}

src/applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php

@@ -0,0 +1,58 @@
+<?php
+
+final class PhabricatorCountdownEpochTransaction
+  extends PhabricatorCountdownTransactionType {
+
+  const TRANSACTIONTYPE = 'countdown:epoch';
+
+  public function generateOldValue($object) {
+    return (int)$object->getEpoch();
+  }
+
+  public function generateNewValue($object, $value) {
+    return $value->newPhutilDateTime()
+      ->newAbsoluteDateTime()
+      ->getEpoch();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setEpoch($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the countdown end from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldDate(),
+      $this->renderNewDate());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the countdown end for %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldDate(),
+      $this->renderNewDate());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if (!$object->getEpoch() && !$xactions) {
+      $errors[] = $this->newRequiredError(
+        pht('You must give the countdown an end date.'));
+      return $errors;
+    }
+
+    foreach ($xactions as $xaction) {
+      $value = $xaction->getNewValue();
+      if (!$value->isValid()) {
+        $errors[] = $this->newInvalidError(
+          pht('You must give the countdown an end date.'));
+      }
+    }
+
+    return $errors;
+  }
+}

src/applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php

@@ -0,0 +1,54 @@
+<?php
+
+final class PhabricatorCountdownTitleTransaction
+  extends PhabricatorCountdownTransactionType {
+
+  const TRANSACTIONTYPE = 'countdown:title';
+
+  public function generateOldValue($object) {
+    return $object->getTitle();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setTitle($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the title for this countdown from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the title for this countdown from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getTitle(), $xactions)) {
+      $errors[] = $this->newRequiredError(pht('Countdowns must have a title.'));
+    }
+
+    $max_length = $object->getColumnMaximumByteLength('title');
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Countdown titles must not be longer than %s character(s).',
+            new PhutilNumber($max_length)));
+      }
+    }
+
+    return $errors;
+  }
+
+}

src/applications/countdown/xaction/PhabricatorCountdownTransactionType.php

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