When a ProfileMenu has a link item that adds URI parameters, highlight it when clicked

Summary:
Depends on D20352. Fixes T12949. If a user adds a link (for example, to a workboard) that takes you to the same page but with some URI parameters, we'd prefer to highlight the "link" item instead of the default "workboard" item when you click it.

For example, you add a `/query/assigned/` "link" item to a workboard, called "Click This To Show Tasks Assigned To Me On This Workboard", i.e. filter the current view.

This is a pretty reasonable thing to want to do. When you click it, we'd like to highlight that item to show that you've activated the "Assigned to Me" filter you added.

However, we currently highlight the thing actually serving the content, i.e. the "Workboard" item.

Instead:

  - When picking what to highlight, look through all the items for one with a link to the current request URI.
  - If we find one or more, pick the one that would be the default.
  - Otherwise, pick the first one.

This means that you can have several items like "?a=1", "?a=2", etc., and we will highlight them correctly when you click them.

This actual patch has some questionable bits (see some discussion in T13275), but I'd like to wait for stronger motivation to refactor it more extensively.

Test Plan:
  - On a portal, added a `?a=1` link. Saw it highlight properly when clikced.
  - On a workboard, added a link to the board itself with a different filter. Saw it highlight appropriately when clicked.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12949

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

src/applications/favorites/engineextension/PhabricatorFavoritesMainMenuBarExtension.php

@@ -54,6 +54,11 @@ final class PhabricatorFavoritesMainMenuBarExtension
       ->setProfileObject($favorites)
       ->setCustomPHID($viewer->getPHID());
 
+    $controller = $this->getController();
+    if ($controller) {
+      $menu_engine->setController($controller);
+    }
+
     $filter_view = $menu_engine->buildNavigation();
 
     $menu_view = $filter_view->getMenu();

src/applications/home/controller/PhabricatorHomeController.php

@@ -31,6 +31,7 @@ abstract class PhabricatorHomeController extends PhabricatorController {
 
       $engine = id(new PhabricatorHomeProfileMenuEngine())
         ->setViewer($viewer)
+        ->setController($this)
         ->setProfileObject($home)
         ->setCustomPHID($viewer->getPHID());
 

src/applications/project/controller/PhabricatorProjectBoardController.php

@@ -1,14 +1,4 @@
 <?php
 
 abstract class PhabricatorProjectBoardController
-  extends PhabricatorProjectController {
-
-  protected function getProfileMenu() {
-    $menu = parent::getProfileMenu();
-
-    $menu->selectFilter(PhabricatorProject::ITEM_WORKBOARD);
-    $menu->addClass('project-board-nav');
-
-    return $menu;
-  }
-}
+  extends PhabricatorProjectController {}

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -172,8 +172,7 @@ final class PhabricatorProjectBoardViewController
         return $content;
       }
 
-      $nav = $this->getProfileMenu();
-      $nav->selectFilter(PhabricatorProject::ITEM_WORKBOARD);
+      $nav = $this->newWorkboardProfileMenu();
 
       $crumbs = $this->buildApplicationCrumbs();
       $crumbs->addTextCrumb(pht('Workboard'));
@@ -720,7 +719,7 @@ final class PhabricatorProjectBoardViewController
       ->appendChild($board)
       ->addClass('project-board-wrapper');
 
-    $nav = $this->getProfileMenu();
+    $nav = $this->newWorkboardProfileMenu();
 
     $divider = id(new PHUIListItemView())
       ->setType(PHUIListItemView::TYPE_DIVIDER);
@@ -1504,4 +1503,15 @@ final class PhabricatorProjectBoardViewController
       ->addCancelButton($profile_uri);
   }
 
+  private function newWorkboardProfileMenu() {
+    $default_item = id(new PhabricatorProfileMenuItemConfiguration())
+      ->setBuiltinKey(PhabricatorProject::ITEM_WORKBOARD);
+
+    $menu = parent::getProfileMenu($default_item);
+
+    $menu->addClass('project-board-nav');
+
+    return $menu;
+  }
+
 }

src/applications/project/controller/PhabricatorProjectController.php

@@ -97,11 +97,11 @@ abstract class PhabricatorProjectController extends PhabricatorController {
     return $menu;
   }
 
