Phriction - kill the old PhrictionDocumentEditor

Summary:
Ref T4029. Long live PhrictionTransactionEditor...! this means that all existing functionality runs 100% through the modern transactions + editor framework. this diff does a few things in sum

 - kills the old editor
 - moves conduit-based edits to new editor
 - moves stubbing out documents to new editor
 - deletes moving of wiki docs for projects functionality... (T4021#59511 is a better bigger battle plan here.)

Test Plan: edited a phriction document via conduit and it worked. created a new phriction document /that/was/deep/ and verified ancestral documents were properly stubbed out. changed a project name and noted no wiki page moves.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T4029

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

src/__phutil_library_map__.php

@@ -2767,7 +2767,6 @@ phutil_register_library_map(array(
     'PhrictionDiffController' => 'applications/phriction/controller/PhrictionDiffController.php',
     'PhrictionDocument' => 'applications/phriction/storage/PhrictionDocument.php',
     'PhrictionDocumentController' => 'applications/phriction/controller/PhrictionDocumentController.php',
-    'PhrictionDocumentEditor' => 'applications/phriction/editor/PhrictionDocumentEditor.php',
     'PhrictionDocumentPHIDType' => 'applications/phriction/phid/PhrictionDocumentPHIDType.php',
     'PhrictionDocumentPreviewController' => 'applications/phriction/controller/PhrictionDocumentPreviewController.php',
     'PhrictionDocumentQuery' => 'applications/phriction/query/PhrictionDocumentQuery.php',
@@ -5979,7 +5978,6 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
     ),
     'PhrictionDocumentController' => 'PhrictionController',
-    'PhrictionDocumentEditor' => 'PhabricatorEditor',
     'PhrictionDocumentPHIDType' => 'PhabricatorPHIDType',
     'PhrictionDocumentPreviewController' => 'PhrictionController',
     'PhrictionDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',

src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php

@@ -34,6 +34,7 @@ final class PhrictionEditConduitAPIMethod extends PhrictionConduitAPIMethod {
     $doc = id(new PhrictionDocumentQuery())
       ->setViewer($request->getUser())
       ->withSlugs(array(PhabricatorSlug::normalize($slug)))
+      ->needContent(true)
       ->requireCapabilities(
         array(
           PhabricatorPolicyCapability::CAN_VIEW,
@@ -44,14 +45,22 @@ final class PhrictionEditConduitAPIMethod extends PhrictionConduitAPIMethod {
       throw new Exception(pht('No such document.'));
     }
 
-    $editor = id(PhrictionDocumentEditor::newForSlug($slug))
-      ->setActor($request->getUser())
-      ->setTitle($request->getValue('title'))
-      ->setContent($request->getValue('content'))
-      ->setDescription($request->getvalue('description'))
-      ->save();
+    $xactions = array();
+    $xactions[] = id(new PhrictionTransaction())
+      ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+      ->setNewValue($request->getValue('title'));
+    $xactions[] = id(new PhrictionTransaction())
+      ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)
+      ->setNewValue($request->getValue('content'));
 
-    return $this->buildDocumentInfoDictionary($editor->getDocument());
+    $editor = id(new PhrictionTransactionEditor())
+      ->setActor($request->getUser())
+      ->setContentSourceFromConduitRequest($request)
+      ->setContinueOnNoEffect(true)
+      ->setDescription($request->getValue('description'))
+      ->applyTransactions($doc, $xactions);
+
+    return $this->buildDocumentInfoDictionary($doc);
   }
 
 }

src/applications/phriction/editor/PhrictionDocumentEditor.php

@@ -1,379 +0,0 @@
-<?php
-
-/**
- * Create or update Phriction documents.
- */
-final class PhrictionDocumentEditor extends PhabricatorEditor {
-
-  private $document;
-  private $content;
-
-  private $newTitle;
-  private $newContent;
-  private $description;
-
-  // For the Feed Story when moving documents
-  private $fromDocumentPHID;
-
-  private function __construct() {
-    // <restricted>
-  }
-
-  public static function newForSlug($slug) {
-    $slug = PhabricatorSlug::normalize($slug);
-
-    // TODO: Get rid of this.
-    $document = id(new PhrictionDocument())->loadOneWhere(
-      'slug = %s',
-      $slug);
-    $content = null;
-
-    if ($document) {
-      $content = id(new PhrictionContent())->load($document->getContentID());
-    } else {
-      $document = new PhrictionDocument();
-      $document->setSlug($slug);
-    }
-
-    if (!$content) {
-      $default_title = PhabricatorSlug::getDefaultTitle($slug);
-      $content = new PhrictionContent();
-      $content->setSlug($slug);
-      $content->setTitle($default_title);
-      $content->setContent('');
-    }
-
-    $obj = new PhrictionDocumentEditor();
-    $obj->document = $document;
-    $obj->content  = $content;
-
-    return $obj;
-  }
-
-  public function setTitle($title) {
-    $this->newTitle = $title;
-    return $this;
-  }
-
-  public function setContent($content) {
-    $this->newContent = $content;
-    return $this;
-  }
-
-  public function setDescription($description) {
-    $this->description = $description;
-    return $this;
-  }
-
-  public function getDocument() {
-    return $this->document;
-  }
-
-  public function moveAway($new_doc_id) {
-    return $this->execute(
-      PhrictionChangeType::CHANGE_MOVE_AWAY, true, $new_doc_id);
-  }
-
-  public function moveHere($old_doc_id, $old_doc_phid) {
-    $this->fromDocumentPHID = $old_doc_phid;
-    return $this->execute(
-      PhrictionChangeType::CHANGE_MOVE_HERE, false, $old_doc_id);
-  }
-
-  private function execute(
-    $change_type, $del_new_content = true, $doc_ref = null) {
-
-    $actor = $this->requireActor();
-
-    $document = $this->document;
-    $content  = $this->content;
-
-    $new_content = $this->buildContentTemplate($document, $content);
-    $new_content->setChangeType($change_type);
-
-    if ($del_new_content) {
-      $new_content->setContent('');
-    }
-
-    if ($doc_ref) {
-      $new_content->setChangeRef($doc_ref);
-    }
-
-    return $this->updateDocument($document, $content, $new_content);
-  }
-
-  public function delete() {
-    return $this->execute(PhrictionChangeType::CHANGE_DELETE, true);
-  }
-
-  public function stub() {
-    return $this->execute(PhrictionChangeType::CHANGE_STUB, true);
-  }
-
-  public function save() {
-    $actor = $this->requireActor();
-
-    if ($this->newContent === '') {
-      // If this is an edit which deletes all the content, just treat it as
-      // a delete. NOTE: null means "don't change the content", not "delete
-      // the page"! Thus the strict type check.
-      return $this->delete();
-    }
-
-    $document = $this->document;
-    $content  = $this->content;
-
-    $new_content = $this->buildContentTemplate($document, $content);
-
-    return $this->updateDocument($document, $content, $new_content);
-  }
-
-  private function buildContentTemplate(
-    PhrictionDocument $document,
-    PhrictionContent $content) {
-
-    $new_content = new PhrictionContent();
-    $new_content->setSlug($document->getSlug());
-    $new_content->setAuthorPHID($this->getActor()->getPHID());
-    $new_content->setChangeType(PhrictionChangeType::CHANGE_EDIT);
-
-    $new_content->setTitle(
-      coalesce(
-        $this->newTitle,
-        $content->getTitle()));
-
-    $new_content->setContent(
-      coalesce(
-        $this->newContent,
-        $content->getContent()));
-
-    if (strlen($this->description)) {
-      $new_content->setDescription($this->description);
-    }
-
-    return $new_content;
-  }
-
-  private function updateDocument($document, $content, $new_content) {
-
-    $is_new = false;
-    if (!$document->getID()) {
-      $is_new = true;
-    }
-
-    $new_content->setVersion($content->getVersion() + 1);
-
-    $change_type = $new_content->getChangeType();
-    switch ($change_type) {
-      case PhrictionChangeType::CHANGE_EDIT:
-        $doc_status = PhrictionDocumentStatus::STATUS_EXISTS;
-        $feed_action = $is_new
-          ? PhrictionActionConstants::ACTION_CREATE
-          : PhrictionActionConstants::ACTION_EDIT;
-        break;
-      case PhrictionChangeType::CHANGE_DELETE:
-        $doc_status = PhrictionDocumentStatus::STATUS_DELETED;
-        $feed_action = PhrictionActionConstants::ACTION_DELETE;
-        if ($is_new) {
-          throw new Exception(
-            "You can not delete a document which doesn't exist yet!");
-        }
-        break;
-      case PhrictionChangeType::CHANGE_STUB:
-        $doc_status = PhrictionDocumentStatus::STATUS_STUB;
-        $feed_action = null;
-        break;
-      case PhrictionChangeType::CHANGE_MOVE_AWAY:
-        $doc_status = PhrictionDocumentStatus::STATUS_MOVED;
-        $feed_action = null;
-        break;
-      case PhrictionChangeType::CHANGE_MOVE_HERE:
-        $doc_status = PhrictionDocumentStatus::STATUS_EXISTS;
-        $feed_action = PhrictionActionConstants::ACTION_MOVE_HERE;
-        break;
-      default:
-        throw new Exception(
-          "Unsupported content change type '{$change_type}'!");
-    }
-
-    $document->setStatus($doc_status);
-
-    // TODO: This should be transactional.
-
-    if ($is_new) {
-      $document->save();
-    }
-
-    $new_content->setDocumentID($document->getID());
-    $new_content->save();
-
-    $document->setContentID($new_content->getID());
-    $document->save();
-
-    $document->attachContent($new_content);
-
-    id(new PhabricatorSearchIndexer())
-      ->queueDocumentForIndexing($document->getPHID());
-
-    // Stub out empty parent documents if they don't exist
-    $ancestral_slugs = PhabricatorSlug::getAncestry($document->getSlug());
-    if ($ancestral_slugs) {
-      $ancestors = id(new PhrictionDocument())->loadAllWhere(
-        'slug IN (%Ls)',
-        $ancestral_slugs);
-      $ancestors = mpull($ancestors, null, 'getSlug');
-      foreach ($ancestral_slugs as $slug) {
-        // We check for change type to prevent near-infinite recursion
-        if (!isset($ancestors[$slug]) &&
-          $new_content->getChangeType() != PhrictionChangeType::CHANGE_STUB) {
-
-          id(PhrictionDocumentEditor::newForSlug($slug))
-            ->setActor($this->getActor())
-            ->setTitle(PhabricatorSlug::getDefaultTitle($slug))
-            ->setContent('')
-            ->setDescription(pht('Empty Parent Document'))
-            ->stub();
-        }
-      }
-    }
-
-    $project_phid = null;
-    $slug = $document->getSlug();
-    if (PhrictionDocument::isProjectSlug($slug)) {
-      $project = id(new PhabricatorProjectQuery())
-        ->setViewer($this->requireActor())
-        ->withPhrictionSlugs(array(
-          PhrictionDocument::getProjectSlugIdentifier($slug),
-        ))
-        ->executeOne();
-      if ($project) {
-        $project_phid = $project->getPHID();
-      }
-    }
-
-    $related_phids = array(
-      $document->getPHID(),
-      $this->getActor()->getPHID(),
-    );
-
-    if ($project_phid) {
-      $related_phids[] = $project_phid;
-    }
-
-    if ($this->fromDocumentPHID) {
-      $related_phids[] = $this->fromDocumentPHID;
-    }
-
-    if ($feed_action) {
-      $content_str = id(new PhutilUTF8StringTruncator())
-        ->setMaximumGlyphs(140)
-        ->truncateString($new_content->getContent());
-      id(new PhabricatorFeedStoryPublisher())
-        ->setRelatedPHIDs($related_phids)
-        ->setStoryAuthorPHID($this->getActor()->getPHID())
-        ->setStoryTime(time())
-        ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_PHRICTION)
-        ->setStoryData(
-          array(
-            'phid'      => $document->getPHID(),
-            'action'    => $feed_action,
-            'content'   => $content_str,
-            'project'   => $project_phid,
-            'movedFromPHID' => $this->fromDocumentPHID,
-          ))
-        ->publish();
-    }
-
-    // TODO: Migrate to ApplicationTransactions fast, so we get rid of this code
-    $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
-      $document->getPHID());
-    $this->sendMailToSubscribers($subscribers, $content);
-
-    return $this;
-  }
-
-  private function getChangeTypeDescription($const, $title) {
-    $map = array(
-      PhrictionChangeType::CHANGE_EDIT =>
-        pht('Phriction Document %s was edited.', $title),
-      PhrictionChangeType::CHANGE_DELETE =>
-        pht('Phriction Document %s was deleted.', $title),
-      PhrictionChangeType::CHANGE_MOVE_HERE =>
-        pht('Phriction Document %s was moved here.', $title),
-      PhrictionChangeType::CHANGE_MOVE_AWAY =>
-        pht('Phriction Document %s was moved away.', $title),
-      PhrictionChangeType::CHANGE_STUB =>
-        pht('Phriction Document %s was created through child.', $title),
-    );
-    return idx($map, $const, pht('Something magical occurred.'));
-  }
-
-  private function sendMailToSubscribers(array $subscribers, $old_content) {
-    if (!$subscribers) {
-      return;
-    }
-
-    $author_phid = $this->getActor()->getPHID();
-    $document = $this->document;
-    $content = $document->getContent();
-    $slug_uri = PhrictionDocument::getSlugURI($document->getSlug());
-    $diff_uri = new PhutilURI('/phriction/diff/'.$document->getID().'/');
-    $prod_uri = PhabricatorEnv::getProductionURI('');
-
-    $vs_head = $diff_uri
-      ->alter('l', $old_content->getVersion())
-      ->alter('r', $content->getVersion());
-
-    $old_title = $old_content->getTitle();
-    $title = $content->getTitle();
-    $name = $this->getChangeTypeDescription($content->getChangeType(), $title);
-    $action = PhrictionChangeType::getChangeTypeLabel(
-      $content->getChangeType());
-
-    $body = array($name);
-    // Content may have changed, you never know
-    if ($content->getChangeType() == PhrictionChangeType::CHANGE_EDIT) {
-
-      if ($old_title != $title) {
-        $body[] = pht('Title was changed from "%s" to "%s"',
-          $old_title, $title);
-      }
-
-      $body[] = pht("Link to new version:\n%s",
-        $prod_uri.$slug_uri.'?v='.$content->getVersion());
-
-      $body[] = pht("Link to diff:\n%s", $prod_uri.$vs_head);
-    } else if ($content->getChangeType() ==
-      PhrictionChangeType::CHANGE_MOVE_AWAY) {
-
-      $target_document = id(new PhrictionDocument())
-        ->load($content->getChangeRef());
-      $slug_uri = PhrictionDocument::getSlugURI($target_document->getSlug());
-      $body[] = pht("Link to destination document:\n%s", $prod_uri.$slug_uri);
-    }
-
-    $body = implode("\n\n", $body);
-
-    $subject_prefix = $this->getMailSubjectPrefix();
-
-    $mail = new PhabricatorMetaMTAMail();
-    $mail->setSubject($name)
-      ->setSubjectPrefix($subject_prefix)
-      ->setVarySubjectPrefix('['.$action.']')
-      ->addHeader('Thread-Topic', $name)
-      ->setFrom($author_phid)
-      ->addTos($subscribers)
-      ->setBody($body)
-      ->setRelatedPHID($document->getPHID())
-      ->setIsBulk(true);
-
-    $mail->saveAndSend();
-  }
-
-  /* --( For less copy-pasting when switching to ApplicationTransactions )--- */
-
-  protected function getMailSubjectPrefix() {
-    return PhabricatorEnv::getEnvConfig('metamta.phriction.subject-prefix');
-  }
-
-}

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -7,6 +7,7 @@ final class PhrictionTransactionEditor
   private $oldContent;
   private $newContent;
   private $moveAwayDocument;
+  private $skipAncestorCheck;
 
   public function setDescription($description) {
     $this->description = $description;
@@ -35,6 +36,15 @@ final class PhrictionTransactionEditor
     return $this->newContent;
   }
 
+  public function setSkipAncestorCheck($bool) {
+    $this->skipAncestorCheck = $bool;
+    return $this;
+  }
+
+  public function getSkipAncestorCheck() {
+    return $this->skipAncestorCheck;
+  }
+
   public function getEditorApplicationClass() {
     return 'PhabricatorPhrictionApplication';
   }
@@ -216,26 +226,40 @@ final class PhrictionTransactionEditor
       $object->attachContent($content);
     }
 
-    if ($this->getIsNewObject()) {
+    if ($this->getIsNewObject() && !$this->getSkipAncestorCheck()) {
       // Stub out empty parent documents if they don't exist
       $ancestral_slugs = PhabricatorSlug::getAncestry($object->getSlug());
       if ($ancestral_slugs) {
-        $ancestors = id(new PhrictionDocument())->loadAllWhere(
-          'slug IN (%Ls)',
-          $ancestral_slugs);
+        $ancestors = id(new PhrictionDocumentQuery())
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
+          ->withSlugs($ancestral_slugs)
+          ->needContent(true)
+          ->execute();
         $ancestors = mpull($ancestors, null, 'getSlug');
+        $stub_type = PhrictionChangeType::CHANGE_STUB;
         foreach ($ancestral_slugs as $slug) {
+          $ancestor_doc = idx($ancestors, $slug);
           // We check for change type to prevent near-infinite recursion
-          if (!isset($ancestors[$slug]) &&
-            $content->getChangeType() !=
-            PhrictionChangeType::CHANGE_STUB) {
-              id(PhrictionDocumentEditor::newForSlug($slug))
-                ->setActor($this->getActor())
-                ->setTitle(PhabricatorSlug::getDefaultTitle($slug))
-                ->setContent('')
-                ->setDescription(pht('Empty Parent Document'))
-                ->stub();
-            }
+          if (!$ancestor_doc && $content->getChangeType() != $stub_type) {
+            $ancestor_doc = PhrictionDocument::initializeNewDocument(
+              $this->getActor(),
+              $slug);
+            $stub_xactions = array();
+            $stub_xactions[] = id(new PhrictionTransaction())
+              ->setTransactionType(PhrictionTransaction::TYPE_TITLE)
+              ->setNewValue(PhabricatorSlug::getDefaultTitle($slug))
+              ->setMetadataValue('stub:create:phid', $object->getPHID());
+            $stub_xactions[] = id(new PhrictionTransaction())
+              ->setTransactionType(PhrictionTransaction::TYPE_CONTENT)
+              ->setNewValue('');
+            $sub_editor = id(new PhrictionTransactionEditor())
+              ->setActor($this->getActor())
+              ->setContentSource($this->getContentSource())
+              ->setContinueOnNoEffect($this->getContinueOnNoEffect())
+              ->setSkipAncestorCheck(true)
+              ->setDescription(pht('Empty Parent Document'))
+              ->applyTransactions($ancestor_doc, $stub_xactions);
+          }
         }
       }
     }

src/applications/phriction/storage/PhrictionTransaction.php

@@ -33,6 +33,11 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_AWAY:
         $phids[] = $new['phid'];
         break;
+      case self::TYPE_TITLE:
+        if ($this->getMetadataValue('stub:create:phid')) {
+          $phids[] = $this->getMetadataValue('stub:create:phid');
+        }
+        break;
     }
 
 
