Update Phriction Delete to modular transactions

Summary: Ref T12625. Updates to modular transactions

Test Plan: Delete a document, restore a document, see feed story.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12625

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

src/__phutil_library_map__.php

@@ -4635,6 +4635,7 @@ phutil_register_library_map(array(
     'PhrictionDocumentAuthorHeraldField' => 'applications/phriction/herald/PhrictionDocumentAuthorHeraldField.php',
     'PhrictionDocumentContentHeraldField' => 'applications/phriction/herald/PhrictionDocumentContentHeraldField.php',
     'PhrictionDocumentController' => 'applications/phriction/controller/PhrictionDocumentController.php',
+    'PhrictionDocumentDeleteTransaction' => 'applications/phriction/xaction/PhrictionDocumentDeleteTransaction.php',
     'PhrictionDocumentFulltextEngine' => 'applications/phriction/search/PhrictionDocumentFulltextEngine.php',
     'PhrictionDocumentHeraldAdapter' => 'applications/phriction/herald/PhrictionDocumentHeraldAdapter.php',
     'PhrictionDocumentHeraldField' => 'applications/phriction/herald/PhrictionDocumentHeraldField.php',
@@ -10297,6 +10298,7 @@ phutil_register_library_map(array(
     'PhrictionDocumentAuthorHeraldField' => 'PhrictionDocumentHeraldField',
     'PhrictionDocumentContentHeraldField' => 'PhrictionDocumentHeraldField',
     'PhrictionDocumentController' => 'PhrictionController',
+    'PhrictionDocumentDeleteTransaction' => 'PhrictionDocumentTransactionType',
     'PhrictionDocumentFulltextEngine' => 'PhabricatorFulltextEngine',
     'PhrictionDocumentHeraldAdapter' => 'HeraldAdapter',
     'PhrictionDocumentHeraldField' => 'HeraldField',

src/applications/phriction/controller/PhrictionDeleteController.php

@@ -26,7 +26,8 @@ final class PhrictionDeleteController extends PhrictionController {
     if ($request->isFormPost()) {
         $xactions = array();
         $xactions[] = id(new PhrictionTransaction())
-          ->setTransactionType(PhrictionTransaction::TYPE_DELETE)
+          ->setTransactionType(
+            PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE)
           ->setNewValue(true);
 
         $editor = id(new PhrictionTransactionEditor())

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -86,7 +86,6 @@ final class PhrictionTransactionEditor
     $types = parent::getTransactionTypes();
 
     $types[] = PhrictionTransaction::TYPE_CONTENT;
-    $types[] = PhrictionTransaction::TYPE_DELETE;
     $types[] = PhrictionTransaction::TYPE_MOVE_AWAY;
 
     $types[] = PhabricatorTransactions::TYPE_EDGE;
@@ -107,7 +106,6 @@ final class PhrictionTransactionEditor
           return null;
         }
         return $this->getOldContent()->getContent();
-      case PhrictionTransaction::TYPE_DELETE:
       case PhrictionTransaction::TYPE_MOVE_AWAY:
         return null;
     }
@@ -119,7 +117,6 @@ final class PhrictionTransactionEditor
 
     switch ($xaction->getTransactionType()) {
       case PhrictionTransaction::TYPE_CONTENT:
-      case PhrictionTransaction::TYPE_DELETE:
         return $xaction->getNewValue();
       case PhrictionTransaction::TYPE_MOVE_AWAY:
         $document = $xaction->getNewValue();
@@ -141,7 +138,7 @@ final class PhrictionTransactionEditor
       switch ($xaction->getTransactionType()) {
       case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
       case PhrictionTransaction::TYPE_CONTENT:
-      case PhrictionTransaction::TYPE_DELETE:
+      case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:
       case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
       case PhrictionTransaction::TYPE_MOVE_AWAY:
         return true;
@@ -169,9 +166,6 @@ final class PhrictionTransactionEditor
       case PhrictionTransaction::TYPE_MOVE_AWAY:
         $object->setStatus(PhrictionDocumentStatus::STATUS_MOVED);
         return;
-      case PhrictionTransaction::TYPE_DELETE:
-        $object->setStatus(PhrictionDocumentStatus::STATUS_DELETED);
-        return;
     }
   }
 
@@ -188,7 +182,8 @@ final class PhrictionTransactionEditor
         $content = $xaction->getNewValue();
         if ($content === '') {
           $xactions[] = id(new PhrictionTransaction())
-            ->setTransactionType(PhrictionTransaction::TYPE_DELETE)
+            ->setTransactionType(
+              PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE)
             ->setNewValue(true)
             ->setMetadataValue('contentDelete', true);
         }
@@ -218,11 +213,6 @@ final class PhrictionTransactionEditor
       case PhrictionTransaction::TYPE_CONTENT:
         $this->getNewContent()->setContent($xaction->getNewValue());
         break;
-      case PhrictionTransaction::TYPE_DELETE:
-        $this->getNewContent()->setContent('');
-        $this->getNewContent()->setChangeType(
-          PhrictionChangeType::CHANGE_DELETE);
-        break;
       case PhrictionTransaction::TYPE_MOVE_AWAY:
         $dict = $xaction->getNewValue();
         $this->getNewContent()->setContent('');
@@ -245,7 +235,7 @@ final class PhrictionTransactionEditor
         case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
         case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
         case PhrictionTransaction::TYPE_CONTENT:
-        case PhrictionTransaction::TYPE_DELETE:
+        case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:
         case PhrictionTransaction::TYPE_MOVE_AWAY:
           $save_content = true;
           break;
@@ -536,35 +526,6 @@ final class PhrictionTransactionEditor
           }
           break;
 
-        case PhrictionTransaction::TYPE_DELETE:
-          switch ($object->getStatus()) {
-            case PhrictionDocumentStatus::STATUS_DELETED:
-              if ($xaction->getMetadataValue('contentDelete')) {
-                $e_text = pht(
-                  'This document is already deleted. You must specify '.
-                  'content to re-create the document and make further edits.');
-              } else {
-                $e_text = pht(
-                  'An already deleted document can not be deleted.');
-              }
-              break;
-            case PhrictionDocumentStatus::STATUS_MOVED:
-              $e_text = pht('A moved document can not be deleted.');
-              break;
-            case PhrictionDocumentStatus::STATUS_STUB:
-              $e_text = pht('A stub document can not be deleted.');
-              break;
-            default:
-              break 2;
-          }
-
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Can not delete document.'),
-            $e_text,
-            $xaction);
-          $errors[] = $error;
-          break;
       }
     }
 

