From 51ed95c00b8de3a7e552858b215aaa25e9871bc5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 22 Jan 2016 05:33:21 -0800 Subject: [PATCH] Give profile menus more straightforward hide/disable/delete/default interactions Summary: Ref T10054. - Just let users delete non-builtin items. - Let users choose a default item explicitly. - Do a better job of cleaning up items which no longer exist or belong to uninstalled applications. (NOTE) This has one user-facing change: workboards are no longer the default on projects with workboards. I think this is probably OK since we're giving users a ton of new toys at the same time, but I'll write some docs at least. Test Plan: - Deleted custom items. - Disabled/enabled builtin items. - Made various things defaults. - Uninstalled Maniphest, saw Workboards tab disappear entirely. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10054 Differential Revision: https://secure.phabricator.com/D15089 --- .../base/PhabricatorApplication.php | 1 + .../PhabricatorProjectBoardViewController.php | 11 +- .../PhabricatorProjectViewController.php | 17 +- .../PhabricatorProjectDetailsProfilePanel.php | 5 + ...habricatorProjectWorkboardProfilePanel.php | 5 + .../engine/PhabricatorProfilePanelEngine.php | 239 ++++++++++++++++-- .../profilepanel/PhabricatorProfilePanel.php | 5 + .../PhabricatorProfilePanelConfiguration.php | 16 +- 8 files changed, 251 insertions(+), 48 deletions(-) diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 4fc3e10314..0ed4917800 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -641,6 +641,7 @@ abstract class PhabricatorApplication return array( '(?Pview)/(?P[^/]+)/' => $controller, '(?Phide)/(?P[^/]+)/' => $controller, + '(?Pdefault)/(?P[^/]+)/' => $controller, '(?Pconfigure)/' => $controller, '(?Preorder)/' => $controller, '(?Pedit)/'.$edit_route => $controller, diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 571d5b866d..1b2d9d65e2 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -712,16 +712,15 @@ final class PhabricatorProjectBoardViewController pht('Import Columns'), pht('Import board columns from another project.')); - $dialog = id(new AphrontDialogView()) - ->setUser($this->getRequest()->getUser()) + + $cancel_uri = $this->getApplicationURI('profile/'.$project->getID().'/'); + + return $this->newDialog() ->setTitle(pht('New Workboard')) ->addSubmitButton('Continue') - ->addCancelButton($this->getApplicationURI('view/'.$project->getID().'/')) + ->addCancelButton($cancel_uri) ->appendParagraph($instructions) ->appendChild($new_selector); - - return id(new AphrontDialogResponse()) - ->setDialog($dialog); } private function noAccessDialog(PhabricatorProject $project) { diff --git a/src/applications/project/controller/PhabricatorProjectViewController.php b/src/applications/project/controller/PhabricatorProjectViewController.php index 087971878b..412565c056 100644 --- a/src/applications/project/controller/PhabricatorProjectViewController.php +++ b/src/applications/project/controller/PhabricatorProjectViewController.php @@ -17,21 +17,14 @@ final class PhabricatorProjectViewController } $project = $this->getProject(); - $columns = id(new PhabricatorProjectColumnQuery()) - ->setViewer($viewer) - ->withProjectPHIDs(array($project->getPHID())) - ->execute(); - if ($columns) { - $controller = 'board'; - } else { - $controller = 'profile'; - } + $engine = $this->getProfilePanelEngine(); + $default = $engine->getDefaultPanel(); - switch ($controller) { - case 'board': + switch ($default->getBuiltinKey()) { + case PhabricatorProject::PANEL_WORKBOARD: $controller_object = new PhabricatorProjectBoardViewController(); break; - case 'profile': + case PhabricatorProject::PANEL_PROFILE: default: $controller_object = new PhabricatorProjectProfileController(); break; diff --git a/src/applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php b/src/applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php index 97ba3a1aaa..07e6ce4c10 100644 --- a/src/applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php +++ b/src/applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php @@ -13,6 +13,11 @@ final class PhabricatorProjectDetailsProfilePanel return pht('Project Details'); } + public function canMakeDefault( + PhabricatorProfilePanelConfiguration $config) { + return true; + } + public function getDisplayName( PhabricatorProfilePanelConfiguration $config) { $name = $config->getPanelProperty('name'); diff --git a/src/applications/project/profilepanel/PhabricatorProjectWorkboardProfilePanel.php b/src/applications/project/profilepanel/PhabricatorProjectWorkboardProfilePanel.php index 02b54537e5..f8b2abfc2f 100644 --- a/src/applications/project/profilepanel/PhabricatorProjectWorkboardProfilePanel.php +++ b/src/applications/project/profilepanel/PhabricatorProjectWorkboardProfilePanel.php @@ -13,6 +13,11 @@ final class PhabricatorProjectWorkboardProfilePanel return pht('Workboard'); } + public function canMakeDefault( + PhabricatorProfilePanelConfiguration $config) { + return true; + } + public function getDisplayName( PhabricatorProfilePanelConfiguration $config) { $name = $config->getPanelProperty('name'); diff --git a/src/applications/search/engine/PhabricatorProfilePanelEngine.php b/src/applications/search/engine/PhabricatorProfilePanelEngine.php index 9ab8d6c591..ea4cefce56 100644 --- a/src/applications/search/engine/PhabricatorProfilePanelEngine.php +++ b/src/applications/search/engine/PhabricatorProfilePanelEngine.php @@ -5,6 +5,7 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { private $viewer; private $profileObject; private $panels; + private $defaultPanel; private $controller; private $navigation; @@ -35,6 +36,17 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { return $this->controller; } + private function setDefaultPanel( + PhabricatorProfilePanelConfiguration $default_panel) { + $this->defaultPanel = $default_panel; + return $this; + } + + public function getDefaultPanel() { + $this->loadPanels(); + return $this->defaultPanel; + } + abstract protected function getPanelURI($path); protected function isPanelEngineConfigurable() { @@ -89,6 +101,7 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { case 'view': case 'info': case 'hide': + case 'default': case 'builtin': if (!$selected_panel) { return new Aphront404Response(); @@ -122,6 +135,11 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { case 'hide': $content = $this->buildPanelHideContent($selected_panel); break; + case 'default': + $content = $this->buildPanelDefaultContent( + $selected_panel, + $panel_list); + break; case 'edit': $content = $this->buildPanelEditContent(); break; @@ -225,7 +243,15 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { foreach ($stored_panels as $stored_panel) { $builtin_key = $stored_panel->getBuiltinKey(); if ($builtin_key !== null) { - $panels[$builtin_key] = $stored_panel; + // If this builtin actually exists, replace the builtin with the + // stored configuration. Otherwise, we're just going to drop the + // stored config: it corresponds to an out-of-date or uninstalled + // panel. + if (isset($panels[$builtin_key])) { + $panels[$builtin_key] = $stored_panel; + } else { + continue; + } } else { $panels[] = $stored_panel; } @@ -243,6 +269,33 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { // partially keyed. $panels = array_values($panels); + + // Make sure exactly one valid panel is marked as default. + $default = null; + $first = null; + foreach ($panels as $panel) { + if (!$panel->canMakeDefault()) { + continue; + } + + if ($panel->isDefault()) { + $default = $panel; + break; + } + + if ($first === null) { + $first = $panel; + } + } + + if (!$default) { + $default = $first; + } + + if ($default) { + $this->setDefaultPanel($default); + } + return $panels; } @@ -542,6 +595,31 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { 'key' => nonempty($id, $builtin_key), )); + if ($id) { + $default_uri = $this->getPanelURI("default/{$id}/"); + } else { + $default_uri = $this->getPanelURI("default/{$builtin_key}/"); + } + + if ($panel->isDefault()) { + $default_icon = 'fa-thumb-tack green'; + $default_text = pht('Current Default'); + } else if ($panel->canMakeDefault()) { + $default_icon = 'fa-thumb-tack'; + $default_text = pht('Make Default'); + } else { + $default_text = null; + } + + if ($default_text !== null) { + $item->addAction( + id(new PHUIListItemView()) + ->setHref($default_uri) + ->setWorkflow(true) + ->setName($default_text) + ->setIcon($default_icon)); + } + if ($id) { $item->setHref($this->getPanelURI("edit/{$id}/")); $hide_uri = $this->getPanelURI("hide/{$id}/"); @@ -550,16 +628,27 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { $hide_uri = $this->getPanelURI("hide/{$builtin_key}/"); } + if ($panel->isDisabled()) { + $hide_icon = 'fa-plus'; + $hide_text = pht('Show'); + } else if ($panel->getBuiltinKey() !== null) { + $hide_icon = 'fa-times'; + $hide_text = pht('Disable'); + } else { + $hide_icon = 'fa-times'; + $hide_text = pht('Delete'); + } + $item->addAction( id(new PHUIListItemView()) ->setHref($hide_uri) ->setWorkflow(true) - ->setIcon(pht('fa-eye'))); + ->setName($hide_text) + ->setIcon($hide_icon)); } if ($panel->isDisabled()) { $item->setDisabled(true); - $item->addIcon('fa-times grey', pht('Disabled')); } $list->addItem($item); @@ -707,18 +796,130 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { $configuration, PhabricatorPolicyCapability::CAN_EDIT); + if ($configuration->getBuiltinKey() === null) { + $new_value = null; + + $title = pht('Delete Menu Item'); + $body = pht('Delete this menu item?'); + $button = pht('Delete Menu Item'); + } else if ($configuration->isDisabled()) { + $new_value = PhabricatorProfilePanelConfiguration::VISIBILITY_VISIBLE; + + $title = pht('Enable Menu Item'); + $body = pht( + 'Enable this menu item? It will appear in the menu again.'); + $button = pht('Enable Menu Item'); + } else { + $new_value = PhabricatorProfilePanelConfiguration::VISIBILITY_DISABLED; + + $title = pht('Disable Menu Item'); + $body = pht( + 'Disable this menu item? It will no longer appear in the menu, but '. + 'you can re-enable it later.'); + $button = pht('Disable Menu Item'); + } + $v_visibility = $configuration->getVisibility(); if ($request->isFormPost()) { - $v_visibility = $request->getStr('visibility'); + if ($new_value === null) { + $configuration->delete(); + } else { + $type_visibility = + PhabricatorProfilePanelConfigurationTransaction::TYPE_VISIBILITY; - $type_visibility = - PhabricatorProfilePanelConfigurationTransaction::TYPE_VISIBILITY; + $xactions = array(); + $xactions[] = id(new PhabricatorProfilePanelConfigurationTransaction()) + ->setTransactionType($type_visibility) + ->setNewValue($new_value); + + $editor = id(new PhabricatorProfilePanelEditor()) + ->setContentSourceFromRequest($request) + ->setActor($viewer) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($configuration, $xactions); + } + + return id(new AphrontRedirectResponse()) + ->setURI($this->getConfigureURI()); + } + + return $controller->newDialog() + ->setTitle($title) + ->appendParagraph($body) + ->addCancelButton($this->getConfigureURI()) + ->addSubmitButton($button); + } + + private function buildPanelDefaultContent( + PhabricatorProfilePanelConfiguration $configuration, + array $panels) { + + $controller = $this->getController(); + $request = $controller->getRequest(); + $viewer = $this->getViewer(); + + PhabricatorPolicyFilter::requireCapability( + $viewer, + $configuration, + PhabricatorPolicyCapability::CAN_EDIT); + + $done_uri = $this->getConfigureURI(); + + if (!$configuration->canMakeDefault()) { + return $controller->newDialog() + ->setTitle(pht('Not Defaultable')) + ->appendParagraph( + pht( + 'This item can not be set as the default item. This is usually '. + 'because the item has no page of its own, or links to an '. + 'external page.')) + ->addCancelButton($done_uri); + } + + if ($configuration->isDefault()) { + return $controller->newDialog() + ->setTitle(pht('Already Default')) + ->appendParagraph( + pht( + 'This item is already set as the default item for this menu.')) + ->addCancelButton($done_uri); + } + + $type_visibility = + PhabricatorProfilePanelConfigurationTransaction::TYPE_VISIBILITY; + + $v_visible = PhabricatorProfilePanelConfiguration::VISIBILITY_VISIBLE; + $v_default = PhabricatorProfilePanelConfiguration::VISIBILITY_DEFAULT; + + if ($request->isFormPost()) { + // First, mark any existing default panels as merely visible. + foreach ($panels as $panel) { + if (!$panel->isDefault()) { + continue; + } + + $xactions = array(); + + $xactions[] = id(new PhabricatorProfilePanelConfigurationTransaction()) + ->setTransactionType($type_visibility) + ->setNewValue($v_visible); + + $editor = id(new PhabricatorProfilePanelEditor()) + ->setContentSourceFromRequest($request) + ->setActor($viewer) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($panel, $xactions); + } + + // Now, make this panel the default. $xactions = array(); $xactions[] = id(new PhabricatorProfilePanelConfigurationTransaction()) ->setTransactionType($type_visibility) - ->setNewValue($v_visibility); + ->setNewValue($v_default); $editor = id(new PhabricatorProfilePanelEditor()) ->setContentSourceFromRequest($request) @@ -728,25 +929,17 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { ->applyTransactions($configuration, $xactions); return id(new AphrontRedirectResponse()) - ->setURI($this->getConfigureURI()); + ->setURI($done_uri); } - $map = PhabricatorProfilePanelConfiguration::getVisibilityNameMap(); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendControl( - id(new AphrontFormSelectControl()) - ->setName('visibility') - ->setLabel(pht('Visibility')) - ->setValue($v_visibility) - ->setOptions($map)); - return $controller->newDialog() - ->setTitle(pht('Change Item Visibility')) - ->appendForm($form) - ->addCancelButton($this->getConfigureURI()) - ->addSubmitButton(pht('Save Changes')); + ->setTitle(pht('Make Default')) + ->appendParagraph( + pht( + 'Set this item as the default for this menu? Users arriving on '. + 'this page will be shown the content of this item by default.')) + ->addCancelButton($done_uri) + ->addSubmitButton(pht('Make Default')); } protected function newPanel() { diff --git a/src/applications/search/profilepanel/PhabricatorProfilePanel.php b/src/applications/search/profilepanel/PhabricatorProfilePanel.php index 43637a6678..49159dbf8d 100644 --- a/src/applications/search/profilepanel/PhabricatorProfilePanel.php +++ b/src/applications/search/profilepanel/PhabricatorProfilePanel.php @@ -30,6 +30,11 @@ abstract class PhabricatorProfilePanel extends Phobject { return false; } + public function canMakeDefault( + PhabricatorProfilePanelConfiguration $config) { + return false; + } + public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; diff --git a/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php b/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php index a4813592f6..127ce6b688 100644 --- a/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php +++ b/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php @@ -17,6 +17,7 @@ final class PhabricatorProfilePanelConfiguration private $profileObject = self::ATTACHABLE; private $panel = self::ATTACHABLE; + const VISIBILITY_DEFAULT = 'default'; const VISIBILITY_VISIBLE = 'visible'; const VISIBILITY_DISABLED = 'disabled'; @@ -56,13 +57,6 @@ final class PhabricatorProfilePanelConfiguration ) + parent::getConfiguration(); } - public static function getVisibilityNameMap() { - return array( - self::VISIBILITY_VISIBLE => pht('Visible'), - self::VISIBILITY_DISABLED => pht('Disabled'), - ); - } - public function generatePHID() { return PhabricatorPHID::generateNewPHID( PhabricatorProfilePanelPHIDType::TYPECONST); @@ -107,6 +101,10 @@ final class PhabricatorProfilePanelConfiguration return $this->getPanel()->getDisplayName($this); } + public function canMakeDefault() { + return $this->getPanel()->canMakeDefault($this); + } + public function getSortKey() { $order = $this->getPanelOrder(); if ($order === null) { @@ -125,6 +123,10 @@ final class PhabricatorProfilePanelConfiguration return ($this->getVisibility() === self::VISIBILITY_DISABLED); } + public function isDefault() { + return ($this->getVisibility() === self::VISIBILITY_DEFAULT); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */