From 260a08a1282d78957ddb27d14b5561f22f3c2d1b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Apr 2017 08:02:37 -0700 Subject: [PATCH] Move Files editing and commenting to EditEngine Summary: Ref T11357. This moves editing and commenting (but not creation) to EditEngine. Since only the name is really editable, this is pretty straightforward. Test Plan: Renamed files; commented on files. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11357 Differential Revision: https://secure.phabricator.com/D17611 --- src/__phutil_library_map__.php | 4 +- .../PhabricatorFilesApplication.php | 3 +- .../PhabricatorFileCommentController.php | 62 ---------- .../PhabricatorFileEditController.php | 112 +----------------- .../PhabricatorFileInfoController.php | 21 ++-- .../editor/PhabricatorFileEditEngine.php | 79 ++++++++++++ .../files/storage/PhabricatorFile.php | 3 + 7 files changed, 98 insertions(+), 186 deletions(-) delete mode 100644 src/applications/files/controller/PhabricatorFileCommentController.php create mode 100644 src/applications/files/editor/PhabricatorFileEditEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 14d84a9ae7..99f9810052 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2743,7 +2743,6 @@ phutil_register_library_map(array( 'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php', 'PhabricatorFileChunkIterator' => 'applications/files/engine/PhabricatorFileChunkIterator.php', 'PhabricatorFileChunkQuery' => 'applications/files/query/PhabricatorFileChunkQuery.php', - 'PhabricatorFileCommentController' => 'applications/files/controller/PhabricatorFileCommentController.php', 'PhabricatorFileComposeController' => 'applications/files/controller/PhabricatorFileComposeController.php', 'PhabricatorFileController' => 'applications/files/controller/PhabricatorFileController.php', 'PhabricatorFileDAO' => 'applications/files/storage/PhabricatorFileDAO.php', @@ -2751,6 +2750,7 @@ phutil_register_library_map(array( 'PhabricatorFileDeleteController' => 'applications/files/controller/PhabricatorFileDeleteController.php', 'PhabricatorFileDropUploadController' => 'applications/files/controller/PhabricatorFileDropUploadController.php', 'PhabricatorFileEditController' => 'applications/files/controller/PhabricatorFileEditController.php', + 'PhabricatorFileEditEngine' => 'applications/files/editor/PhabricatorFileEditEngine.php', 'PhabricatorFileEditField' => 'applications/transactions/editfield/PhabricatorFileEditField.php', 'PhabricatorFileEditor' => 'applications/files/editor/PhabricatorFileEditor.php', 'PhabricatorFileExternalRequest' => 'applications/files/storage/PhabricatorFileExternalRequest.php', @@ -7860,7 +7860,6 @@ phutil_register_library_map(array( 'Iterator', ), 'PhabricatorFileChunkQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', - 'PhabricatorFileCommentController' => 'PhabricatorFileController', 'PhabricatorFileComposeController' => 'PhabricatorFileController', 'PhabricatorFileController' => 'PhabricatorController', 'PhabricatorFileDAO' => 'PhabricatorLiskDAO', @@ -7868,6 +7867,7 @@ phutil_register_library_map(array( 'PhabricatorFileDeleteController' => 'PhabricatorFileController', 'PhabricatorFileDropUploadController' => 'PhabricatorFileController', 'PhabricatorFileEditController' => 'PhabricatorFileController', + 'PhabricatorFileEditEngine' => 'PhabricatorEditEngine', 'PhabricatorFileEditField' => 'PhabricatorEditField', 'PhabricatorFileEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorFileExternalRequest' => array( diff --git a/src/applications/files/application/PhabricatorFilesApplication.php b/src/applications/files/application/PhabricatorFilesApplication.php index 5733439fcf..1010818c19 100644 --- a/src/applications/files/application/PhabricatorFilesApplication.php +++ b/src/applications/files/application/PhabricatorFilesApplication.php @@ -78,7 +78,8 @@ final class PhabricatorFilesApplication extends PhabricatorApplication { 'comment/(?P[1-9]\d*)/' => 'PhabricatorFileCommentController', 'thread/(?P[^/]+)/' => 'PhabricatorFileLightboxController', 'delete/(?P[1-9]\d*)/' => 'PhabricatorFileDeleteController', - 'edit/(?P[1-9]\d*)/' => 'PhabricatorFileEditController', + $this->getEditRoutePattern('edit/') + => 'PhabricatorFileEditController', 'info/(?P[^/]+)/' => 'PhabricatorFileInfoController', 'imageproxy/' => 'PhabricatorFileImageProxyController', 'transforms/(?P[1-9]\d*)/' => diff --git a/src/applications/files/controller/PhabricatorFileCommentController.php b/src/applications/files/controller/PhabricatorFileCommentController.php deleted file mode 100644 index 11833ea8d8..0000000000 --- a/src/applications/files/controller/PhabricatorFileCommentController.php +++ /dev/null @@ -1,62 +0,0 @@ -getViewer(); - $id = $request->getURIData('id'); - - if (!$request->isFormPost()) { - return new Aphront400Response(); - } - - $file = id(new PhabricatorFileQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->executeOne(); - if (!$file) { - return new Aphront404Response(); - } - - $is_preview = $request->isPreviewRequest(); - $draft = PhabricatorDraft::buildFromRequest($request); - - $view_uri = $file->getInfoURI(); - - $xactions = array(); - $xactions[] = id(new PhabricatorFileTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) - ->attachComment( - id(new PhabricatorFileTransactionComment()) - ->setContent($request->getStr('comment'))); - - $editor = id(new PhabricatorFileEditor()) - ->setActor($viewer) - ->setContinueOnNoEffect($request->isContinueRequest()) - ->setContentSourceFromRequest($request) - ->setIsPreview($is_preview); - - try { - $xactions = $editor->applyTransactions($file, $xactions); - } catch (PhabricatorApplicationTransactionNoEffectException $ex) { - return id(new PhabricatorApplicationTransactionNoEffectResponse()) - ->setCancelURI($view_uri) - ->setException($ex); - } - - if ($draft) { - $draft->replaceOrDelete(); - } - - if ($request->isAjax() && $is_preview) { - return id(new PhabricatorApplicationTransactionResponse()) - ->setViewer($viewer) - ->setTransactions($xactions) - ->setIsPreview($is_preview); - } else { - return id(new AphrontRedirectResponse()) - ->setURI($view_uri); - } - } - -} diff --git a/src/applications/files/controller/PhabricatorFileEditController.php b/src/applications/files/controller/PhabricatorFileEditController.php index a1e7a16d80..ea07d72722 100644 --- a/src/applications/files/controller/PhabricatorFileEditController.php +++ b/src/applications/files/controller/PhabricatorFileEditController.php @@ -1,114 +1,12 @@ getViewer(); - $id = $request->getURIData('id'); - - $file = id(new PhabricatorFileQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - if (!$file) { - return new Aphront404Response(); - } - - $title = pht('Edit File: %s', $file->getName()); - $file_name = $file->getName(); - $header_icon = 'fa-pencil'; - $view_uri = '/'.$file->getMonogram(); - $error_name = true; - $validation_exception = null; - - if ($request->isFormPost()) { - $can_view = $request->getStr('canView'); - $file_name = $request->getStr('name'); - $errors = array(); - - $type_name = PhabricatorFileNameTransaction::TRANSACTIONTYPE; - - $xactions = array(); - - $xactions[] = id(new PhabricatorFileTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) - ->setNewValue($can_view); - - $xactions[] = id(new PhabricatorFileTransaction()) - ->setTransactionType($type_name) - ->setNewValue($file_name); - - $editor = id(new PhabricatorFileEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true); - - try { - $editor->applyTransactions($file, $xactions); - return id(new AphrontRedirectResponse())->setURI($view_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; - $error_name = $ex->getShortMessage($type_name); - - $file->setViewPolicy($can_view); - } - } - - - $policies = id(new PhabricatorPolicyQuery()) - ->setViewer($viewer) - ->setObject($file) - ->execute(); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormTextControl()) - ->setName('name') - ->setValue($file_name) - ->setLabel(pht('Name')) - ->setError($error_name)) - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setUser($viewer) - ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) - ->setPolicyObject($file) - ->setPolicies($policies) - ->setName('canView')) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($view_uri) - ->setValue(pht('Save Changes'))); - - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($file->getMonogram(), $view_uri) - ->addTextCrumb(pht('Edit')) - ->setBorder(true); - - $box = id(new PHUIObjectBoxView()) - ->setHeaderText($title) - ->setValidationException($validation_exception) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->appendChild($form); - - $header = id(new PHUIHeaderView()) - ->setHeader($title) - ->setHeaderIcon($header_icon); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($box); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - + return id(new PhabricatorFileEditEngine()) + ->setController($this) + ->buildResponse(); } } diff --git a/src/applications/files/controller/PhabricatorFileInfoController.php b/src/applications/files/controller/PhabricatorFileInfoController.php index 4ca3bbb72e..8e92423fbc 100644 --- a/src/applications/files/controller/PhabricatorFileInfoController.php +++ b/src/applications/files/controller/PhabricatorFileInfoController.php @@ -94,25 +94,18 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { $file, new PhabricatorFileTransactionQuery()); - $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); + $comment_view = id(new PhabricatorFileEditEngine()) + ->setViewer($viewer) + ->buildEditEngineCommentView($file); - $add_comment_header = $is_serious - ? pht('Add Comment') - : pht('Question File Integrity'); + $monogram = $file->getMonogram(); - $draft = PhabricatorDraft::newFromUserAndKey($viewer, $file->getPHID()); - - $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) - ->setUser($viewer) - ->setObjectPHID($file->getPHID()) - ->setDraft($draft) - ->setHeaderText($add_comment_header) - ->setAction($this->getApplicationURI('/comment/'.$file->getID().'/')) - ->setSubmitButtonName(pht('Add Comment')); + $timeline->setQuoteRef($monogram); + $comment_view->setTransactionTimeline($timeline); return array( $timeline, - $add_comment_form, + $comment_view, ); } diff --git a/src/applications/files/editor/PhabricatorFileEditEngine.php b/src/applications/files/editor/PhabricatorFileEditEngine.php new file mode 100644 index 0000000000..04c4753bd5 --- /dev/null +++ b/src/applications/files/editor/PhabricatorFileEditEngine.php @@ -0,0 +1,79 @@ +getName()); + } + + protected function getObjectEditShortText($object) { + return $object->getMonogram(); + } + + protected function getObjectCreateShortText() { + return pht('Create File'); + } + + protected function getObjectName() { + return pht('File'); + } + + protected function getObjectViewURI($object) { + return $object->getURI(); + } + + protected function buildCustomEditFields($object) { + return array( + id(new PhabricatorTextEditField()) + ->setKey('name') + ->setLabel(pht('Name')) + ->setTransactionType(PhabricatorFileNameTransaction::TRANSACTIONTYPE) + ->setDescription(pht('The name of the file.')) + ->setConduitDescription(pht('Rename the file.')) + ->setConduitTypeDescription(pht('New file name.')) + ->setValue($object->getName()), + ); + } + +} diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index d301abceca..da7ad3f729 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -774,6 +774,9 @@ final class PhabricatorFile extends PhabricatorFileDAO return $format->newReadIterator($raw_iterator); } + public function getURI() { + return $this->getInfoURI(); + } public function getViewURI() { if (!$this->getPHID()) {