Load global and custom profile menu items in a single query

Summary: Ref T5867. Use a single query to load both personal and global items, then reorder them and add a divider if both groups have some stuff.

Test Plan: Viewed menu, edited personal and global items, viewed/edited existing project menus.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17213

src/applications/favorites/application/PhabricatorFavoritesApplication.php

@@ -55,11 +55,20 @@ final class PhabricatorFavoritesApplication extends PhabricatorApplication {
       ->withInstalled(true)
       ->executeOne();
 
-    $filter_view = id(new PhabricatorFavoritesProfileMenuEngine())
+    $menu_engine = id(new PhabricatorFavoritesProfileMenuEngine())
       ->setViewer($viewer)
-      ->setProfileObject($favorites)
+      ->setProfileObject($favorites);
-      ->setMenuType(PhabricatorProfileMenuEngine::MENU_COMBINED)
+
-      ->buildNavigation();
+    if ($viewer->getPHID()) {
+      $menu_engine
+        ->setCustomPHID($viewer->getPHID())
+        ->setMenuType(PhabricatorProfileMenuEngine::MENU_COMBINED);
+    } else {
+      $menu_engine
+        ->setMenuType(PhabricatorProfileMenuEngine::MENU_GLOBAL);
+    }
+
+    $filter_view = $menu_engine->buildNavigation();
 
     $menu_view = $filter_view->getMenu();
     $item_views = $menu_view->getItems();

src/applications/favorites/engine/PhabricatorFavoritesProfileMenuEngine.php

@@ -20,44 +20,40 @@ final class PhabricatorFavoritesProfileMenuEngine
 
   protected function getBuiltinProfileItems($object) {
     $items = array();
-    $custom_phid = $this->getCustomPHID();
     $viewer = $this->getViewer();
 
-    // Built-in Global Defaults
+    $create_task = array(
-    if (!$custom_phid) {
+      'name' => null,
-      $create_task = array(
+      'formKey' =>
-        'name' => null,
+        id(new ManiphestEditEngine())->getProfileMenuItemDefault(),
-        'formKey' =>
+    );
-          id(new ManiphestEditEngine())->getProfileMenuItemDefault(),
-      );
 
-      $create_project = array(
+    $create_project = array(
-        'name' => null,
+      'name' => null,
-        'formKey' =>
+      'formKey' =>
-          id(new PhabricatorProjectEditEngine())->getProfileMenuItemDefault(),
+        id(new PhabricatorProjectEditEngine())->getProfileMenuItemDefault(),
-      );
+    );
 
-      $create_repository = array(
+    $create_repository = array(
-        'name' => null,
+      'name' => null,
-        'formKey' =>
+      'formKey' =>
-          id(new DiffusionRepositoryEditEngine())->getProfileMenuItemDefault(),
+        id(new DiffusionRepositoryEditEngine())->getProfileMenuItemDefault(),
-      );
+    );
 
-      $items[] = $this->newItem()
+    $items[] = $this->newItem()
-        ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_TASK)
+      ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_TASK)
-        ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
+      ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
-        ->setMenuItemProperties($create_task);
+      ->setMenuItemProperties($create_task);
 
-      $items[] = $this->newItem()
+    $items[] = $this->newItem()
-        ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_PROJECT)
+      ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_PROJECT)
-        ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
+      ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
-        ->setMenuItemProperties($create_project);
+      ->setMenuItemProperties($create_project);
 
-      $items[] = $this->newItem()
+    $items[] = $this->newItem()
-        ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_REPOSITORY)
+      ->setBuiltinKey(PhabricatorFavoritesConstants::ITEM_REPOSITORY)
-        ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
+      ->setMenuItemKey(PhabricatorEditEngineProfileMenuItem::MENUITEMKEY)
-        ->setMenuItemProperties($create_repository);
+      ->setMenuItemProperties($create_repository);
-    }
 
     return $items;
   }

src/applications/search/engine/PhabricatorProfileMenuEngine.php

@@ -6,7 +6,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
   private $profileObject;
   private $customPHID;
   private $items;
-  private $menuType;
+  private $menuType = self::MENU_GLOBAL;
   private $defaultItem;
   private $controller;
   private $navigation;
@@ -15,6 +15,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
   const MENU_GLOBAL = 'global';
   const MENU_PERSONAL = 'personal';
   const MENU_COMBINED = 'menu';
+  const ITEM_CUSTOM_DIVIDER = 'engine.divider';
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -82,9 +83,16 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
   }
 
   abstract protected function getItemURI($path);
-
   abstract protected function isMenuEngineConfigurable();
 
+  abstract protected function getBuiltinProfileItems($object);
+
+  protected function getBuiltinCustomProfileItems(
+    $object,
+    $custom_phid) {
+    return array();
+  }
+
   public function buildResponse() {
     $controller = $this->getController();
 
@@ -230,6 +238,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
       ->setBaseURI(new PhutilURI($this->getItemURI('')));
 
     $menu_items = $this->getItems();
+
     $filtered_items = array();
     foreach ($menu_items as $menu_item) {
       if ($menu_item->isDisabled()) {
@@ -294,23 +303,26 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
     $object = $this->getProfileObject();
 
     $items = $this->loadBuiltinProfileItems();
-    $menu = $this->getMenuType();
 
-    if ($this->getCustomPHID()) {
+    $query = id(new PhabricatorProfileMenuItemConfigurationQuery())
-      $stored_items = id(new PhabricatorProfileMenuItemConfigurationQuery())
+      ->setViewer($viewer)
-        ->setViewer($viewer)
+      ->withProfilePHIDs(array($object->getPHID()));
-        ->withProfilePHIDs(array($object->getPHID()))
+
-        ->withCustomPHIDs(array($this->getCustomPHID()))
+    $menu_type = $this->getMenuType();
-        ->setMenuType($menu)
+    switch ($menu_type) {
-        ->execute();
+      case self::MENU_GLOBAL:
-    } else {
+        $query->withCustomPHIDs(array(), true);
-      $stored_items = id(new PhabricatorProfileMenuItemConfigurationQuery())
+        break;
-        ->setViewer($viewer)
+      case self::MENU_PERSONAL:
-        ->withProfilePHIDs(array($object->getPHID()))
+        $query->withCustomPHIDs(array($this->getCustomPHID()), false);
-        ->setMenuType($menu)
+        break;
-        ->execute();
+      case self::MENU_COMBINED:
+        $query->withCustomPHIDs(array($this->getCustomPHID()), true);
+        break;
     }
 
+    $stored_items = $query->execute();
+
     foreach ($stored_items as $stored_item) {
       $impl = $stored_item->getMenuItem();
       $impl->setViewer($viewer);
@@ -339,12 +351,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
       }
     }
 
-    $items = msort($items, 'getSortKey');
+    $items = $this->arrangeItems($items);
-
-    // Normalize keys since callers shouldn't rely on this array being
-    // partially keyed.
-    $items = array_values($items);
-
 
     // Make sure exactly one valid item is marked as default.
     $default = null;
@@ -377,7 +384,26 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
 
   private function loadBuiltinProfileItems() {
     $object = $this->getProfileObject();
-    $builtins = $this->getBuiltinProfileItems($object);
+
+    $menu_type = $this->getMenuType();
+    switch ($menu_type) {
+      case self::MENU_GLOBAL:
+        $builtins = $this->getBuiltinProfileItems($object);
+        break;
+      case self::MENU_PERSONAL:
+        $builtins = $this->getBuiltinCustomProfileItems(
+          $object,
+          $this->getCustomPHID());
+        break;
+      case self::MENU_COMBINED:
+        $builtins = array();
+        $builtins[] = $this->getBuiltinCustomProfileItems(
+          $object,
+          $this->getCustomPHID());
+        $builtins[] = $this->getBuiltinProfileItems($object);
+        $builtins = array_mergev($builtins);
+        break;
+    }
 
     $items = PhabricatorProfileMenuItem::getAllMenuItems();
     $viewer = $this->getViewer();
@@ -990,4 +1016,40 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
     return $this;
   }
 
+  private function arrangeItems(array $items) {
+    // Sort the items.
+    $items = msortv($items, 'getSortVector');
+
+    // If we have some global items and some custom items and are in "combined"
+    // mode, put a hard-coded divider item between them.
+    if ($this->getMenuType() == self::MENU_COMBINED) {
+      $list = array();
+      $seen_custom = false;
+      $seen_global = false;
+      foreach ($items as $item) {
+        if ($item->getCustomPHID()) {
+          $seen_custom = true;
+        } else {
+          if ($seen_custom && !$seen_global) {
+            $list[] = $this->newItem()
+              ->setBuiltinKey(self::ITEM_CUSTOM_DIVIDER)
+              ->setMenuItemKey(PhabricatorDividerProfileMenuItem::MENUITEMKEY)
+              ->attachMenuItem(
+                new PhabricatorDividerProfileMenuItem());
+          }
+          $seen_global = true;
+        }
+        $list[] = $item;
+      }
+      $items = $list;
+    }
+
+    // Normalize keys since callers shouldn't rely on this array being
+    // partially keyed.
+    $items = array_values($items);
+
+    return $items;
+  }
+
+
 }

src/applications/search/query/PhabricatorProfileMenuItemConfigurationQuery.php

@@ -7,7 +7,7 @@ final class PhabricatorProfileMenuItemConfigurationQuery
   private $phids;
   private $profilePHIDs;
   private $customPHIDs;
-  private $menuType;
+  private $includeGlobal;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -24,13 +24,9 @@ final class PhabricatorProfileMenuItemConfigurationQuery
     return $this;
   }
 
-  public function withCustomPHIDs(array $phids) {
+  public function withCustomPHIDs(array $phids, $include_global = false) {
     $this->customPHIDs = $phids;
-    return $this;
+    $this->includeGlobal = $include_global;
-  }
-
-  public function setMenuType($type) {
-    $this->menuType = $type;
     return $this;
   }
 
@@ -67,10 +63,21 @@ final class PhabricatorProfileMenuItemConfigurationQuery
     }
 
     if ($this->customPHIDs !== null) {
-      $where[] = qsprintf(
+      if ($this->customPHIDs && $this->includeGlobal) {
-        $conn,
+        $where[] = qsprintf(
-        'customPHID IN (%Ls)',
+          $conn,
-        $this->customPHIDs);
+          'customPHID IN (%Ls) OR customPHID IS NULL',
+          $this->customPHIDs);
+      } else if ($this->customPHIDs) {
+        $where[] = qsprintf(
+          $conn,
+          'customPHID IN (%Ls)',
+          $this->customPHIDs);
+      } else {
+        $where[] = qsprintf(
+          $conn,
+          'customPHID IS NULL');
+      }
     }
 
     return $where;

src/applications/search/storage/PhabricatorProfileMenuItemConfiguration.php

@@ -126,18 +126,28 @@ final class PhabricatorProfileMenuItemConfiguration
     return $this->getMenuItem()->willBuildNavigationItems($items);
   }
 
-  public function getSortKey() {
+  public function getSortVector() {
-    $order = $this->getMenuItemOrder();
+    // Sort custom items above global items.
-    if ($order === null) {
+    if ($this->getCustomPHID()) {
-      $order = 'Z';
+      $is_global = 0;
     } else {
-      $order = sprintf('%020d', $order);
+      $is_global = 1;
     }
 
-    return sprintf(
+    // Sort items with an explicit order above items without an explicit order,
-      '~%s%020d',
+    // so any newly created builtins go to the bottom.
-      $order,
+    $order = $this->getMenuItemOrder();
-      $this->getID());
+    if ($order !== null) {
+      $has_order = 0;
+    } else {
+      $has_order = 1;
+    }
+
+    return id(new PhutilSortVector())
+      ->addInt($is_global)
+      ->addInt($has_order)
+      ->addInt((int)$order)
+      ->addInt((int)$this->getID());
   }
 
   public function isDisabled() {