Update PhrictionContent for modular transactions

Summary: Fixes T12625. Moves TYPE_CONTENT in Phriction over to modular transactions.

Test Plan: Edit some documents.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12625

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

src/__phutil_library_map__.php

@@ -4637,6 +4637,7 @@ phutil_register_library_map(array(
     'PhrictionDocument' => 'applications/phriction/storage/PhrictionDocument.php',
     'PhrictionDocumentAuthorHeraldField' => 'applications/phriction/herald/PhrictionDocumentAuthorHeraldField.php',
     'PhrictionDocumentContentHeraldField' => 'applications/phriction/herald/PhrictionDocumentContentHeraldField.php',
+    'PhrictionDocumentContentTransaction' => 'applications/phriction/xaction/PhrictionDocumentContentTransaction.php',
     'PhrictionDocumentController' => 'applications/phriction/controller/PhrictionDocumentController.php',
     'PhrictionDocumentDeleteTransaction' => 'applications/phriction/xaction/PhrictionDocumentDeleteTransaction.php',
     'PhrictionDocumentFulltextEngine' => 'applications/phriction/search/PhrictionDocumentFulltextEngine.php',
@@ -10304,6 +10305,7 @@ phutil_register_library_map(array(
     ),
     'PhrictionDocumentAuthorHeraldField' => 'PhrictionDocumentHeraldField',
     'PhrictionDocumentContentHeraldField' => 'PhrictionDocumentHeraldField',
+    'PhrictionDocumentContentTransaction' => 'PhrictionDocumentTransactionType',
     'PhrictionDocumentController' => 'PhrictionController',
     'PhrictionDocumentDeleteTransaction' => 'PhrictionDocumentTransactionType',
     'PhrictionDocumentFulltextEngine' => 'PhabricatorFulltextEngine',

src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php

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

src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php

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

src/applications/phriction/controller/PhrictionEditController.php

@@ -136,7 +136,8 @@ final class PhrictionEditController
         ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
         ->setNewValue($title);
       $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)
+        ->setTransactionType(
+          PhrictionDocumentContentTransaction::TRANSACTIONTYPE)
         ->setNewValue($content_text);
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
@@ -178,7 +179,8 @@ final class PhrictionEditController
             PhrictionDocumentTitleTransaction::TRANSACTIONTYPE),
           true);
         $e_content = nonempty(
-          $ex->getShortMessage(PhrictionTransaction::TYPE_CONTENT),
+          $ex->getShortMessage(
+            PhrictionDocumentContentTransaction::TRANSACTIONTYPE),
           true);
 
         // if we're not supposed to process the content version error, then

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -85,8 +85,6 @@ final class PhrictionTransactionEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhrictionTransaction::TYPE_CONTENT;
-
     $types[] = PhabricatorTransactions::TYPE_EDGE;
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
@@ -95,29 +93,6 @@ final class PhrictionTransactionEditor
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_CONTENT:
-        if ($this->getIsNewObject()) {
-          return null;
-        }
-        return $this->getOldContent()->getContent();
-    }
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_CONTENT:
-        return $xaction->getNewValue();
-    }
-  }
-
   protected function shouldApplyInitialEffects(
     PhabricatorLiskDAO $object,
     array $xactions) {
@@ -125,7 +100,7 @@ final class PhrictionTransactionEditor
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
         case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
-      case PhrictionTransaction::TYPE_CONTENT:
+        case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentMoveAwayTransaction::TRANSACTIONTYPE:
@@ -143,24 +118,13 @@ final class PhrictionTransactionEditor
     $this->setNewContent($this->buildNewContentTemplate($object));
   }
 
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_CONTENT:
-        $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
-        return;
-    }
-  }
-
   protected function expandTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
 
     $xactions = parent::expandTransaction($object, $xaction);
     switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_CONTENT:
+      case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
         if ($this->getIsNewObject()) {
           break;
         }
@@ -190,19 +154,6 @@ final class PhrictionTransactionEditor
     return $xactions;
   }
 
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhrictionTransaction::TYPE_CONTENT:
-        $this->getNewContent()->setContent($xaction->getNewValue());
-        break;
-      default:
-        break;
-    }
-  }
-
   protected function applyFinalEffects(
     PhabricatorLiskDAO $object,
     array $xactions) {
@@ -214,7 +165,7 @@ final class PhrictionTransactionEditor
         case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentMoveAwayTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:
-        case PhrictionTransaction::TYPE_CONTENT:
+        case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
           $save_content = true;
           break;
         default:
@@ -257,7 +208,8 @@ final class PhrictionTransactionEditor
               ->setNewValue(PhabricatorSlug::getDefaultTitle($slug))
               ->setMetadataValue('stub:create:phid', $object->getPHID());
             $stub_xactions[] = id(new PhrictionTransaction())
-              ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)
+              ->setTransactionType(
+                PhrictionDocumentContentTransaction::TRANSACTIONTYPE)
               ->setNewValue('')
               ->setMetadataValue('stub:create:phid', $object->getPHID());
             $stub_xactions[] = id(new PhrictionTransaction())
