Allow pastes to be edited

Summary: Fixes T4814.

Test Plan: Edited pastes from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4814

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

resources/sql/patches/20130801.pastexactions.php

@@ -30,7 +30,7 @@ foreach ($rows as $row) {
     $row['phid'],
     'public',
     $row['authorPHID'],
-    PhabricatorPasteTransaction::TYPE_CREATE,
+    PhabricatorPasteTransaction::TYPE_CONTENT,
     'null',
     $row['filePHID'],
     PhabricatorContentSource::newForSource(

src/applications/paste/conduit/ConduitAPI_paste_create_Method.php

@@ -51,7 +51,7 @@ final class ConduitAPI_paste_create_Method extends ConduitAPI_paste_Method {
     $xactions = array();
 
     $xactions[] = id(new PhabricatorPasteTransaction())
-      ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE)
+      ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT)
       ->setNewValue($file->getPHID());
 
     $xactions[] = id(new PhabricatorPasteTransaction())

src/applications/paste/controller/PhabricatorPasteEditController.php

@@ -42,6 +42,7 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
       }
 
       $paste->setAuthorPHID($user->getPHID());
+      $paste->attachRawContent('');
     } else {
       $is_create = false;
 
@@ -53,6 +54,7 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
             PhabricatorPolicyCapability::CAN_EDIT,
           ))
         ->withIDs(array($this->id))
+        ->needRawContent(true)
         ->executeOne();
       if (!$paste) {
         return new Aphront404Response();
@@ -69,14 +71,13 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
     } else {
       $v_title = $paste->getTitle();
       $v_language = $paste->getLanguage();
-      $v_text = '';
+      $v_text = $paste->getRawContent();
     }
     $v_policy = $paste->getViewPolicy();
 
     if ($request->isFormPost()) {
       $xactions = array();
 
-      if ($is_create) {
       $v_text = $request->getStr('text');
       if (!strlen($v_text)) {
         $e_text = pht('Required');
@@ -84,7 +85,6 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
       } else {
         $e_text = null;
       }
-     }
 
       $v_title = $request->getStr('title');
       $v_language = $request->getStr('language');
@@ -94,14 +94,14 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
       // so it's impossible for them to choose an invalid policy.
 
       if (!$errors) {
-        if ($is_create) {
+        if ($is_create || ($v_text !== $paste->getRawContent())) {
           $file = PhabricatorPasteEditor::initializeFileForPaste(
             $user,
             $v_title,
             $v_text);
 
           $xactions[] = id(new PhabricatorPasteTransaction())
-            ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE)
+            ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT)
             ->setNewValue($file->getPHID());
         }
 
@@ -161,7 +161,6 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
         ->setPolicies($policies)
         ->setName('can_view'));
 
-    if ($is_create) {
     $form
       ->appendChild(
         id(new AphrontFormTextAreaControl())
@@ -171,21 +170,6 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController {
           ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
           ->setCustomClass('PhabricatorMonospaced')
           ->setName('text'));
-    } else {
-      $fork_link = phutil_tag(
-        'a',
-        array(
-          'href' => $this->getApplicationURI('?parent='.$paste->getID())
-        ),
-        pht('Fork'));
-      $form
-        ->appendChild(
-          id(new AphrontFormMarkupControl())
-          ->setLabel(pht('Text'))
-          ->setValue(pht(
-            'Paste text can not be edited. %s to create a new paste.',
-            $fork_link)));
-    }
 
     $submit = new AphrontFormSubmitControl();
 

src/applications/paste/editor/PhabricatorPasteEditor.php

@@ -23,7 +23,7 @@ final class PhabricatorPasteEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhabricatorPasteTransaction::TYPE_CREATE;
+    $types[] = PhabricatorPasteTransaction::TYPE_CONTENT;
     $types[] = PhabricatorPasteTransaction::TYPE_TITLE;
     $types[] = PhabricatorPasteTransaction::TYPE_LANGUAGE;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
@@ -37,8 +37,8 @@ final class PhabricatorPasteEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
-        return null;
+        return $object->getFilePHID();
       case PhabricatorPasteTransaction::TYPE_TITLE:
         return $object->getTitle();
       case PhabricatorPasteTransaction::TYPE_LANGUAGE:
@@ -51,7 +51,7 @@ final class PhabricatorPasteEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
       case PhabricatorPasteTransaction::TYPE_TITLE:
       case PhabricatorPasteTransaction::TYPE_LANGUAGE:
         return $xaction->getNewValue();
@@ -63,7 +63,7 @@ final class PhabricatorPasteEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
         $object->setFilePHID($xaction->getNewValue());
         return;
       case PhabricatorPasteTransaction::TYPE_TITLE:
@@ -72,6 +72,9 @@ final class PhabricatorPasteEditor
       case PhabricatorPasteTransaction::TYPE_LANGUAGE:
         $object->setLanguage($xaction->getNewValue());
         return;
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+        $object->setViewPolicy($xaction->getNewValue());
+        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -82,9 +85,10 @@ final class PhabricatorPasteEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
       case PhabricatorPasteTransaction::TYPE_TITLE:
       case PhabricatorPasteTransaction::TYPE_LANGUAGE:
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
         return;
     }
 
@@ -96,7 +100,7 @@ final class PhabricatorPasteEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
         return array($xaction->getNewValue());
     }
 
@@ -108,7 +112,7 @@ final class PhabricatorPasteEditor
     array $xactions) {
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case PhabricatorPasteTransaction::TYPE_CREATE:
+        case PhabricatorPasteTransaction::TYPE_CONTENT:
           return false;
         default:
           break;

src/applications/paste/mail/PasteCreateMailReceiver.php

@@ -44,7 +44,7 @@ final class PasteCreateMailReceiver
     $xactions = array();
 
     $xactions[] = id(new PhabricatorPasteTransaction())
-      ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE)
+      ->setTransactionType(PhabricatorPasteTransaction::TYPE_CONTENT)
       ->setNewValue($file->getPHID());
 
     $xactions[] = id(new PhabricatorPasteTransaction())

src/applications/paste/query/PhabricatorPasteQuery.php

@@ -157,7 +157,13 @@ final class PhabricatorPasteQuery
   }
 
   private function getContentCacheKey(PhabricatorPaste $paste) {
-    return 'P'.$paste->getID().':content/'.$paste->getLanguage();
+    return implode(
+      ':',
+      array(
+        'P'.$paste->getID(),
+        $paste->getFilePHID(),
+        $paste->getLanguage(),
+      ));
   }
 
   private function loadRawContent(array $pastes) {

src/applications/paste/storage/PhabricatorPasteTransaction.php

@@ -3,7 +3,7 @@
 final class PhabricatorPasteTransaction
   extends PhabricatorApplicationTransaction {
 
-  const TYPE_CREATE = 'paste.create';
+  const TYPE_CONTENT = 'paste.create';
   const TYPE_TITLE = 'paste.title';
   const TYPE_LANGUAGE = 'paste.language';
 
@@ -23,7 +23,7 @@ final class PhabricatorPasteTransaction
     $phids = parent::getRequiredHandlePHIDs();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_CREATE:
+      case self::TYPE_CONTENT:
         $phids[] = $this->getObjectPHID();
         break;
     }
@@ -36,14 +36,14 @@ final class PhabricatorPasteTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_TITLE:
       case self::TYPE_LANGUAGE:
-        return $old === null;
+        return ($old === null);
     }
     return parent::shouldHide();
   }
 
   public function getIcon() {
     switch ($this->getTransactionType()) {
-      case self::TYPE_CREATE:
+      case self::TYPE_CONTENT:
         return 'fa-plus';
         break;
       case self::TYPE_TITLE:
@@ -63,11 +63,16 @@ final class PhabricatorPasteTransaction
 
     $type = $this->getTransactionType();
     switch ($type) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
+        if ($old === null) {
           return pht(
-          '%s created "%s".',
+            '%s created this paste.',
-          $this->renderHandleLink($author_phid),
+            $this->renderHandleLink($author_phid));
-          $this->renderHandleLink($object_phid));
+        } else {
+          return pht(
+            '%s edited the content of this paste.',
+            $this->renderHandleLink($author_phid));
+        }
         break;
       case PhabricatorPasteTransaction::TYPE_TITLE:
         return pht(
@@ -94,11 +99,18 @@ final class PhabricatorPasteTransaction
 
     $type = $this->getTransactionType();
     switch ($type) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case PhabricatorPasteTransaction::TYPE_CONTENT:
+        if ($old === null) {
           return pht(
             '%s created %s.',
             $this->renderHandleLink($author_phid),
             $this->renderHandleLink($object_phid));
+        } else {
+          return pht(
+            '%s edited %s.',
+            $this->renderHandleLink($author_phid),
+            $this->renderHandleLink($object_phid));
+        }
         break;
       case PhabricatorPasteTransaction::TYPE_TITLE:
         return pht(
@@ -122,10 +134,52 @@ final class PhabricatorPasteTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case PhabricatorPasteTransaction::TYPE_CREATE:
+      case self::TYPE_CONTENT:
         return PhabricatorTransactions::COLOR_GREEN;
     }
 
     return parent::getColor();
   }
+
+
+  public function hasChangeDetails() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CONTENT:
+        return ($this->getOldValue() !== null);
+    }
+
+    return parent::hasChangeDetails();
+  }
+
+  public function renderChangeDetails(PhabricatorUser $viewer) {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CONTENT:
+        $old = $this->getOldValue();
+        $new = $this->getNewValue();
+
+        $files = id(new PhabricatorFileQuery())
+          ->setViewer($viewer)
+          ->withPHIDs(array_filter(array($old, $new)))
+          ->execute();
+        $files = mpull($files, null, 'getPHID');
+
+        $old_text = '';
+        if (idx($files, $old)) {
+          $old_text = $files[$old]->loadFileData();
+        }
+
+        $new_text = '';
+        if (idx($files, $new)) {
+          $new_text = $files[$new]->loadFileData();
+        }
+
+        return $this->renderTextCorpusChangeDetails(
+          $viewer,
+          $old_text,
+          $new_text);
+    }
+
+    return parent::renderChangeDetails($viewer);
+  }
+
 }

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -822,7 +822,10 @@ abstract class PhabricatorApplicationTransaction
         break;
     }
 
-    return $this->renderTextCorpusChangeDetails();
+    return $this->renderTextCorpusChangeDetails(
+      $viewer,
+      $this->getOldValue(),
+      $this->getNewValue());
   }
 
   public function renderTextCorpusChangeDetails(