From 59c5cd95e7c9f9a7011f4ddd2b9efa37bbc5958a Mon Sep 17 00:00:00 2001 From: lkassianik Date: Fri, 13 Nov 2015 14:36:58 -0800 Subject: [PATCH] Remarkup links to link to short url instead of long and fix commenting on Phurl's Summary: Ref T6049, remarkup links to use short URLs and make commenting on Phurl's actually work Test Plan: - Create Phurl `U123` - Comment on that Phurl `((123))` Comment should link to `/u/123` Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T6049 Differential Revision: https://secure.phabricator.com/D14477 --- src/__phutil_library_map__.php | 2 + .../option/PhabricatorPhurlConfigOptions.php | 2 +- .../PhabricatorPhurlApplication.php | 2 + .../PhabricatorPhurlURLCommentController.php | 63 +++++++++++++++++++ .../PhabricatorPhurlURLEditController.php | 6 +- .../PhabricatorPhurlURLViewController.php | 2 +- .../editor/PhabricatorPhurlURLEditor.php | 16 ----- .../PhabricatorPhurlLinkRemarkupRule.php | 20 ++++-- .../phurl/storage/PhabricatorPhurlURL.php | 16 +++++ 9 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 src/applications/phurl/controller/PhabricatorPhurlURLCommentController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9e1c1a8742..e28eec6f6a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2669,6 +2669,7 @@ phutil_register_library_map(array( 'PhabricatorPhurlShortURLDefaultController' => 'applications/phurl/controller/PhabricatorPhurlShortURLDefaultController.php', 'PhabricatorPhurlURL' => 'applications/phurl/storage/PhabricatorPhurlURL.php', 'PhabricatorPhurlURLAccessController' => 'applications/phurl/controller/PhabricatorPhurlURLAccessController.php', + 'PhabricatorPhurlURLCommentController' => 'applications/phurl/controller/PhabricatorPhurlURLCommentController.php', 'PhabricatorPhurlURLEditController' => 'applications/phurl/controller/PhabricatorPhurlURLEditController.php', 'PhabricatorPhurlURLEditor' => 'applications/phurl/editor/PhabricatorPhurlURLEditor.php', 'PhabricatorPhurlURLListController' => 'applications/phurl/controller/PhabricatorPhurlURLListController.php', @@ -6846,6 +6847,7 @@ phutil_register_library_map(array( 'PhabricatorSpacesInterface', ), 'PhabricatorPhurlURLAccessController' => 'PhabricatorPhurlController', + 'PhabricatorPhurlURLCommentController' => 'PhabricatorPhurlController', 'PhabricatorPhurlURLEditController' => 'PhabricatorPhurlController', 'PhabricatorPhurlURLEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorPhurlURLListController' => 'PhabricatorPhurlController', diff --git a/src/applications/config/option/PhabricatorPhurlConfigOptions.php b/src/applications/config/option/PhabricatorPhurlConfigOptions.php index 378724a4b5..726540d3dd 100644 --- a/src/applications/config/option/PhabricatorPhurlConfigOptions.php +++ b/src/applications/config/option/PhabricatorPhurlConfigOptions.php @@ -16,7 +16,7 @@ final class PhabricatorPhurlConfigOptions } public function getGroup() { - return 'phurl'; + return 'apps'; } public function getOptions() { diff --git a/src/applications/phurl/application/PhabricatorPhurlApplication.php b/src/applications/phurl/application/PhabricatorPhurlApplication.php index 015ed75fe7..75ac91c8c2 100644 --- a/src/applications/phurl/application/PhabricatorPhurlApplication.php +++ b/src/applications/phurl/application/PhabricatorPhurlApplication.php @@ -46,6 +46,8 @@ final class PhabricatorPhurlApplication extends PhabricatorApplication { => 'PhabricatorPhurlURLEditController', 'edit/(?P[1-9]\d*)/' => 'PhabricatorPhurlURLEditController', + 'comment/(?P[1-9]\d*)/' + => 'PhabricatorPhurlURLCommentController', ), ), ); diff --git a/src/applications/phurl/controller/PhabricatorPhurlURLCommentController.php b/src/applications/phurl/controller/PhabricatorPhurlURLCommentController.php new file mode 100644 index 0000000000..8726a38706 --- /dev/null +++ b/src/applications/phurl/controller/PhabricatorPhurlURLCommentController.php @@ -0,0 +1,63 @@ +isFormPost()) { + return new Aphront400Response(); + } + + $viewer = $request->getViewer(); + $id = $request->getURIData('id'); + + $is_preview = $request->isPreviewRequest(); + $draft = PhabricatorDraft::buildFromRequest($request); + + $phurl = id(new PhabricatorPhurlURLQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$phurl) { + return new Aphront404Response(); + } + + $view_uri = '/'.$phurl->getMonogram(); + + $xactions = array(); + $xactions[] = id(new PhabricatorPhurlURLTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new PhabricatorPhurlURLTransactionComment()) + ->setContent($request->getStr('comment'))); + + $editor = id(new PhabricatorPhurlURLEditor()) + ->setActor($viewer) + ->setContinueOnNoEffect($request->isContinueRequest()) + ->setContentSourceFromRequest($request) + ->setIsPreview($is_preview); + + try { + $xactions = $editor->applyTransactions($phurl, $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/phurl/controller/PhabricatorPhurlURLEditController.php b/src/applications/phurl/controller/PhabricatorPhurlURLEditController.php index 5a7cd9686d..a4960d0c03 100644 --- a/src/applications/phurl/controller/PhabricatorPhurlURLEditController.php +++ b/src/applications/phurl/controller/PhabricatorPhurlURLEditController.php @@ -9,7 +9,6 @@ final class PhabricatorPhurlURLEditController $viewer = $this->getViewer(); $user_phid = $viewer->getPHID(); - $error_name = true; $error_long_url = true; $error_alias = null; $validation_exception = null; @@ -131,8 +130,6 @@ final class PhabricatorPhurlURLEditController ->setURI($url->getURI()); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; - $error_name = $ex->getShortMessage( - PhabricatorPhurlURLTransaction::TYPE_NAME); $error_long_url = $ex->getShortMessage( PhabricatorPhurlURLTransaction::TYPE_URL); $error_alias = $ex->getShortMessage( @@ -148,8 +145,7 @@ final class PhabricatorPhurlURLEditController $name = id(new AphrontFormTextControl()) ->setLabel(pht('Name')) ->setName('name') - ->setValue($name) - ->setError($error_name); + ->setValue($name); $long_url = id(new AphrontFormTextControl()) ->setLabel(pht('URL')) diff --git a/src/applications/phurl/controller/PhabricatorPhurlURLViewController.php b/src/applications/phurl/controller/PhabricatorPhurlURLViewController.php index 7702d67b7c..b0649cc62a 100644 --- a/src/applications/phurl/controller/PhabricatorPhurlURLViewController.php +++ b/src/applications/phurl/controller/PhabricatorPhurlURLViewController.php @@ -49,7 +49,7 @@ final class PhabricatorPhurlURLViewController : pht('More Cowbell'); $draft = PhabricatorDraft::newFromUserAndKey($viewer, $url->getPHID()); $comment_uri = $this->getApplicationURI( - '/phurl/comment/'.$url->getID().'/'); + '/url/comment/'.$url->getID().'/'); $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($viewer) ->setObjectPHID($url->getPHID()) diff --git a/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php b/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php index e7a3c7394b..ef7f547621 100644 --- a/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php +++ b/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php @@ -106,22 +106,6 @@ final class PhabricatorPhurlURLEditor $errors = parent::validateTransaction($object, $type, $xactions); switch ($type) { - case PhabricatorPhurlURLTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('URL name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; case PhabricatorPhurlURLTransaction::TYPE_ALIAS: $overdrawn = $this->validateIsTextFieldTooLong( $object->getName(), diff --git a/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php b/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php index bdb6c1959f..aca7c14dc7 100644 --- a/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php +++ b/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php @@ -7,7 +7,7 @@ final class PhabricatorPhurlLinkRemarkupRule extends PhutilRemarkupRule { } public function apply($text) { - // `((U123))` remarkup link to `/u/123` + // `((123))` remarkup link to `/u/123` // `((alias))` remarkup link to `/u/alias` return preg_replace_callback( '/\(\(([^ )]+)\)\)/', @@ -25,8 +25,15 @@ final class PhabricatorPhurlLinkRemarkupRule extends PhutilRemarkupRule { } $ref = $matches[1]; + $monogram = null; + $is_monogram = '/^U(?P[1-9]\d*)/'; - if (ctype_digit($ref)) { + if (preg_match($is_monogram, $ref, $monogram)) { + $phurls = id(new PhabricatorPhurlURLQuery()) + ->setViewer($viewer) + ->withIDs(array($monogram[1])) + ->execute(); + } else if (ctype_digit($ref)) { $phurls = id(new PhabricatorPhurlURLQuery()) ->setViewer($viewer) ->withIDs(array($ref)) @@ -42,16 +49,19 @@ final class PhabricatorPhurlLinkRemarkupRule extends PhutilRemarkupRule { if ($phurl) { if ($text_mode) { - return $phurl->getName().' <'.$phurl->getLongURL().'>'; + return $phurl->getDisplayName(). + ' <'. + $phurl->getRedirectURI(). + '>'; } $link = phutil_tag( 'a', array( - 'href' => $phurl->getLongURL(), + 'href' => $phurl->getRedirectURI(), 'target' => '_blank', ), - $phurl->getName()); + $phurl->getDisplayName()); return $this->getEngine()->storeText($link); } else { diff --git a/src/applications/phurl/storage/PhabricatorPhurlURL.php b/src/applications/phurl/storage/PhabricatorPhurlURL.php index 35c5382d52..cc90b6ce6a 100644 --- a/src/applications/phurl/storage/PhabricatorPhurlURL.php +++ b/src/applications/phurl/storage/PhabricatorPhurlURL.php @@ -79,6 +79,22 @@ final class PhabricatorPhurlURL extends PhabricatorPhurlDAO return isset($allowed_protocols[$uri->getProtocol()]); } + public function getDisplayName() { + if ($this->getName()) { + return $this->getName(); + } else { + return $this->getMonogram(); + } + } + + public function getRedirectURI() { + if (strlen($this->getAlias())) { + return '/u/'.$this->getAlias(); + } else { + return '/u/'.$this->getID(); + } + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */