Separate workboard view state (ordering, filtering, hidden columns) from the View controller

Summary:
Depends on D20627. Ref T4900. If a user orders a board by "Sort by Title", then toggles the visibility of hidden columns, we want to keep the board sorted by title. To accomplish this, we pass the board state around to all the workflows here.

Pull the "bag of state properties" code out of the View controller. This class basically:

  - reads state from a request (order, hidden, filter);
  - manages defaults;
  - provides the application with the current settings; and
  - generates URIs with "?order=X&hidden=Y&filter=Z" to preserve state.

This is still a little questionable/transitional since some of the controllers need more cleanup.

Test Plan:
Toggled state, order, filters, clicked around various workflows and saw the filters preserved.

A lot of these workflows are pretty serious edge cases. For example, here's a feature this implements:

  - Changed workboard order to "Title".
  - Selected "Bulk Edit Tasks..." in an empty column and command-clicked it to open the link in a new window.
  - Hovered over "Cancel".
  - Saw the link properly generate with "?order=title", preserving the order.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T4900

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

src/__phutil_library_map__.php

@@ -4949,6 +4949,7 @@ phutil_register_library_map(array(
     'PhabricatorWeekStartDaySetting' => 'applications/settings/setting/PhabricatorWeekStartDaySetting.php',
     'PhabricatorWildConfigType' => 'applications/config/type/PhabricatorWildConfigType.php',
     'PhabricatorWordPressAuthProvider' => 'applications/auth/provider/PhabricatorWordPressAuthProvider.php',
+    'PhabricatorWorkboardViewState' => 'applications/project/state/PhabricatorWorkboardViewState.php',
     'PhabricatorWorker' => 'infrastructure/daemon/workers/PhabricatorWorker.php',
     'PhabricatorWorkerActiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php',
     'PhabricatorWorkerActiveTaskQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerActiveTaskQuery.php',
@@ -11356,6 +11357,7 @@ phutil_register_library_map(array(
     'PhabricatorWeekStartDaySetting' => 'PhabricatorSelectSetting',
     'PhabricatorWildConfigType' => 'PhabricatorJSONConfigType',
     'PhabricatorWordPressAuthProvider' => 'PhabricatorOAuth2AuthProvider',
+    'PhabricatorWorkboardViewState' => 'Phobject',
     'PhabricatorWorker' => 'Phobject',
     'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask',
     'PhabricatorWorkerActiveTaskQuery' => 'PhabricatorWorkerTaskQuery',

src/applications/project/controller/PhabricatorProjectBoardController.php

@@ -1,4 +1,25 @@
 <?php
 
 abstract class PhabricatorProjectBoardController
-  extends PhabricatorProjectController {}
+  extends PhabricatorProjectController {
+
+  private $viewState;
+
+  final protected function getViewState() {
+    if ($this->viewState === null) {
+      $this->viewState = $this->newViewState();
+    }
+
+    return $this->viewState;
+  }
+
+  final private function newViewState() {
+    $project = $this->getProject();
+    $request = $this->getRequest();
+
+    return id(new PhabricatorWorkboardViewState())
+      ->setProject($project)
+      ->readFromRequest($request);
+  }
+
+}

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -5,10 +5,6 @@ final class PhabricatorProjectBoardViewController
 
   const BATCH_EDIT_ALL = 'all';
 
-  private $queryKey;
-  private $sortKey;
-  private $showHidden;
-
   public function shouldAllowPublic() {
     return true;
   }
@@ -22,10 +18,8 @@ final class PhabricatorProjectBoardViewController
     }
 
     $project = $this->getProject();
-
-    $this->readRequestState();
-
-    $board_uri = $this->getApplicationURI('board/'.$project->getID().'/');
+    $state = $this->getViewState();
+    $board_uri = $project->getWorkboardURI();
 
     $search_engine = id(new ManiphestTaskSearchEngine())
       ->setViewer($viewer)
@@ -51,24 +45,15 @@ final class PhabricatorProjectBoardViewController
           ->addSubmitButton(pht('Apply Filter'))
           ->addCancelButton($board_uri);
       }
-      return id(new AphrontRedirectResponse())->setURI(
-        $this->getURIWithState(
-          $search_engine->getQueryResultsPageURI($saved->getQueryKey())));
+
+      $query_key = $saved->getQueryKey();
+      $results_uri = $search_engine->getQueryResultsPageURI($query_key);
+      $results_uri = $state->newURI($results_uri);
+
+      return id(new AphrontRedirectResponse())->setURI($results_uri);
     }
 
-    $query_key = $this->getDefaultFilter($project);
-
-    $request_query = $request->getStr('filter');
-    if (strlen($request_query)) {
-      $query_key = $request_query;
-    }
-
-    $uri_query = $request->getURIData('queryKey');
-    if (strlen($uri_query)) {
-      $query_key = $uri_query;
-    }
-
-    $this->queryKey = $query_key;
+    $query_key = $state->getQueryKey();
 
     $custom_query = null;
     if ($search_engine->isBuiltinQuery($query_key)) {
@@ -260,7 +245,7 @@ final class PhabricatorProjectBoardViewController
       }
 
       if (!$batch_tasks) {
-        $cancel_uri = $this->getURIWithState($board_uri);
+        $cancel_uri = $state->newWorkboardURI();
         return $this->newDialog()
           ->setTitle(pht('No Editable Tasks'))
           ->appendParagraph(
@@ -308,7 +293,7 @@ final class PhabricatorProjectBoardViewController
       }
 
       $move_tasks = array_select_keys($tasks, $move_task_phids);
-      $cancel_uri = $this->getURIWithState($board_uri);
+      $cancel_uri = $state->newWorkboardURI();
 
       if (!$move_tasks) {
         return $this->newDialog()
@@ -504,7 +489,7 @@ final class PhabricatorProjectBoardViewController
     $column_phids = array();
     $visible_phids = array();
     foreach ($columns as $column) {
-      if (!$this->showHidden) {
+      if (!$state->getShowHidden()) {
         if ($column->isHidden()) {
           continue;
         }
@@ -649,7 +634,7 @@ final class PhabricatorProjectBoardViewController
       );
     }
 
-    $order_key = $this->sortKey;
+    $order_key = $state->getOrder();
 
     $ordering_map = PhabricatorProjectColumnOrder::getEnabledOrders();
     $ordering = id(clone $ordering_map[$order_key])
@@ -683,7 +668,7 @@ final class PhabricatorProjectBoardViewController
       'pointsEnabled' => ManiphestTaskPoints::getIsEnabled(),
 
       'boardPHID' => $project->getPHID(),
-      'order' => $this->sortKey,
+      'order' => $state->getOrder(),
       'orders' => $order_maps,
       'headers' => $headers,
       'headerKeys' => $header_keys,
@@ -701,7 +686,7 @@ final class PhabricatorProjectBoardViewController
     $sort_menu = $this->buildSortMenu(
       $viewer,
       $project,
-      $this->sortKey,
+      $state->getOrder(),
       $ordering_map);
 
     $filter_menu = $this->buildFilterMenu(
@@ -711,7 +696,7 @@ final class PhabricatorProjectBoardViewController
       $search_engine,
       $query_key);
 
-    $manage_menu = $this->buildManageMenu($project, $this->showHidden);
+    $manage_menu = $this->buildManageMenu($project, $state->getShowHidden());
 
     $header_link = phutil_tag(
       'a',
@@ -775,54 +760,14 @@ final class PhabricatorProjectBoardViewController
     return $page;
   }
 
-  private function readRequestState() {
-    $request = $this->getRequest();
-    $project = $this->getProject();
-
-    $this->showHidden = $request->getBool('hidden');
-
-    $sort_key = $this->getDefaultSort($project);
-
-    $request_sort = $request->getStr('order');
-    if ($this->isValidSort($request_sort)) {
-      $sort_key = $request_sort;
-    }
-
-    $this->sortKey = $sort_key;
-  }
-
-  private function getDefaultSort(PhabricatorProject $project) {
-    $default_sort = $project->getDefaultWorkboardSort();
-
-    if ($this->isValidSort($default_sort)) {
-      return $default_sort;
-    }
-
-    return PhabricatorProjectColumnNaturalOrder::ORDERKEY;
-  }
-
-  private function getDefaultFilter(PhabricatorProject $project) {
-    $default_filter = $project->getDefaultWorkboardFilter();
-
-    if (strlen($default_filter)) {
-      return $default_filter;
-    }
-
-    return 'open';
-  }
-
-  private function isValidSort($sort) {
-    $map = PhabricatorProjectColumnOrder::getEnabledOrders();
-    return isset($map[$sort]);
-  }
-
   private function buildSortMenu(
     PhabricatorUser $viewer,
     PhabricatorProject $project,
     $sort_key,
     array $ordering_map) {
 
-    $base_uri = $this->getURIWithState();
+    $state = $this->getViewState();
+    $base_uri = $state->newWorkboardURI();
 
     $items = array();
     foreach ($ordering_map as $key => $ordering) {
@@ -997,6 +942,7 @@ final class PhabricatorProjectBoardViewController
 
     $request = $this->getRequest();
     $viewer = $request->getUser();
+    $state = $this->getViewState();
 
     $id = $project->getID();
 
@@ -1026,12 +972,12 @@ final class PhabricatorProjectBoardViewController
       ->setWorkflow(true);
 
     if ($show_hidden) {
-      $hidden_uri = $this->getURIWithState()
+      $hidden_uri = $state->newWorkboardURI()
         ->removeQueryParam('hidden');
       $hidden_icon = 'fa-eye-slash';
       $hidden_text = pht('Hide Hidden Columns');
     } else {
-      $hidden_uri = $this->getURIWithState()
+      $hidden_uri = $state->newWorkboardURI()
         ->replaceQueryParam('hidden', 'true');
       $hidden_icon = 'fa-eye';
       $hidden_text = pht('Show Hidden Columns');
@@ -1307,41 +1253,11 @@ final class PhabricatorProjectBoardViewController
    * @return PhutilURI URI with state parameters.
    */
   private function getURIWithState($base = null, $force = false) {
-    $project = $this->getProject();
-
     if ($base === null) {
-      $base = $this->getRequest()->getPath();
+      $base = $this->getProject()->getWorkboardURI();
     }
 
-    $base = new PhutilURI($base);
-
-    if ($force || ($this->sortKey != $this->getDefaultSort($project))) {
-      if ($this->sortKey !== null) {
-        $base->replaceQueryParam('order', $this->sortKey);
-      } else {
-        $base->removeQueryParam('order');
-      }
-    } else {
-      $base->removeQueryParam('order');
-    }
-
-    if ($force || ($this->queryKey != $this->getDefaultFilter($project))) {
-      if ($this->queryKey !== null) {
-        $base->replaceQueryParam('filter', $this->queryKey);
-      } else {
-        $base->removeQueryParam('filter');
-      }
-    } else {
-      $base->removeQueryParam('filter');
-    }
-
-    if ($this->showHidden) {
-      $base->replaceQueryParam('hidden', 'true');
-    } else {
-      $base->removeQueryParam('hidden');
-    }
-
-    return $base;
+    return $this->getViewState()->newURI($base, $force);
   }
 
   private function buildInitializeContent(PhabricatorProject $project) {

src/applications/project/state/PhabricatorWorkboardViewState.php

@@ -0,0 +1,150 @@
+<?php
+
+final class PhabricatorWorkboardViewState
+  extends Phobject {
+
+  private $project;
+  private $requestState = array();
+
+  public function setProject(PhabricatorProject $project) {
+    $this->project = $project;
+    return $this;
+  }
+
+  public function getProject() {
+    return $this->project;
+  }
+
+  public function readFromRequest(AphrontRequest $request) {
+    if ($request->getExists('hidden')) {
+      $this->requestState['hidden'] = $request->getBool('hidden');
+    }
+
+    if ($request->getExists('order')) {
+      $this->requestState['order'] = $request->getStr('order');
+    }
+
+    // On some pathways, the search engine query key may be specified with
+    // either a "?filter=X" query parameter or with a "/query/X/" URI
+    // component. If both are present, the URI component is controlling.
+
+    // In particular, the "queryKey" URI parameter is used by
+    // "buildSavedQueryFromRequest()" when we are building custom board filters
+    // by invoking SearchEngine code.
+
+    if ($request->getExists('filter')) {
+      $this->requestState['filter'] = $request->getStr('filter');
+    }
+
+    if (strlen($request->getURIData('queryKey'))) {
+      $this->requestState['filter'] = $request->getURIData('queryKey');
+    }
+
+    return $this;
+  }
+
+  public function newWorkboardURI($path = null) {
+    $project = $this->getProject();
+    $uri = urisprintf('%p%p', $project->getWorkboardURI(), $path);
+    return $this->newURI($uri);
+  }
+
+  public function newURI($path, $force = false) {
+    $project = $this->getProject();
+    $uri = new PhutilURI($path);
+
+    $request_order = $this->getOrder();
+    $default_order = $this->getDefaultOrder();
+    if ($force || ($request_order !== $default_order)) {
+      $request_value = idx($this->requestState, 'order');
+      if ($request_value !== null) {
+        $uri->replaceQueryParam('order', $request_value);
+      } else {
+        $uri->removeQueryParam('order');
+      }
+    } else {
+      $uri->removeQueryParam('order');
+    }
+
+    $request_query = $this->getQueryKey();
+    $default_query = $this->getDefaultQueryKey();
+    if ($force || ($request_query !== $default_query)) {
+      $request_value = idx($this->requestState, 'filter');
+      if ($request_value !== null) {
+        $uri->replaceQueryParam('filter', $request_value);
+      } else {
+        $uri->removeQueryParam('filter');
+      }
+    } else {
+      $uri->removeQueryParam('filter');
+    }
+
+    if ($this->getShowHidden()) {
+      $uri->replaceQueryParam('hidden', 'true');
+    } else {
+      $uri->removeQueryParam('hidden');
+    }
+
+    return $uri;
+  }
+
+  public function getShowHidden() {
+    $request_show = idx($this->requestState, 'hidden');
+
+    if ($request_show !== null) {
+      return $request_show;
+    }
+
+    return false;
+  }
+
+  public function getOrder() {
+    $request_order = idx($this->requestState, 'order');
+    if ($request_order !== null) {
+      if ($this->isValidOrder($request_order)) {
+        return $request_order;
+      }
+    }
+
+    return $this->getDefaultOrder();
+  }
+
+  public function getQueryKey() {
+    $request_query = idx($this->requestState, 'filter');
+    if (strlen($request_query)) {
+      return $request_query;
+    }
+
+    return $this->getDefaultQueryKey();
+  }
+
+  private function isValidOrder($order) {
+    $map = PhabricatorProjectColumnOrder::getEnabledOrders();
+    return isset($map[$order]);
+  }
+
+  private function getDefaultOrder() {
+    $project = $this->getProject();
+
+    $default_order = $project->getDefaultWorkboardSort();
+
+    if ($this->isValidOrder($default_order)) {
+      return $default_order;
+    }
+
+    return PhabricatorProjectColumnNaturalOrder::ORDERKEY;
+  }
+
+  private function getDefaultQueryKey() {
+    $project = $this->getProject();
+
+    $default_query = $project->getDefaultWorkboardFilter();
+
+    if (strlen($default_query)) {
+      return $default_query;
+    }
+
+    return 'open';
+  }
+
+}