@@ -70,6 +75,8 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_TO:
       case self::TYPE_MOVE_AWAY:
         return true;
+      case self::TYPE_TITLE:
+        return $this->getMetadataValue('stub:create:phid', false);
     }
     return parent::shouldHideForMail($xactions);
   }
@@ -79,6 +86,8 @@ final class PhrictionTransaction
       case self::TYPE_MOVE_TO:
       case self::TYPE_MOVE_AWAY:
         return true;
+      case self::TYPE_TITLE:
+        return $this->getMetadataValue('stub:create:phid', false);
     }
     return parent::shouldHideForFeed();
   }
@@ -106,7 +115,11 @@ final class PhrictionTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_TITLE:
         if ($old === null) {
-          return pht('Created');
+          if ($this->getMetadataValue('stub:create:phid')) {
+            return pht('Stubbed');
+          } else {
+            return pht('Created');
+          }
         }
 
         return pht('Retitled');
@@ -156,9 +169,17 @@ final class PhrictionTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_TITLE:
         if ($old === null) {
-          return pht(
-            '%s created this document.',
-            $this->renderHandleLink($author_phid));
+          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".',

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -127,7 +127,7 @@ final class PhabricatorProjectTransactionEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorProjectTransaction::TYPE_NAME:
         // First, remove the old and new slugs. Removing the old slug is
-        // important when changing the project's capitalization or puctuation.
+        // important when changing the project's capitalization or punctuation.
         // Removing the new slug is important when changing the project's name
         // so that one of its secondary slugs is now the primary slug.
         if ($old !== null) {
@@ -140,40 +140,6 @@ final class PhabricatorProjectTransactionEditor
           ->setProjectPHID($object->getPHID())
           ->save();
 
-        // TODO -- delete all of the below once we sever automagical project
-        // to phriction stuff
-        if ($xaction->getOldValue() === null) {
-          // Project was just created, we don't need to move anything.
-          return;
-        }
-
-        $clone_object = clone $object;
-        $clone_object->setPhrictionSlug($xaction->getOldValue());
-        $old_slug = $clone_object->getFullPhrictionSlug();
-
-        $old_document = id(new PhrictionDocument())
-          ->loadOneWhere('slug = %s', $old_slug);
-        if ($old_document && $old_document->getStatus() ==
-            PhrictionDocumentStatus::STATUS_EXISTS) {
-          $content = id(new PhrictionContent())
-            ->load($old_document->getContentID());
-          $from_editor = id(PhrictionDocumentEditor::newForSlug($old_slug))
-            ->setActor($this->getActor())
-            ->setTitle($content->getTitle())
-            ->setContent($content->getContent())
-            ->setDescription($content->getDescription());
-
-          $target_editor = id(PhrictionDocumentEditor::newForSlug(
-            $object->getFullPhrictionSlug()))
-            ->setActor($this->getActor())
-            ->setTitle($content->getTitle())
-            ->setContent($content->getContent())
-            ->setDescription($content->getDescription())
-            ->moveHere($old_document->getID(), $old_document->getPHID());
-
-          $target_document = $target_editor->getDocument();
-          $from_editor->moveAway($target_document->getID());
-        }
         return;
       case PhabricatorProjectTransaction::TYPE_SLUGS:
         $old = $xaction->getOldValue();