From fed30dfb4c81cb01a097b7c03df2b5145fc686a9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 24 Aug 2012 13:19:14 -0700 Subject: [PATCH] Split paste create/edit and list views Summary: We have this hybrid "create / last few pastes" landing screen right now but I ~never use the list at the bottom and it makes the controller kind of complicated. I want to let you edit pastes too, and this generally simplifies things. Also makes the textarea monospaced and cleans up the fork logic a bit. Test Plan: Created, forked pastes. Viewed paste lists. Viewed pastes. Reviewers: vrana, btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1690 Differential Revision: https://secure.phabricator.com/D3375 --- conf/default.conf.php | 3 - src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationPaste.php | 2 +- .../controller/PhabricatorPasteController.php | 2 +- .../PhabricatorPasteEditController.php | 154 +++++++++++ .../PhabricatorPasteListController.php | 256 ++---------------- .../PhabricatorPasteViewController.php | 2 +- .../paste/storage/PhabricatorPaste.php | 6 +- .../handle/PhabricatorObjectHandleData.php | 2 +- .../control/AphrontFormTextAreaControl.php | 14 +- 10 files changed, 194 insertions(+), 249 deletions(-) create mode 100644 src/applications/paste/controller/PhabricatorPasteEditController.php diff --git a/conf/default.conf.php b/conf/default.conf.php index fd9aeb8d3b..5ff5fefd8c 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -1196,7 +1196,6 @@ return array( 'erb' => 'Embedded Ruby/ERB', 'erlang' => 'Erlang', 'html' => 'HTML', - 'infer' => 'Infer from title (extension)', 'java' => 'Java', 'js' => 'Javascript', 'mysql' => 'MySQL', @@ -1210,8 +1209,6 @@ return array( 'xml' => 'XML', ), - 'pygments.dropdown-default' => 'infer', - // This is an override list of regular expressions which allows you to choose // what language files are highlighted as. If your projects have certain rules // about filenames or use unusual or ambiguous language extensions, you can diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5bf6aceb21..411db2cfd3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -892,6 +892,7 @@ phutil_register_library_map(array( 'PhabricatorPaste' => 'applications/paste/storage/PhabricatorPaste.php', 'PhabricatorPasteController' => 'applications/paste/controller/PhabricatorPasteController.php', 'PhabricatorPasteDAO' => 'applications/paste/storage/PhabricatorPasteDAO.php', + 'PhabricatorPasteEditController' => 'applications/paste/controller/PhabricatorPasteEditController.php', 'PhabricatorPasteListController' => 'applications/paste/controller/PhabricatorPasteListController.php', 'PhabricatorPasteQuery' => 'applications/paste/query/PhabricatorPasteQuery.php', 'PhabricatorPasteViewController' => 'applications/paste/controller/PhabricatorPasteViewController.php', @@ -1992,6 +1993,7 @@ phutil_register_library_map(array( ), 'PhabricatorPasteController' => 'PhabricatorController', 'PhabricatorPasteDAO' => 'PhabricatorLiskDAO', + 'PhabricatorPasteEditController' => 'PhabricatorPasteController', 'PhabricatorPasteListController' => 'PhabricatorPasteController', 'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyQuery', 'PhabricatorPasteViewController' => 'PhabricatorPasteController', diff --git a/src/applications/paste/application/PhabricatorApplicationPaste.php b/src/applications/paste/application/PhabricatorApplicationPaste.php index c0e7aa61ea..925db87345 100644 --- a/src/applications/paste/application/PhabricatorApplicationPaste.php +++ b/src/applications/paste/application/PhabricatorApplicationPaste.php @@ -34,7 +34,7 @@ final class PhabricatorApplicationPaste extends PhabricatorApplication { return array( '/P(?P\d+)' => 'PhabricatorPasteViewController', '/paste/' => array( - '' => 'PhabricatorPasteListController', + '' => 'PhabricatorPasteEditController', 'filter/(?P\w+)/' => 'PhabricatorPasteListController', ), ); diff --git a/src/applications/paste/controller/PhabricatorPasteController.php b/src/applications/paste/controller/PhabricatorPasteController.php index cf0a10c1f6..7aaa584fd0 100644 --- a/src/applications/paste/controller/PhabricatorPasteController.php +++ b/src/applications/paste/controller/PhabricatorPasteController.php @@ -28,7 +28,7 @@ abstract class PhabricatorPasteController extends PhabricatorController { } $nav->addLabel('Create'); - $nav->addFilter('create', 'New Paste'); + $nav->addFilter('edit', 'New Paste', $this->getApplicationURI()); $nav->addSpacer(); $nav->addLabel('Pastes'); diff --git a/src/applications/paste/controller/PhabricatorPasteEditController.php b/src/applications/paste/controller/PhabricatorPasteEditController.php new file mode 100644 index 0000000000..e9d15ad541 --- /dev/null +++ b/src/applications/paste/controller/PhabricatorPasteEditController.php @@ -0,0 +1,154 @@ +getRequest(); + $user = $request->getUser(); + + $paste = new PhabricatorPaste(); + $title = 'Create Paste'; + + $parent_id = $request->getStr('parent'); + $parent = null; + if ($parent_id) { + // NOTE: If the Paste is forked from a paste which the user no longer + // has permission to see, we still let them edit it. + $parent = id(new PhabricatorPasteQuery()) + ->setViewer($user) + ->withIDs(array($parent_id)) + ->execute(); + $parent = head($parent); + + if ($parent) { + $paste->setParentPHID($parent->getPHID()); + } + } + + $text = null; + $e_text = true; + $errors = array(); + if ($request->isFormPost()) { + $text = $request->getStr('text'); + if (!strlen($text)) { + $e_text = 'Required'; + $errors[] = 'The paste may not be blank.'; + } else { + $e_text = null; + } + + $paste->setTitle($request->getStr('title')); + $paste->setLanguage($request->getStr('language')); + + if (!$errors) { + $paste_file = PhabricatorFile::newFromFileData( + $text, + array( + 'name' => $title, + 'mime-type' => 'text/plain; charset=utf-8', + 'authorPHID' => $user->getPHID(), + )); + $paste->setFilePHID($paste_file->getPHID()); + $paste->setAuthorPHID($user->getPHID()); + $paste->save(); + + return id(new AphrontRedirectResponse())->setURI($paste->getURI()); + } + } else { + if ($parent) { + $paste->setTitle('Fork of '.$parent->getFullName()); + $paste->setLanguage($parent->getLanguage()); + + $parent_file = id(new PhabricatorFile())->loadOneWhere( + 'phid = %s', + $parent->getFilePHID()); + $text = $parent_file->loadFileData(); + } + } + + $error_view = null; + if ($errors) { + $error_view = id(new AphrontErrorView()) + ->setTitle('A fatal omission!') + ->setErrors($errors); + } + + $form = new AphrontFormView(); + $form->setFlexible(true); + + $langs = array( + '' => '(Detect With Wizardly Powers)', + ) + PhabricatorEnv::getEnvConfig('pygments.dropdown-choices'); + + $submit = id(new AphrontFormSubmitControl()) + ->setValue('Create Paste'); + + $form + ->setUser($user) + ->addHiddenInput('parent', $parent_id) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel('Title') + ->setValue($paste->getTitle()) + ->setName('title')) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel('Language') + ->setName('language') + ->setValue($paste->getLanguage()) + ->setOptions($langs)) + ->appendChild( + id(new AphrontFormTextAreaControl()) + ->setLabel('Text') + ->setError($e_text) + ->setValue($text) + ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) + ->setCustomClass('PhabricatorMonospaced') + ->setName('text')) + + /* TODO: Doesn't have any useful options yet. + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setLabel('Visible To') + ->setUser($user) + ->setValue( + $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW)) + ->setName('policy')) + */ + + ->appendChild($submit); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('edit'); + $nav->appendChild( + array( + id(new PhabricatorHeaderView())->setHeader('Create Paste'), + $error_view, + $form, + )); + + return $this->buildApplicationPage( + $nav, + array( + 'title' => $title, + 'device' => true, + )); + } + +} diff --git a/src/applications/paste/controller/PhabricatorPasteListController.php b/src/applications/paste/controller/PhabricatorPasteListController.php index cf8f633f01..859ab21f1e 100644 --- a/src/applications/paste/controller/PhabricatorPasteListController.php +++ b/src/applications/paste/controller/PhabricatorPasteListController.php @@ -17,282 +17,60 @@ */ final class PhabricatorPasteListController extends PhabricatorPasteController { + private $filter; - private $errorView; - private $errorText; - private $paste; - private $pasteText; - - private function setFilter($filter) { - $this->filter = $filter; - return $this; - } - private function getFilter() { - return $this->filter; - } - - private function setErrorView($error_view) { - $this->errorView = $error_view; - return $this; - } - - private function getErrorView() { - return $this->errorView; - } - - private function setErrorText($error_text) { - $this->errorText = $error_text; - return $this; - } - private function getErrorText() { - return $this->errorText; - } - - private function setPaste(PhabricatorPaste $paste) { - $this->paste = $paste; - return $this; - } - private function getPaste() { - return $this->paste; - } - - private function setPasteText($paste_text) { - $this->pasteText = $paste_text; - return $this; - } - private function getPasteText() { - return $this->pasteText; - } - public function willProcessRequest(array $data) { - $this->setFilter(idx($data, 'filter', 'create')); + $this->filter = idx($data, 'filter'); } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); - $pager = new AphrontCursorPagerView(); - $pager->readFromRequest($request); - $query = new PhabricatorPasteQuery(); $query->setViewer($user); - switch ($this->getFilter()) { - case 'create': - default: - // if we successfully create a paste, we redirect to view it - $created_paste_redirect = $this->processCreateRequest(); - if ($created_paste_redirect) { - return $created_paste_redirect; - } + $nav = $this->buildSideNavView(); + $filter = $nav->selectFilter($this->filter, 'my'); - $query->setLimit(10); - $paste_list = $query->execute(); - - $pager = null; - break; + switch ($filter) { case 'my': $query->withAuthorPHIDs(array($user->getPHID())); - $paste_list = $query->executeWithCursorPager($pager); + $title = 'My Pastes'; break; case 'all': - $paste_list = $query->executeWithCursorPager($pager); + $title = 'All Pastes'; break; } - $side_nav = $this->buildSideNavView(); - $side_nav->selectFilter($this->getFilter()); + $pager = new AphrontCursorPagerView(); + $pager->readFromRequest($request); + $pastes = $query->executeWithCursorPager($pager); - if ($this->getErrorView()) { - $side_nav->appendChild($this->getErrorView()); - } - switch ($this->getFilter()) { - case 'create': - default: - $side_nav->appendChild($this->renderCreatePaste()); - $see_all = phutil_render_tag( - 'a', - array( - 'href' => '/paste/filter/all', - ), - 'See all Pastes'); - $header = "Recent Pastes"; - break; - case 'my': - $header = 'Your Pastes'; - break; - case 'all': - $header = 'All Pastes'; - break; - } - - $this->loadHandles(mpull($paste_list, 'getAuthorPHID')); - - $list = $this->buildPasteList($paste_list); - $list->setHeader($header); + $list = $this->buildPasteList($pastes); + $list->setHeader($title); $list->setPager($pager); - $side_nav->appendChild($list); + $nav->appendChild($list); return $this->buildApplicationPage( - $side_nav, + $nav, array( - 'title' => 'Paste', + 'title' => $title, 'device' => true, ) ); } - private function processCreateRequest() { - $request = $this->getRequest(); - $user = $request->getUser(); - $fork = $request->getInt('fork'); - - $error_view = null; - $e_text = true; - $new_paste = new PhabricatorPaste(); - $new_paste_text = null; - $new_paste_language = PhabricatorEnv::getEnvConfig( - 'pygments.dropdown-default'); - - if ($request->isFormPost()) { - $errors = array(); - - $text = $request->getStr('text'); - if (!strlen($text)) { - $e_text = 'Required'; - $errors[] = 'The paste may not be blank.'; - } else { - $e_text = null; - } - - $parent_phid = $request->getStr('parent'); - if ($parent_phid) { - $parent = id(new PhabricatorPaste())->loadOneWhere('phid = %s', - $parent_phid); - if ($parent) { - $new_paste->setParentPHID($parent->getPHID()); - } - } - - $title = $request->getStr('title'); - $new_paste->setTitle($title); - - $new_paste_language = $request->getStr('language'); - - if (!$errors) { - if ($new_paste_language == 'infer') { - // If it's infer, store an empty string. Otherwise, store the - // language name. We do this so we can refer to 'infer' elsewhere - // in the code (such as default value) while retaining backwards - // compatibility with old posts with no language stored. - $new_paste_language = ''; - } - $new_paste->setLanguage($new_paste_language); - - $new_paste_file = PhabricatorFile::newFromFileData( - $text, - array( - 'name' => $title, - 'mime-type' => 'text/plain; charset=utf-8', - 'authorPHID' => $user->getPHID(), - )); - $new_paste->setFilePHID($new_paste_file->getPHID()); - $new_paste->setAuthorPHID($user->getPHID()); - $new_paste->save(); - - return id(new AphrontRedirectResponse()) - ->setURI('/P'.$new_paste->getID()); - } else { - $error_view = new AphrontErrorView(); - $error_view->setErrors($errors); - $error_view->setTitle('A problem has occurred!'); - } - } else if ($fork) { - $fork_paste = id(new PhabricatorPaste())->load($fork); - if ($fork_paste) { - $new_paste->setTitle('Fork of '.$fork_paste->getID().': '. - $fork_paste->getTitle()); - $fork_file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $fork_paste->getFilePHID()); - $new_paste_text = $fork_file->loadFileData(); - $new_paste_language = nonempty($fork_paste->getLanguage(), 'infer'); - $new_paste->setParentPHID($fork_paste->getPHID()); - } - } - $this->setErrorView($error_view); - $this->setErrorText($e_text); - $this->setPasteText($new_paste_text); - $new_paste->setLanguage($new_paste_language); - $this->setPaste($new_paste); - } - - private function renderCreatePaste() { - $request = $this->getRequest(); - $user = $request->getUser(); - - $new_paste = $this->getPaste(); - - $form = new AphrontFormView(); - $form->setFlexible(true); - - $available_languages = PhabricatorEnv::getEnvConfig( - 'pygments.dropdown-choices'); - asort($available_languages); - $language_select = id(new AphrontFormSelectControl()) - ->setLabel('Language') - ->setName('language') - ->setValue($new_paste->getLanguage()) - ->setOptions($available_languages); - - $form - ->setUser($user) - ->setAction($request->getRequestURI()->getPath()) - ->addHiddenInput('parent', $new_paste->getParentPHID()) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel('Title') - ->setValue($new_paste->getTitle()) - ->setName('title')) - ->appendChild($language_select) - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setLabel('Text') - ->setError($this->getErrorText()) - ->setValue($this->getPasteText()) - ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) - ->setName('text')) - - /* TODO: Doesn't have any useful options yet. - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setLabel('Visible To') - ->setUser($user) - ->setValue( - $new_paste->getPolicy(PhabricatorPolicyCapability::CAN_VIEW)) - ->setName('policy')) - */ - - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton('/paste/') - ->setValue('Create Paste')); - - return array( - id(new PhabricatorHeaderView())->setHeader('Create Paste'), - $form, - ); - } - private function buildPasteList(array $pastes) { assert_instances_of($pastes, 'PhabricatorPaste'); $user = $this->getRequest()->getUser(); + $this->loadHandles(mpull($pastes, 'getAuthorPHID')); + $list = new PhabricatorObjectItemListView(); foreach ($pastes as $paste) { $created = phabricator_datetime($paste->getDateCreated(), $user); diff --git a/src/applications/paste/controller/PhabricatorPasteViewController.php b/src/applications/paste/controller/PhabricatorPasteViewController.php index 3e1cf1d3d4..ce975f83cf 100644 --- a/src/applications/paste/controller/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/PhabricatorPasteViewController.php @@ -97,7 +97,7 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { id(new PhabricatorActionView()) ->setName(pht('Fork This Paste')) ->setIcon('fork') - ->setHref($this->getApplicationURI('?fork='.$paste->getID()))) + ->setHref($this->getApplicationURI('?parent='.$paste->getID()))) ->addAction( id(new PhabricatorActionView()) ->setName(pht('View Raw File')) diff --git a/src/applications/paste/storage/PhabricatorPaste.php b/src/applications/paste/storage/PhabricatorPaste.php index 3fdfe1c114..b3b73546e4 100644 --- a/src/applications/paste/storage/PhabricatorPaste.php +++ b/src/applications/paste/storage/PhabricatorPaste.php @@ -26,6 +26,10 @@ final class PhabricatorPaste extends PhabricatorPasteDAO protected $language; protected $parentPHID; + public function getURI() { + return '/P'.$this->getID(); + } + public function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -54,7 +58,7 @@ final class PhabricatorPaste extends PhabricatorPasteDAO public function getFullName() { $title = $this->getTitle(); if (!$title) { - $title = 'Untitled Masterwork'; + $title = '(An Untitled Masterwork)'; } return 'P'.$this->getID().' '.$title; } diff --git a/src/applications/phid/handle/PhabricatorObjectHandleData.php b/src/applications/phid/handle/PhabricatorObjectHandleData.php index 7d59dfede3..52e03a8cbc 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php @@ -482,7 +482,7 @@ final class PhabricatorObjectHandleData { } else { $paste = $pastes[$phid]; $handle->setName($paste->getTitle()); - $handle->setFullName('P'.$paste->getID().': '.$paste->getTitle()); + $handle->setFullName($paste->getFullName()); $handle->setURI('/P'.$paste->getID()); $handle->setComplete(true); } diff --git a/src/view/form/control/AphrontFormTextAreaControl.php b/src/view/form/control/AphrontFormTextAreaControl.php index bcd5ebd6d4..da581b6191 100644 --- a/src/view/form/control/AphrontFormTextAreaControl.php +++ b/src/view/form/control/AphrontFormTextAreaControl.php @@ -25,7 +25,7 @@ final class AphrontFormTextAreaControl extends AphrontFormControl { private $height; private $readOnly; private $enableDragAndDropFileUploads; - + private $customClass; public function setHeight($height) { $this->height = $height; @@ -50,6 +50,11 @@ final class AphrontFormTextAreaControl extends AphrontFormControl { return $this; } + public function setCustomClass($custom_class) { + $this->customClass = $custom_class; + return $this; + } + protected function renderInput() { $height_class = null; @@ -61,6 +66,11 @@ final class AphrontFormTextAreaControl extends AphrontFormControl { break; } + $classes = array(); + $classes[] = $height_class; + $classes[] = $this->customClass; + $classes = trim(implode(' ', $classes)); + $id = $this->getID(); if ($this->enableDragAndDropFileUploads) { if (!$id) { @@ -81,7 +91,7 @@ final class AphrontFormTextAreaControl extends AphrontFormControl { 'name' => $this->getName(), 'disabled' => $this->getDisabled() ? 'disabled' : null, 'readonly' => $this->getReadonly() ? 'readonly' : null, - 'class' => $height_class, + 'class' => $classes, 'style' => $this->getControlStyle(), 'id' => $id, ),