Move Phriction Title transaction to Modular Transactions

Summary: Ref T12625. Moves TYPE_TITLE to modular transaction.

Test Plan: New Document, Edit Document, test validation, verify feed stories.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12625

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

src/__phutil_library_map__.php

@@ -4621,6 +4621,8 @@ phutil_register_library_map(array(
     'PhrictionDocumentQuery' => 'applications/phriction/query/PhrictionDocumentQuery.php',
     'PhrictionDocumentStatus' => 'applications/phriction/constants/PhrictionDocumentStatus.php',
     'PhrictionDocumentTitleHeraldField' => 'applications/phriction/herald/PhrictionDocumentTitleHeraldField.php',
+    'PhrictionDocumentTitleTransaction' => 'applications/phriction/xaction/PhrictionDocumentTitleTransaction.php',
+    'PhrictionDocumentTransactionType' => 'applications/phriction/xaction/PhrictionDocumentTransactionType.php',
     'PhrictionEditConduitAPIMethod' => 'applications/phriction/conduit/PhrictionEditConduitAPIMethod.php',
     'PhrictionEditController' => 'applications/phriction/controller/PhrictionEditController.php',
     'PhrictionHistoryConduitAPIMethod' => 'applications/phriction/conduit/PhrictionHistoryConduitAPIMethod.php',
@@ -10257,6 +10259,8 @@ phutil_register_library_map(array(
     'PhrictionDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhrictionDocumentStatus' => 'PhrictionConstants',
     'PhrictionDocumentTitleHeraldField' => 'PhrictionDocumentHeraldField',
+    'PhrictionDocumentTitleTransaction' => 'PhrictionDocumentTransactionType',
+    'PhrictionDocumentTransactionType' => 'PhabricatorModularTransactionType',
     'PhrictionEditConduitAPIMethod' => 'PhrictionConduitAPIMethod',
     'PhrictionEditController' => 'PhrictionController',
     'PhrictionHistoryConduitAPIMethod' => 'PhrictionConduitAPIMethod',
@@ -10270,7 +10274,7 @@ phutil_register_library_map(array(
     'PhrictionReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'PhrictionSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'PhrictionSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'PhrictionTransaction' => 'PhabricatorApplicationTransaction',
+    'PhrictionTransaction' => 'PhabricatorModularTransaction',
     'PhrictionTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhrictionTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhrictionTransactionQuery' => 'PhabricatorApplicationTransactionQuery',

src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php

@@ -47,7 +47,7 @@ final class PhrictionCreateConduitAPIMethod extends PhrictionConduitAPIMethod {
 
     $xactions = array();
     $xactions[] = id(new PhrictionTransaction())
-      ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+      ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
       ->setNewValue($request->getValue('title'));
     $xactions[] = id(new PhrictionTransaction())
       ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)

src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php

@@ -42,7 +42,7 @@ final class PhrictionEditConduitAPIMethod extends PhrictionConduitAPIMethod {
 
     $xactions = array();
     $xactions[] = id(new PhrictionTransaction())
-      ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+      ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
       ->setNewValue($request->getValue('title'));
     $xactions[] = id(new PhrictionTransaction())
       ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)

src/applications/phriction/controller/PhrictionEditController.php

@@ -133,7 +133,7 @@ final class PhrictionEditController
 
       $xactions = array();
       $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+        ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
         ->setNewValue($title);
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)
@@ -174,7 +174,8 @@ final class PhrictionEditController
       } catch (PhabricatorApplicationTransactionValidationException $ex) {
         $validation_exception = $ex;
         $e_title = nonempty(
-          $ex->getShortMessage(PhrictionTransaction::TYPE_TITLE),
+          $ex->getShortMessage(
+            PhrictionDocumentTitleTransaction::TRANSACTIONTYPE),
           true);
         $e_content = nonempty(
           $ex->getShortMessage(PhrictionTransaction::TYPE_CONTENT),

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -29,7 +29,7 @@ final class PhrictionTransactionEditor
     return $this;
   }
 
-  private function getOldContent() {
+  public function getOldContent() {
     return $this->oldContent;
   }
 
@@ -38,7 +38,7 @@ final class PhrictionTransactionEditor
     return $this;
   }
 
-  private function getNewContent() {
+  public function getNewContent() {
     return $this->newContent;
   }
 
@@ -80,7 +80,6 @@ final class PhrictionTransactionEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhrictionTransaction::TYPE_TITLE;
     $types[] = PhrictionTransaction::TYPE_CONTENT;
     $types[] = PhrictionTransaction::TYPE_DELETE;
     $types[] = PhrictionTransaction::TYPE_MOVE_TO;
@@ -99,11 +98,6 @@ final class PhrictionTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_TITLE:
-        if ($this->getIsNewObject()) {
-          return null;
-        }
-        return $this->getOldContent()->getTitle();
       case PhrictionTransaction::TYPE_CONTENT:
         if ($this->getIsNewObject()) {
           return null;
@@ -121,7 +115,6 @@ final class PhrictionTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_TITLE:
       case PhrictionTransaction::TYPE_CONTENT:
       case PhrictionTransaction::TYPE_DELETE:
         return $xaction->getNewValue();
@@ -154,7 +147,7 @@ final class PhrictionTransactionEditor
 
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_TITLE:
+      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
       case PhrictionTransaction::TYPE_CONTENT:
       case PhrictionTransaction::TYPE_DELETE:
       case PhrictionTransaction::TYPE_MOVE_TO:
@@ -178,7 +171,6 @@ final class PhrictionTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_TITLE:
       case PhrictionTransaction::TYPE_CONTENT:
       case PhrictionTransaction::TYPE_MOVE_TO:
         $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
@@ -232,9 +224,6 @@ final class PhrictionTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_TITLE:
-        $this->getNewContent()->setTitle($xaction->getNewValue());
-        break;
       case PhrictionTransaction::TYPE_CONTENT:
         $this->getNewContent()->setContent($xaction->getNewValue());
         break;
@@ -270,7 +259,7 @@ final class PhrictionTransactionEditor
     $save_content = false;
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case PhrictionTransaction::TYPE_TITLE:
+        case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         case PhrictionTransaction::TYPE_CONTENT:
         case PhrictionTransaction::TYPE_DELETE:
         case PhrictionTransaction::TYPE_MOVE_AWAY:
@@ -312,7 +301,8 @@ final class PhrictionTransactionEditor
               $slug);
             $stub_xactions = array();
             $stub_xactions[] = id(new PhrictionTransaction())
-              ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+              ->setTransactionType(
+                PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
               ->setNewValue(PhabricatorSlug::getDefaultTitle($slug))
               ->setMetadataValue('stub:create:phid', $object->getPHID());
             $stub_xactions[] = id(new PhrictionTransaction())
@@ -477,30 +467,6 @@ final class PhrictionTransactionEditor
 
     foreach ($xactions as $xaction) {
       switch ($type) {
-        case PhrictionTransaction::TYPE_TITLE:
-          $title = $object->getContent()->getTitle();
-          $missing = $this->validateIsEmptyTextField(
-            $title,
-            $xactions);
-
-          if ($missing) {
-            $error = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Required'),
-              pht('Document title is required.'),
-              nonempty(last($xactions), null));
-
-            $error->setIsMissingFieldError(true);
-            $errors[] = $error;
-          } else if ($this->getProcessContentVersionError()) {
-            $error = $this->validateContentVersion($object, $type, $xaction);
-            if ($error) {
-              $this->setProcessContentVersionError(false);
-              $errors[] = $error;
-            }
-          }
-          break;
-
         case PhrictionTransaction::TYPE_CONTENT:
           if ($xaction->getMetadataValue('stub:create:phid')) {
             continue;

src/applications/phriction/storage/PhrictionTransaction.php

@@ -1,9 +1,8 @@
 <?php
 
 final class PhrictionTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
 
-  const TYPE_TITLE = 'title';
   const TYPE_CONTENT = 'content';
   const TYPE_DELETE  = 'delete';
   const TYPE_MOVE_TO = 'move-to';
@@ -27,6 +26,10 @@ final class PhrictionTransaction
     return new PhrictionTransactionComment();
   }
 
+  public function getBaseTransactionClass() {
+    return 'PhrictionDocumentTransactionType';
+  }
+
   public function getRequiredHandlePHIDs() {
     $phids = parent::getRequiredHandlePHIDs();
     $new = $this->getNewValue();
@@ -35,14 +38,13 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_AWAY:
         $phids[] = $new['phid'];
         break;
-      case self::TYPE_TITLE:
+      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         if ($this->getMetadataValue('stub:create:phid')) {
           $phids[] = $this->getMetadataValue('stub:create:phid');
         }
         break;
     }
 
-
     return $phids;
   }
 
@@ -77,7 +79,7 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_TO:
       case self::TYPE_MOVE_AWAY:
         return true;
-      case self::TYPE_TITLE:
+      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         return $this->getMetadataValue('stub:create:phid', false);
     }
     return parent::shouldHideForMail($xactions);
@@ -88,7 +90,7 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_TO:
       case self::TYPE_MOVE_AWAY:
         return true;
-      case self::TYPE_TITLE:
+      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         return $this->getMetadataValue('stub:create:phid', false);
     }
     return parent::shouldHideForFeed();
@@ -96,8 +98,6 @@ final class PhrictionTransaction
 
   public function getActionStrength() {
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
-        return 1.4;
       case self::TYPE_CONTENT:
         return 1.3;
       case self::TYPE_DELETE:
@@ -115,29 +115,14 @@ final class PhrictionTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
-        if ($old === null) {
-          if ($this->getMetadataValue('stub:create:phid')) {
-            return pht('Stubbed');
-          } else {
-            return pht('Created');
-          }
-        }
-
-        return pht('Retitled');
-
       case self::TYPE_CONTENT:
         return pht('Edited');
-
       case self::TYPE_DELETE:
         return pht('Deleted');
-
       case self::TYPE_MOVE_TO:
         return pht('Moved');
-
       case self::TYPE_MOVE_AWAY:
         return pht('Moved Away');
-
     }
 
     return parent::getActionName();
@@ -148,7 +133,6 @@ final class PhrictionTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
       case self::TYPE_CONTENT:
         return 'fa-pencil';
       case self::TYPE_DELETE:
@@ -169,26 +153,6 @@ final class PhrictionTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
-        if ($old === null) {
-          if ($this->getMetadataValue('stub:create:phid')) {
-            return pht(
-              '%s stubbed out this document when creating %s.',
-              $this->renderHandleLink($author_phid),
-              $this->renderHandleLink(
-                $this->getMetadataValue('stub:create:phid')));
-          } else {
-            return pht(
-              '%s created this document.',
-              $this->renderHandleLink($author_phid));
-          }
-        }
-        return pht(
-          '%s changed the title from "%s" to "%s".',
-          $this->renderHandleLink($author_phid),
-          $old,
-          $new);
-
       case self::TYPE_CONTENT:
         return pht(
           '%s edited the document content.',
@@ -224,20 +188,6 @@ final class PhrictionTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
-        if ($old === null) {
-          return pht(
-            '%s created %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid));
-        }
-
-        return pht(
-          '%s renamed %s from "%s" to "%s".',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid),
-          $old,
-          $new);
 
       case self::TYPE_CONTENT:
         return pht(
@@ -273,7 +223,7 @@ final class PhrictionTransaction
   public function getMailTags() {
     $tags = array();
     switch ($this->getTransactionType()) {
-      case self::TYPE_TITLE:
+      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_TITLE;
         break;
       case self::TYPE_CONTENT:

src/applications/phriction/xaction/PhrictionDocumentTitleTransaction.php

@@ -0,0 +1,94 @@
+<?php
+
+final class PhrictionDocumentTitleTransaction
+  extends PhrictionDocumentTransactionType {
+
+  const TRANSACTIONTYPE = 'title';
+
+  public function generateOldValue($object) {
+    if ($this->isNewObject()) {
+      return null;
+    }
+    return $this->getEditor()->getOldContent()->getTitle();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
+    $this->getEditor()->getNewContent()->setTitle($value);
+  }
+
+  public function getActionStrength() {
+    return 1.4;
+  }
+
+  public function getActionName() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if ($old === null) {
+      if ($this->getMetadataValue('stub:create:phid')) {
+        return pht('Stubbed');
+      } else {
+        return pht('Created');
+      }
+    }
+    return pht('Retitled');
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if ($old === null) {
+      if ($this->getMetadataValue('stub:create:phid')) {
+        return pht(
+          '%s stubbed out this document when creating %s.',
+          $this->renderAuthor(),
+          $this->renderHandleLink(
+            $this->getMetadataValue('stub:create:phid')));
+      } else {
+        return pht(
+          '%s created this document.',
+          $this->renderAuthor());
+      }
+    }
+
+    return pht(
+      '%s changed the title from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if ($old === null) {
+      return pht(
+        '%s created %s.',
+        $this->renderAuthor(),
+        $this->renderObject());
+    }
+
+    return pht(
+      '%s renamed %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $title = $object->getContent()->getTitle();
+    if ($this->isEmptyTextTransaction($title, $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Documents must have a title.'));
+    }
+
+    return $errors;
+  }
+
+}

src/applications/phriction/xaction/PhrictionDocumentTransactionType.php

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

src/applications/transactions/storage/PhabricatorModularTransaction.php

@@ -160,7 +160,7 @@ abstract class PhabricatorModularTransaction
     return parent::attachViewer($viewer);
   }
 
-  final public function hasChangeDetails() {
+  /* final */ public function hasChangeDetails() {
     if ($this->getTransactionImplementation()->hasChangeDetailView()) {
       return true;
     }
@@ -168,7 +168,7 @@ abstract class PhabricatorModularTransaction
     return parent::hasChangeDetails();
   }
 
-  final public function renderChangeDetails(PhabricatorUser $viewer) {
+  /* final */ public function renderChangeDetails(PhabricatorUser $viewer) {
     $impl = $this->getTransactionImplementation();
     $impl->setViewer($viewer);
     $view = $impl->newChangeDetailView();