@@ -295,7 +247,7 @@ final class PhrictionTransactionEditor
     // Compute the content diff URI for the publishing phase.
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case PhrictionTransaction::TYPE_CONTENT:
+        case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
           $uri = id(new PhutilURI('/phriction/diff/'.$object->getID().'/'))
             ->alter('l', $this->getOldContent()->getVersion())
             ->alter('r', $this->getNewContent()->getVersion());
@@ -419,29 +371,12 @@ final class PhrictionTransactionEditor
 
     foreach ($xactions as $xaction) {
       switch ($type) {
-        case PhrictionTransaction::TYPE_CONTENT:
+        case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
           if ($xaction->getMetadataValue('stub:create:phid')) {
             continue;
           }
 
-          $missing = false;
+          if ($this->getProcessContentVersionError()) {
-          if ($this->getIsNewObject()) {
-            $content = $object->getContent()->getContent();
-            $missing = $this->validateIsEmptyTextField(
-              $content,
-              $xactions);
-          }
-
-          if ($missing) {
-            $error = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Required'),
-              pht('Document content 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);
@@ -459,7 +394,6 @@ final class PhrictionTransactionEditor
               $errors = array_merge($errors, $ancestry_errors);
             }
           }
-
           break;
 
         case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:

src/applications/phriction/storage/PhrictionTransaction.php

@@ -3,8 +3,6 @@
 final class PhrictionTransaction
   extends PhabricatorModularTransaction {
 
-  const TYPE_CONTENT = 'content';
-
   const MAILTAG_TITLE       = 'phriction-title';
   const MAILTAG_CONTENT     = 'phriction-content';
   const MAILTAG_DELETE      = 'phriction-delete';
@@ -45,32 +43,6 @@ final class PhrictionTransaction
     return $phids;
   }
 
-  public function getRemarkupBlocks() {
-    $blocks = parent::getRemarkupBlocks();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        $blocks[] = $this->getNewValue();
-        break;
-    }
-
-    return $blocks;
-  }
-
-  public function shouldHide() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        if ($this->getOldValue() === null) {
-          return true;
-        } else {
-          return false;
-        }
-        break;
-    }
-
-    return parent::shouldHide();
-  }
-
   public function shouldHideForMail(array $xactions) {
     switch ($this->getTransactionType()) {
       case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
@@ -93,89 +65,6 @@ final class PhrictionTransaction
     return parent::shouldHideForFeed();
   }
 
-  public function getActionStrength() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        return 1.3;
-    }
-
-    return parent::getActionStrength();
-  }
-
-  public function getActionName() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        return pht('Edited');
-    }
-
-    return parent::getActionName();
-  }
-
-  public function getIcon() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        return 'fa-pencil';
-    }
-
-    return parent::getIcon();
-  }
-
-
-  public function getTitle() {
-    $author_phid = $this->getAuthorPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        return pht(
-          '%s edited the document content.',
-          $this->renderHandleLink($author_phid));
-    }
-
-    return parent::getTitle();
-  }
-
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-
-      case self::TYPE_CONTENT:
-        return pht(
-          '%s edited the content of %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-
-    }
-    return parent::getTitleForFeed();
-  }
-
-  public function hasChangeDetails() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_CONTENT:
-        return true;
-    }
-    return parent::hasChangeDetails();
-  }
-
-  public function renderChangeDetails(PhabricatorUser $viewer) {
-    return $this->renderTextCorpusChangeDetails(
-      $viewer,
-      $this->getOldValue(),
-      $this->getNewValue());
-  }
 
   public function getMailTags() {
     $tags = array();
@@ -183,7 +72,7 @@ final class PhrictionTransaction
       case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_TITLE;
         break;
-      case self::TYPE_CONTENT:
+      case PhrictionDocumentContentTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_CONTENT;
         break;
       case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:

src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php

@@ -0,0 +1,95 @@
+<?php
+
+final class PhrictionDocumentContentTransaction
+  extends PhrictionDocumentTransactionType {
+
+  const TRANSACTIONTYPE = 'content';
+
+  public function generateOldValue($object) {
+    if ($this->getEditor()->getIsNewObject()) {
+      return null;
+    }
+    return $object->getContent()->getContent();
+  }
+
+  public function generateNewValue($object, $value) {
+    return $value;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $this->getEditor()->getNewContent()->setContent($value);
+  }
+
+  public function shouldHide() {
+    if ($this->getOldValue() === null) {
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  public function getActionStrength() {
+    return 1.3;
+  }
+
+  public function getActionName() {
+    return pht('Edited');
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s edited the content of this document.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s edited the content of %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO DOCUMENT CONTENT');
+  }
+
+  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;
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $content = $object->getContent()->getContent();
+    if ($this->isEmptyTextTransaction($content, $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Documents must have content.'));
+    }
+
+    return $errors;
+  }
+
+}