From 38d30af362e60452c1f2a89e65455cbd0a4713f6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 19 Jul 2019 08:57:50 -0700 Subject: [PATCH] Give "Auth Messages" a view/detail state before users customize them Summary: Depends on D20663. Ref T13343. Currently, if an Auth message hasn't been customized yet, clicking the message type takes you straight to an edit screen to create a message. If an auth message has already been customized, you go to a detail screen instead. Since there's no detail screen on the "create for the first time" flow, we don't have anywhere to put a more detailed description or a preview of a default value. Add a view screen that works if a message is "empty" so we can add this stuff. (The only reason we don't already have this is that it took a little work to build; this also generally improves the consistency and predictability of this interface.) Test Plan: {F6607665} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13343 Differential Revision: https://secure.phabricator.com/D20664 --- .../PhabricatorAuthApplication.php | 2 +- .../PhabricatorAuthMessageListController.php | 5 +- .../PhabricatorAuthMessageViewController.php | 87 +++++++++++++++---- 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index 3446ad597d..6d4a70b735 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -108,7 +108,7 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'PhabricatorAuthMessageListController', $this->getEditRoutePattern('edit/') => 'PhabricatorAuthMessageEditController', - '(?P[1-9]\d*)/' => + '(?P[^/]+)/' => 'PhabricatorAuthMessageViewController', ), diff --git a/src/applications/auth/controller/message/PhabricatorAuthMessageListController.php b/src/applications/auth/controller/message/PhabricatorAuthMessageListController.php index a3c518ab36..7981a03f16 100644 --- a/src/applications/auth/controller/message/PhabricatorAuthMessageListController.php +++ b/src/applications/auth/controller/message/PhabricatorAuthMessageListController.php @@ -19,11 +19,14 @@ final class PhabricatorAuthMessageListController $list = new PHUIObjectItemListView(); foreach ($types as $type) { $message = idx($messages, $type->getMessageTypeKey()); + if ($message) { $href = $message->getURI(); $name = $message->getMessageTypeDisplayName(); } else { - $href = '/auth/message/edit/?messageKey='.$type->getMessageTypeKey(); + $href = urisprintf( + '/auth/message/%s/', + $type->getMessageTypeKey()); $name = $type->getDisplayName(); } diff --git a/src/applications/auth/controller/message/PhabricatorAuthMessageViewController.php b/src/applications/auth/controller/message/PhabricatorAuthMessageViewController.php index db7e7e65e0..fab5dcafb0 100644 --- a/src/applications/auth/controller/message/PhabricatorAuthMessageViewController.php +++ b/src/applications/auth/controller/message/PhabricatorAuthMessageViewController.php @@ -9,26 +9,61 @@ final class PhabricatorAuthMessageViewController $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $message = id(new PhabricatorAuthMessageQuery()) - ->setViewer($viewer) - ->withIDs(array($request->getURIData('id'))) - ->executeOne(); - if (!$message) { - return new Aphront404Response(); + // The "id" in the URI may either be an actual storage record ID (if a + // message has already been created) or a message type key (for a message + // type which does not have a record yet). + + // This flow allows messages which have not been set yet to have a detail + // page (so users can get detailed information about the message and see + // any default value). + + $id = $request->getURIData('id'); + if (ctype_digit($id)) { + $message = id(new PhabricatorAuthMessageQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$message) { + return new Aphront404Response(); + } + } else { + $types = PhabricatorAuthMessageType::getAllMessageTypes(); + if (!isset($types[$id])) { + return new Aphront404Response(); + } + + // If this message type already has a storage record, redirect to the + // canonical page for the record. + $message = id(new PhabricatorAuthMessageQuery()) + ->setViewer($viewer) + ->withMessageKeys(array($id)) + ->executeOne(); + if ($message) { + $message_uri = $message->getURI(); + return id(new AphrontRedirectResponse())->setURI($message_uri); + } + + // Otherwise, create an empty placeholder message object with the + // appropriate message type. + $message = PhabricatorAuthMessage::initializeNewMessage($types[$id]); } $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($message->getObjectName()) + ->addTextCrumb($message->getMessageType()->getDisplayName()) ->setBorder(true); $header = $this->buildHeaderView($message); $properties = $this->buildPropertiesView($message); $curtain = $this->buildCurtain($message); - $timeline = $this->buildTransactionTimeline( - $message, - new PhabricatorAuthMessageTransactionQuery()); - $timeline->setShouldTerminate(true); + if ($message->getID()) { + $timeline = $this->buildTransactionTimeline( + $message, + new PhabricatorAuthMessageTransactionQuery()); + $timeline->setShouldTerminate(true); + } else { + $timeline = null; + } $view = id(new PHUITwoColumnView()) ->setHeader($header) @@ -69,12 +104,14 @@ final class PhabricatorAuthMessageViewController pht('Description'), $message->getMessageType()->getShortDescription()); - $view->addSectionHeader( - pht('Message Preview'), - PHUIPropertyListView::ICON_SUMMARY); + if (strlen($message->getMessageText())) { + $view->addSectionHeader( + pht('Message Preview'), + PHUIPropertyListView::ICON_SUMMARY); - $view->addTextContent( - new PHUIRemarkupView($viewer, $message->getMessageText())); + $view->addTextContent( + new PHUIRemarkupView($viewer, $message->getMessageText())); + } return $view; } @@ -88,13 +125,27 @@ final class PhabricatorAuthMessageViewController $message, PhabricatorPolicyCapability::CAN_EDIT); + if ($id) { + $edit_uri = urisprintf('message/edit/%s/', $id); + $edit_name = pht('Edit Message'); + } else { + $edit_uri = urisprintf('message/edit/'); + $params = array( + 'messageKey' => $message->getMessageKey(), + ); + $edit_uri = new PhutilURI($edit_uri, $params); + + $edit_name = pht('Customize Message'); + } + $edit_uri = $this->getApplicationURI($edit_uri); + $curtain = $this->newCurtainView($message); $curtain->addAction( id(new PhabricatorActionView()) - ->setName(pht('Edit Message')) + ->setName($edit_name) ->setIcon('fa-pencil') - ->setHref($this->getApplicationURI("message/edit/{$id}/")) + ->setHref($edit_uri) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit));