From c2f58496ad9369b428f778b52d2e0d650d4cbfb2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 4 May 2014 11:11:34 -0700 Subject: [PATCH] Make the paste "Create" transaction take a file PHID instead of content Summary: Ref T4814. Although this approach made sense at one point, we have more file infrastructure now and T4814 will be easier if we just pass a PHID in. Also swap Conduit over to use the Editor. Test Plan: - Created a paste. - Created a paste via Conduit. - Verified that files had correct permissions and appropriate object links in Files. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4814 Differential Revision: https://secure.phabricator.com/D8969 --- .../ConduitAPI_paste_create_Method.php | 40 ++++---- .../PhabricatorPasteEditController.php | 10 +- .../PhabricatorPasteViewController.php | 16 ++-- .../paste/editor/PhabricatorPasteEditor.php | 93 ++++++++----------- .../paste/mail/PasteCreateMailReceiver.php | 23 +++-- 5 files changed, 92 insertions(+), 90 deletions(-) diff --git a/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php b/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php index dfe60be0fd..a2d08ecd24 100644 --- a/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php +++ b/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php @@ -39,28 +39,36 @@ final class ConduitAPI_paste_create_Method extends ConduitAPI_paste_Method { $title = nonempty($title, 'Masterwork From Distant Lands'); $language = nonempty($language, ''); - $user = $request->getUser(); + $viewer = $request->getUser(); - $paste_file = PhabricatorFile::newFromFileData( - $content, - array( - 'name' => $title, - 'mime-type' => 'text/plain; charset=utf-8', - 'authorPHID' => $user->getPHID(), - )); + $paste = PhabricatorPaste::initializeNewPaste($viewer); - // TODO: This should use PhabricatorPasteEditor. + $file = PhabricatorPasteEditor::initializeFileForPaste( + $viewer, + $title, + $content); - $paste = PhabricatorPaste::initializeNewPaste($user); - $paste->setTitle($title); - $paste->setLanguage($language); - $paste->setFilePHID($paste_file->getPHID()); - $paste->save(); + $xactions = array(); - $paste_file->attachToObject($user, $paste->getPHID()); + $xactions[] = id(new PhabricatorPasteTransaction()) + ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE) + ->setNewValue($file->getPHID()); + + $xactions[] = id(new PhabricatorPasteTransaction()) + ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) + ->setNewValue($title); + + $xactions[] = id(new PhabricatorPasteTransaction()) + ->setTransactionType(PhabricatorPasteTransaction::TYPE_LANGUAGE) + ->setNewValue($language); + + $editor = id(new PhabricatorPasteEditor()) + ->setActor($viewer) + ->setContentSourceFromConduitRequest($request); + + $xactions = $editor->applyTransactions($paste, $xactions); $paste->attachRawContent($content); - return $this->buildPasteInfoDictionary($paste); } diff --git a/src/applications/paste/controller/PhabricatorPasteEditController.php b/src/applications/paste/controller/PhabricatorPasteEditController.php index 394d62eae1..61dd106af1 100644 --- a/src/applications/paste/controller/PhabricatorPasteEditController.php +++ b/src/applications/paste/controller/PhabricatorPasteEditController.php @@ -95,12 +95,16 @@ final class PhabricatorPasteEditController extends PhabricatorPasteController { if (!$errors) { if ($is_create) { + $file = PhabricatorPasteEditor::initializeFileForPaste( + $user, + $v_title, + $v_text); + $xactions[] = id(new PhabricatorPasteTransaction()) ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE) - ->setNewValue(array( - 'title' => $v_title, - 'text' => $v_text)); + ->setNewValue($file->getPHID()); } + $xactions[] = id(new PhabricatorPasteTransaction()) ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) ->setNewValue($v_title); diff --git a/src/applications/paste/controller/PhabricatorPasteViewController.php b/src/applications/paste/controller/PhabricatorPasteViewController.php index 6ece058faf..06c954bef8 100644 --- a/src/applications/paste/controller/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/PhabricatorPasteViewController.php @@ -171,6 +171,13 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { ->setUser($user) ->setObject($paste) ->setObjectURI($this->getRequest()->getRequestURI()) + ->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Paste')) + ->setIcon('edit') + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit) + ->setHref($this->getApplicationURI('/edit/'.$paste->getID().'/'))) ->addAction( id(new PhabricatorActionView()) ->setName(pht('Fork This Paste')) @@ -182,14 +189,7 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { id(new PhabricatorActionView()) ->setName(pht('View Raw File')) ->setIcon('file') - ->setHref($file->getBestURI())) - ->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Edit Paste')) - ->setIcon('edit') - ->setDisabled(!$can_edit) - ->setWorkflow(!$can_edit) - ->setHref($this->getApplicationURI('/edit/'.$paste->getID().'/'))); + ->setHref($file->getBestURI())); } private function buildPropertyView( diff --git a/src/applications/paste/editor/PhabricatorPasteEditor.php b/src/applications/paste/editor/PhabricatorPasteEditor.php index a3bc4f577e..599cdd4d4b 100644 --- a/src/applications/paste/editor/PhabricatorPasteEditor.php +++ b/src/applications/paste/editor/PhabricatorPasteEditor.php @@ -5,6 +5,21 @@ final class PhabricatorPasteEditor private $pasteFile; + public static function initializeFileForPaste( + PhabricatorUser $actor, + $name, + $data) { + + return PhabricatorFile::newFromFileData( + $data, + array( + 'name' => $name, + 'mime-type' => 'text/plain; charset=utf-8', + 'authorPHID' => $actor->getPHID(), + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + )); + } + public function getTransactionTypes() { $types = parent::getTransactionTypes(); @@ -37,8 +52,6 @@ final class PhabricatorPasteEditor switch ($xaction->getTransactionType()) { case PhabricatorPasteTransaction::TYPE_CREATE: - // this was set via applyInitialEffects - return $object->getFilePHID(); case PhabricatorPasteTransaction::TYPE_TITLE: case PhabricatorPasteTransaction::TYPE_LANGUAGE: return $xaction->getNewValue(); @@ -50,73 +63,45 @@ final class PhabricatorPasteEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorPasteTransaction::TYPE_CREATE: + $object->setFilePHID($xaction->getNewValue()); + return; case PhabricatorPasteTransaction::TYPE_TITLE: $object->setTitle($xaction->getNewValue()); - break; + return; case PhabricatorPasteTransaction::TYPE_LANGUAGE: $object->setLanguage($xaction->getNewValue()); - break; + return; } + + return parent::applyCustomInternalTransaction($object, $xaction); } protected function applyCustomExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - } - - protected function shouldApplyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - foreach ($xactions as $xaction) { - if ($xaction->getTransactionType() == - PhabricatorPasteTransaction::TYPE_CREATE) { - return true; - } - } - return false; - } - - protected function applyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case PhabricatorPasteTransaction::TYPE_CREATE: - $data = $xaction->getNewValue(); - $paste_file = PhabricatorFile::newFromFileData( - $data['text'], - array( - 'name' => $data['title'], - 'mime-type' => 'text/plain; charset=utf-8', - 'authorPHID' => $this->getActor()->getPHID(), - )); - $object->setFilePHID($paste_file->getPHID()); - - $this->pasteFile = $paste_file; - break; - } - } - } - - protected function applyFinalEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - // TODO: This should use extractFilePHIDs() instead, but the way - // the transactions work right now makes pretty messy. - - if ($this->pasteFile) { - $this->pasteFile->attachToObject( - $this->getActor(), - $object->getPHID()); + switch ($xaction->getTransactionType()) { + case PhabricatorPasteTransaction::TYPE_CREATE: + case PhabricatorPasteTransaction::TYPE_TITLE: + case PhabricatorPasteTransaction::TYPE_LANGUAGE: + return; } - return $xactions; + return parent::applyCustomExternalTransaction($object, $xaction); } + protected function extractFilePHIDsFromCustomTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PhabricatorPasteTransaction::TYPE_CREATE: + return array($xaction->getNewValue()); + } + + return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); + } protected function shouldSendMail( PhabricatorLiskDAO $object, diff --git a/src/applications/paste/mail/PasteCreateMailReceiver.php b/src/applications/paste/mail/PasteCreateMailReceiver.php index e5053ea70e..885c518990 100644 --- a/src/applications/paste/mail/PasteCreateMailReceiver.php +++ b/src/applications/paste/mail/PasteCreateMailReceiver.php @@ -33,31 +33,36 @@ final class PasteCreateMailReceiver $title = $mail->getSubject(); if (!$title) { - $title = pht('Pasted via email.'); + $title = pht('Email Paste'); } + + $file = PhabricatorPasteEditor::initializeFileForPaste( + $sender, + $title, + $mail->getCleanTextBody()); + $xactions = array(); + $xactions[] = id(new PhabricatorPasteTransaction()) ->setTransactionType(PhabricatorPasteTransaction::TYPE_CREATE) - ->setNewValue(array( - 'title' => $title, - 'text' => $mail->getCleanTextBody())); + ->setNewValue($file->getPHID()); + $xactions[] = id(new PhabricatorPasteTransaction()) ->setTransactionType(PhabricatorPasteTransaction::TYPE_TITLE) ->setNewValue($title); + $xactions[] = id(new PhabricatorPasteTransaction()) ->setTransactionType(PhabricatorPasteTransaction::TYPE_LANGUAGE) ->setNewValue(''); // auto-detect - $xactions[] = id(new PhabricatorPasteTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) - ->setNewValue(PhabricatorPolicies::POLICY_USER); - $paste = id(new PhabricatorPaste()) - ->setAuthorPHID($sender->getPHID()); + $paste = PhabricatorPaste::initializeNewPaste($sender); + $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_EMAIL, array( 'id' => $mail->getID(), )); + $editor = id(new PhabricatorPasteEditor()) ->setActor($sender) ->setContentSource($content_source)