Phriction - move "move" to modern editor + transactions

Summary:
Ref T4029. Much like D10756, D10761 this does the bare minimum to get things in there. I have a sticky with "TODOs" about moving the error-checking business logic into the editor in all three cases.

Up next - policy...

Test Plan: moved a document and it worked! verified no feed story. verified both documents involved looked good

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4029

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

src/applications/phriction/controller/PhrictionMoveController.php

@@ -16,6 +16,7 @@ final class PhrictionMoveController extends PhrictionController {
       $document = id(new PhrictionDocumentQuery())
         ->setViewer($user)
         ->withIDs(array($this->id))
+        ->needContent(true)
         ->requireCapabilities(
           array(
             PhabricatorPolicyCapability::CAN_VIEW,
@@ -32,6 +33,7 @@ final class PhrictionMoveController extends PhrictionController {
       $document = id(new PhrictionDocumentQuery())
         ->setViewer($user)
         ->withSlugs(array($slug))
+        ->needContent(true)
         ->requireCapabilities(
           array(
             PhabricatorPolicyCapability::CAN_VIEW,
@@ -74,50 +76,46 @@ final class PhrictionMoveController extends PhrictionController {
       return id(new AphrontDialogResponse())->setDialog($error_dialog);
     }
 
-    $content = id(new PhrictionContent())->load($document->getContentID());
+    $content = $document->getContent();
 
     if ($request->isFormPost() && !count($errors)) {
-      if (!count($errors)) { // First check if the target document exists
 
-        // NOTE: We use the ominpotent user because we can't let users overwrite
+      // NOTE: We use the ominpotent user because we can't let users overwrite
-        // documents even if they can't see them.
+      // documents even if they can't see them.
-        $target_document = id(new PhrictionDocumentQuery())
+      $target_document = id(new PhrictionDocumentQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withSlugs(array($target_slug))
+        ->withSlugs(array($target_slug))
-          ->executeOne();
+        ->needContent(true)
+        ->executeOne();
 
-        // Considering to overwrite existing docs? Nuke this!
+      // Considering to overwrite existing docs? Nuke this!
-        if ($target_document && $target_document->getStatus() ==
+      $exists = PhrictionDocumentStatus::STATUS_EXISTS;
-          PhrictionDocumentStatus::STATUS_EXISTS) {
+      if ($target_document && $target_document->getStatus() == $exists) {
-
+        $errors[] = pht('Can not overwrite existing target document.');
-          $errors[] = pht('Can not overwrite existing target document.');
+        $e_url = pht('Already exists.');
-          $e_url = pht('Already exists.');
-        }
       }
 
-      if (!count($errors)) { // I like to move it, move it!
+      if (!count($errors)) {
-        $from_editor = id(PhrictionDocumentEditor::newForSlug($slug))
+
+        $editor = id(new PhrictionTransactionEditor())
           ->setActor($user)
-          ->setTitle($content->getTitle())
+          ->setContentSourceFromRequest($request)
-          ->setContent($content->getContent())
+          ->setContinueOnNoEffect(true)
-          ->setDescription($content->getDescription());
+          ->setDescription($request->getStr('description'));
 
-        $target_editor = id(PhrictionDocumentEditor::newForSlug(
+        $xactions = array();
-          $target_slug))
+        $xactions[] = id(new PhrictionTransaction())
-          ->setActor($user)
+          ->setTransactionType(PhrictionTransaction::TYPE_MOVE_TO)
-          ->setTitle($content->getTitle())
+          ->setNewValue($document);
-          ->setContent($content->getContent())
+        if (!$target_document) {
-          ->setDescription($content->getDescription());
+          $target_document = PhrictionDocument::initializeNewDocument(
+            $user,
+            $target_slug);
+        }
+        $editor->applyTransactions($target_document, $xactions);
 
-        // Move it!
+        $redir_uri = PhrictionDocument::getSlugURI(
-        $target_editor->moveHere($document->getID(), $document->getPHID());
+          $target_document->getSlug());
-
-        // Retrieve the target doc directly from the editor
-        // No need to load it per Sql again
-        $target_document = $target_editor->getDocument();
-        $from_editor->moveAway($target_document->getID());
-
-        $redir_uri = PhrictionDocument::getSlugURI($target_document->getSlug());
         return id(new AphrontRedirectResponse())->setURI($redir_uri);
       }
     }
@@ -131,21 +129,21 @@ final class PhrictionMoveController extends PhrictionController {
       ->setUser($user)
       ->appendChild(
         id(new AphrontFormStaticControl())
-          ->setLabel(pht('Title'))
+        ->setLabel(pht('Title'))
-          ->setValue($content->getTitle()))
+        ->setValue($content->getTitle()))
       ->appendChild(
         id(new AphrontFormTextControl())
-          ->setLabel(pht('New URI'))
+        ->setLabel(pht('New URI'))
-          ->setValue($target_slug)
+        ->setValue($target_slug)
-          ->setError($e_url)
+        ->setError($e_url)
-          ->setName('new-slug')
+        ->setName('new-slug')
-          ->setCaption(pht('The new location of the document.')))
+        ->setCaption(pht('The new location of the document.')))
       ->appendChild(
         id(new AphrontFormTextControl())
-          ->setLabel(pht('Edit Notes'))
+        ->setLabel(pht('Edit Notes'))
-          ->setValue($content->getDescription())
+        ->setValue($content->getDescription())
-          ->setError(null)
+        ->setError(null)
-          ->setName('description'));
+        ->setName('description'));
 
     $dialog = id(new AphrontDialogView())
       ->setUser($user)

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -6,6 +6,7 @@ final class PhrictionTransactionEditor
   private $description;
   private $oldContent;
   private $newContent;
+  private $moveAwayDocument;
 
   public function setDescription($description) {
     $this->description = $description;
@@ -49,6 +50,8 @@ final class PhrictionTransactionEditor
     $types[] = PhrictionTransaction::TYPE_TITLE;
     $types[] = PhrictionTransaction::TYPE_CONTENT;
     $types[] = PhrictionTransaction::TYPE_DELETE;
+    $types[] = PhrictionTransaction::TYPE_MOVE_TO;
+    $types[] = PhrictionTransaction::TYPE_MOVE_AWAY;
 
     /* TODO
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
@@ -74,6 +77,8 @@ final class PhrictionTransactionEditor
         }
         return $this->getOldContent()->getContent();
       case PhrictionTransaction::TYPE_DELETE:
+      case PhrictionTransaction::TYPE_MOVE_TO:
+      case PhrictionTransaction::TYPE_MOVE_AWAY:
         return null;
     }
   }
@@ -87,6 +92,22 @@ final class PhrictionTransactionEditor
       case PhrictionTransaction::TYPE_CONTENT:
       case PhrictionTransaction::TYPE_DELETE:
         return $xaction->getNewValue();
+      case PhrictionTransaction::TYPE_MOVE_TO:
+        $document = $xaction->getNewValue();
+        // grab the real object now for the sub-editor to come
+        $this->moveAwayDocument = $document;
+        $dict = array(
+          'id' => $document->getID(),
+          'phid' => $document->getPHID(),
+          'content' => $document->getContent()->getContent(),);
+        return $dict;
+      case PhrictionTransaction::TYPE_MOVE_AWAY:
+        $document = $xaction->getNewValue();
+        $dict = array(
+          'id' => $document->getID(),
+          'phid' => $document->getPHID(),
+          'content' => $document->getContent()->getContent(),);
+        return $dict;
     }
   }
 
@@ -99,6 +120,8 @@ final class PhrictionTransactionEditor
       case PhrictionTransaction::TYPE_TITLE:
       case PhrictionTransaction::TYPE_CONTENT:
       case PhrictionTransaction::TYPE_DELETE:
+      case PhrictionTransaction::TYPE_MOVE_TO:
+      case PhrictionTransaction::TYPE_MOVE_AWAY:
         return true;
       }
     }
@@ -120,8 +143,12 @@ final class PhrictionTransactionEditor
     switch ($xaction->getTransactionType()) {
       case PhrictionTransaction::TYPE_TITLE:
       case PhrictionTransaction::TYPE_CONTENT:
+      case PhrictionTransaction::TYPE_MOVE_TO:
         $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
         return;
+      case PhrictionTransaction::TYPE_MOVE_AWAY:
+        $object->setStatus(PhrictionDocumentStatus::STATUS_MOVED);
+        return;
     }
   }
 
@@ -141,6 +168,20 @@ final class PhrictionTransactionEditor
         $this->getNewContent()->setChangeType(
           PhrictionChangeType::CHANGE_DELETE);
         break;
+      case PhrictionTransaction::TYPE_MOVE_TO:
+        $dict = $xaction->getNewValue();
+        $this->getNewContent()->setContent($dict['content']);
+        $this->getNewContent()->setChangeType(
+          PhrictionChangeType::CHANGE_MOVE_HERE);
+        $this->getNewContent()->setChangeRef($dict['id']);
+        break;
+      case PhrictionTransaction::TYPE_MOVE_AWAY:
+        $dict = $xaction->getNewValue();
+        $this->getNewContent()->setContent('');
+        $this->getNewContent()->setChangeType(
+          PhrictionChangeType::CHANGE_MOVE_AWAY);
+        $this->getNewContent()->setChangeRef($dict['id']);
+        break;
       default:
         break;
     }
@@ -156,6 +197,8 @@ final class PhrictionTransactionEditor
         case PhrictionTransaction::TYPE_TITLE:
         case PhrictionTransaction::TYPE_CONTENT:
         case PhrictionTransaction::TYPE_DELETE:
+        case PhrictionTransaction::TYPE_MOVE_AWAY:
+        case PhrictionTransaction::TYPE_MOVE_TO:
           $save_content = true;
           break;
         default:
@@ -196,15 +239,27 @@ final class PhrictionTransactionEditor
         }
       }
     }
+
+    if ($this->moveAwayDocument !== null) {
+      $move_away_xactions = array();
+      $move_away_xactions[] = id(new PhrictionTransaction())
+        ->setTransactionType(PhrictionTransaction::TYPE_MOVE_AWAY)
+        ->setNewValue($object);
+      $sub_editor = id(new PhrictionTransactionEditor())
+        ->setActor($this->getActor())
+        ->setContentSource($this->getContentSource())
+        ->setContinueOnNoEffect($this->getContinueOnNoEffect())
+        ->setDescription($this->getDescription())
+        ->applyTransactions($this->moveAwayDocument, $move_away_xactions);
+    }
+
     return $xactions;
   }
 
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-
+    return true;
-    $xactions = mfilter($xactions, 'shouldHide', true);
-    return $xactions;
   }
 
   protected function getMailSubjectPrefix() {
@@ -274,8 +329,16 @@ final class PhrictionTransactionEditor
     array $xactions) {
 
     $phids = parent::getFeedRelatedPHIDs($object, $xactions);
-    // TODO - once the editor supports moves, we'll need to surface the
+
-    // "from document phid" to related phids.
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhrictionTransaction::TYPE_MOVE_TO:
+          $dict = $xaction->getNewValue();
+          $phids[] = $dict['phid'];
+          break;
+      }
+    }
+
     return $phids;
   }
 

src/applications/phriction/storage/PhrictionTransaction.php

@@ -6,6 +6,8 @@ final class PhrictionTransaction
   const TYPE_TITLE = 'title';
   const TYPE_CONTENT = 'content';
   const TYPE_DELETE  = 'delete';
+  const TYPE_MOVE_TO = 'move-to';
+  const TYPE_MOVE_AWAY = 'move-away';
 
   const MAILTAG_TITLE = 'phriction-title';
   const MAILTAG_CONTENT = 'phriction-content';
@@ -23,6 +25,20 @@ final class PhrictionTransaction
     return new PhrictionTransactionComment();
   }
 
+  public function getRequiredHandlePHIDs() {
+    $phids = parent::getRequiredHandlePHIDs();
+    $new = $this->getNewValue();
+    switch ($this->getTransactionType()) {
+      case self::TYPE_MOVE_TO:
+      case self::TYPE_MOVE_AWAY:
+        $phids[] = $new['phid'];
+        break;
+    }
+
+
+    return $phids;
+  }
+
   public function getRemarkupBlocks() {
     $blocks = parent::getRemarkupBlocks();
 
@@ -49,6 +65,24 @@ final class PhrictionTransaction
     return parent::shouldHide();
   }
 
+  public function shouldHideForMail(array $xactions) {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_MOVE_TO:
+      case self::TYPE_MOVE_AWAY:
+        return true;
+    }
+    return parent::shouldHideForMail($xactions);
+  }
+
+  public function shouldHideForFeed() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_MOVE_TO:
+      case self::TYPE_MOVE_AWAY:
+        return true;
+    }
+    return parent::shouldHideForFeed();
+  }
+
   public function getActionStrength() {
     switch ($this->getTransactionType()) {
       case self::TYPE_TITLE:
@@ -57,6 +91,9 @@ final class PhrictionTransaction
         return 1.3;
       case self::TYPE_DELETE:
         return 1.5;
+      case self::TYPE_MOVE_TO:
+      case self::TYPE_MOVE_AWAY:
+        return 1.0;
     }
 
     return parent::getActionStrength();
@@ -80,6 +117,12 @@ final class PhrictionTransaction
       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();
@@ -95,6 +138,9 @@ final class PhrictionTransaction
         return 'fa-pencil';
       case self::TYPE_DELETE:
         return 'fa-times';
+      case self::TYPE_MOVE_TO:
+      case self::TYPE_MOVE_AWAY:
+        return 'fa-arrows';
     }
 
     return parent::getIcon();
@@ -130,6 +176,17 @@ final class PhrictionTransaction
           '%s deleted this document.',
           $this->renderHandleLink($author_phid));
 
+      case self::TYPE_MOVE_TO:
+        return pht(
+          '%s moved this document from %s',
+          $this->renderHandleLink($author_phid),
+          $this->renderHandleLink($new['phid']));
+
+      case self::TYPE_MOVE_AWAY:
+        return pht(
+          '%s moved this document to %s',
+          $this->renderHandleLink($author_phid),
+          $this->renderHandleLink($new['phid']));
 
     }