src/applications/phriction/storage/PhrictionTransaction.php

@@ -4,7 +4,6 @@ final class PhrictionTransaction
   extends PhabricatorModularTransaction {
 
   const TYPE_CONTENT = 'content';
-  const TYPE_DELETE  = 'delete';
   const TYPE_MOVE_AWAY = 'move-away';
 
   const MAILTAG_TITLE       = 'phriction-title';
@@ -99,8 +98,6 @@ final class PhrictionTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_CONTENT:
         return 1.3;
-      case self::TYPE_DELETE:
-        return 1.5;
       case self::TYPE_MOVE_AWAY:
         return 1.0;
     }
@@ -115,8 +112,6 @@ final class PhrictionTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_CONTENT:
         return pht('Edited');
-      case self::TYPE_DELETE:
-        return pht('Deleted');
       case self::TYPE_MOVE_AWAY:
         return pht('Moved Away');
     }
@@ -131,8 +126,6 @@ final class PhrictionTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_CONTENT:
         return 'fa-pencil';
-      case self::TYPE_DELETE:
-        return 'fa-times';
       case self::TYPE_MOVE_AWAY:
         return 'fa-arrows';
     }
@@ -153,11 +146,6 @@ final class PhrictionTransaction
           '%s edited the document content.',
           $this->renderHandleLink($author_phid));
 
-      case self::TYPE_DELETE:
-        return pht(
-          '%s deleted this document.',
-          $this->renderHandleLink($author_phid));
-
       case self::TYPE_MOVE_AWAY:
         return pht(
           '%s moved this document to %s',
@@ -184,12 +172,6 @@ final class PhrictionTransaction
           $this->renderHandleLink($author_phid),
           $this->renderHandleLink($object_phid));
 
-      case self::TYPE_DELETE:
-        return pht(
-          '%s deleted %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-
     }
     return parent::getTitleForFeed();
   }
@@ -218,7 +200,7 @@ final class PhrictionTransaction
       case self::TYPE_CONTENT:
         $tags[] = self::MAILTAG_CONTENT;
         break;
-      case self::TYPE_DELETE:
+      case PhrictionDocumentDeleteTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_DELETE;
         break;
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:

src/applications/phriction/xaction/PhrictionDocumentDeleteTransaction.php

@@ -0,0 +1,86 @@
+<?php
+
+final class PhrictionDocumentDeleteTransaction
+  extends PhrictionDocumentTransactionType {
+
+  const TRANSACTIONTYPE = 'delete';
+
+  public function generateOldValue($object) {
+    return null;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setStatus(PhrictionDocumentStatus::STATUS_DELETED);
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $this->getEditor()->getNewContent()->setContent('');
+    $this->getEditor()->getNewContent()->setChangeType(
+      PhrictionChangeType::CHANGE_DELETE);
+  }
+
+  public function getActionStrength() {
+    return 1.5;
+  }
+
+  public function getActionName() {
+    return pht('Deleted');
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s deleted this document.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s deleted %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $e_text = null;
+    foreach ($xactions as $xaction) {
+      switch ($object->getStatus()) {
+        case PhrictionDocumentStatus::STATUS_DELETED:
+          if ($xaction->getMetadataValue('contentDelete')) {
+            $e_text = pht(
+              'This document is already deleted. You must specify '.
+              'content to re-create the document and make further edits.');
+          } else {
+            $e_text = pht(
+              'An already deleted document can not be deleted.');
+          }
+          break;
+        case PhrictionDocumentStatus::STATUS_MOVED:
+          $e_text = pht('A moved document can not be deleted.');
+          break;
+        case PhrictionDocumentStatus::STATUS_STUB:
+          $e_text = pht('A stub document can not be deleted.');
+          break;
+        default:
+          break;
+      }
+
+      if ($e_text !== null) {
+        $errors[] = $this->newInvalidError($e_text);
+      }
+
+    }
+
+    return $errors;
+  }
+
+  public function getIcon() {
+    return 'fa-trash-o';
+  }
+
+  public function getColor() {
+    return 'red';
+  }
+
+}