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
2024-12-24 14:30:56 +01:00 · 2016-01-22 05:33:21 -08:00 · 2016-01-22 05:33:21 -08:00 · 51ed95c00b
commit 51ed95c00b
parent c25de5e02f
8 changed files with 251 additions and 48 deletions
--- a/src/applications/base/PhabricatorApplication.php
+++ b/src/applications/base/PhabricatorApplication.php
@ -641,6 +641,7 @@ abstract class PhabricatorApplication
    return array(
      '(?P<panelAction>view)/(?P<panelID>[^/]+)/' => $controller,
      '(?P<panelAction>hide)/(?P<panelID>[^/]+)/' => $controller,
+      '(?P<panelAction>default)/(?P<panelID>[^/]+)/' => $controller,
      '(?P<panelAction>configure)/' => $controller,
      '(?P<panelAction>reorder)/' => $controller,
      '(?P<panelAction>edit)/'.$edit_route => $controller,
--- 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) {
--- 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;
--- 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');
--- 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');
--- 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() {
--- 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;
--- 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  )----------------------------------------- */