-  protected function getProfileMenu() {
+  protected function getProfileMenu($default_item = null) {
     if (!$this->profileMenu) {
       $engine = $this->getProfileMenuEngine();
       if ($engine) {
-        $this->profileMenu = $engine->buildNavigation();
+        $this->profileMenu = $engine->buildNavigation($default_item);
       }
     }
 

src/applications/search/engine/PhabricatorProfileMenuEngine.php

@@ -183,7 +183,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
         break;
     }
 
-    $navigation = $this->buildNavigation();
+    $navigation = $this->buildNavigation($selected_item);
 
     $crumbs = $controller->buildApplicationCrumbsForEditEngine();
 
@@ -223,8 +223,6 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
     switch ($item_action) {
       case 'view':
         if ($selected_item) {
-          $navigation->selectFilter($selected_item->getDefaultMenuItemKey());
-
           try {
             $content = $this->buildItemViewContent($selected_item);
           } catch (Exception $ex) {
@@ -335,7 +333,9 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
     return $page;
   }
 
-  public function buildNavigation() {
+  public function buildNavigation(
+    PhabricatorProfileMenuItemConfiguration $selected_item = null) {
+
     if ($this->navigation) {
       return $this->navigation;
     }
@@ -398,6 +398,12 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
 
     $nav->selectFilter(null);
 
+    $navigation_items = $nav->getMenu()->getItems();
+    $select_key = $this->pickHighlightedMenuItem(
+      $navigation_items,
+      $selected_item);
+    $nav->selectFilter($select_key);
+
     $this->navigation = $nav;
     return $this->navigation;
   }
@@ -1367,4 +1373,97 @@ abstract class PhabricatorProfileMenuEngine extends Phobject {
     return null;
   }
 
+  private function pickHighlightedMenuItem(
+    array $items,
+    PhabricatorProfileMenuItemConfiguration $selected_item = null) {
+
+    assert_instances_of($items, 'PHUIListItemView');
+
+    $default_key = null;
+    if ($selected_item) {
+      $default_key = $selected_item->getDefaultMenuItemKey();
+    }
+
+    $controller = $this->getController();
+
+    // In some rare cases, when like building the "Favorites" menu on a
+    // 404 page, we may not have a controller. Just accept whatever default
+    // behavior we'd otherwise end up with.
+    if (!$controller) {
+      return $default_key;
+    }
+
+    $request = $controller->getRequest();
+
+    // See T12949. If one of the menu items is a link to the same URI that
+    // the page was accessed with, we want to highlight that item. For example,
+    // this allows you to add links to a menu that apply filters to a
+    // workboard.
+
+    $matches = array();
+    foreach ($items as $item) {
+      $href = $item->getHref();
+      if ($this->isMatchForRequestURI($request, $href)) {
+        $matches[] = $item;
+      }
+    }
+
+    foreach ($matches as $match) {
+      if ($match->getKey() === $default_key) {
+        return $default_key;
+      }
+    }
+
+    if ($matches) {
+      return head($matches)->getKey();
+    }
+
+    return $default_key;
+  }
+
+  private function isMatchForRequestURI(AphrontRequest $request, $item_uri) {
+    $request_uri = $request->getAbsoluteRequestURI();
+    $item_uri = new PhutilURI($item_uri);
+
+    // If the request URI and item URI don't have matching paths, they
+    // do not match.
+    if ($request_uri->getPath() !== $item_uri->getPath()) {
+      return false;
+    }
+
+    // If the request URI and item URI don't have matching parameters, they
+    // also do not match. We're specifically trying to let "?filter=X" work
+    // on Workboards, among other use cases, so this is important.
+    $request_params = $request_uri->getQueryParamsAsPairList();
+    $item_params = $item_uri->getQueryParamsAsPairList();
+    if ($request_params !== $item_params) {
+      return false;
+    }
+
+    // If the paths and parameters match, the item domain must be: empty; or
+    // match the request domain; or match the production domain.
+
+    $request_domain = $request_uri->getDomain();
+
+    $production_uri = PhabricatorEnv::getProductionURI('/');
+    $production_domain = id(new PhutilURI($production_uri))
+      ->getDomain();
+
+    $allowed_domains = array(
+      '',
+      $request_domain,
+      $production_domain,
+    );
+    $allowed_domains = array_fuse($allowed_domains);
+
+    $item_domain = $item_uri->getDomain();
+    $item_domain = (string)$item_domain;
+
+    if (isset($allowed_domains[$item_domain])) {
+      return true;
+    }
+
+    return false;
+  }
+
 }