Allow workboard sorting and filtering to be saved as defaults

Summary:
Fixes T6641. This allows users who have permission to edit a project to use "Save as Default" to save the current order and filter as defaults for the project.

These are per-board defaults, and apply to all users. The rationale is that I think the best default ordering/filtering depends mostly on the board, not the viewer.

This seems to align with most requests in the task, although rationale is a bit light. But, for example, it seems reasonable you might want to change the default filter to "All Tasks" on a sprint board, so you can see what's in the "Done" column.

This also fixes some minor issues I ran into:

  - Herald could hit an issue while checking permissions if the project was a subproject and a non-member had a triggering rule.
  - "Advanced filter..." did not prefill with the current filter.

Test Plan:
  - Set default order and filter on a workboard.
  - Reloaded board, saw settings stick.
  - Tried to edit a board as an unprivileged user (disabled menu items, error).
  - Reviewed transaction log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6641

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

resources/sql/autopatches/20160212.proj.1.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_project.project
+  ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20160212.proj.2.sql

@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_project.project
+  SET properties = '{}' WHERE properties = '';

src/__phutil_library_map__.php

@@ -2896,6 +2896,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectCustomFieldStringIndex' => 'applications/project/storage/PhabricatorProjectCustomFieldStringIndex.php',
     'PhabricatorProjectDAO' => 'applications/project/storage/PhabricatorProjectDAO.php',
     'PhabricatorProjectDatasource' => 'applications/project/typeahead/PhabricatorProjectDatasource.php',
+    'PhabricatorProjectDefaultController' => 'applications/project/controller/PhabricatorProjectDefaultController.php',
     'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php',
     'PhabricatorProjectDetailsProfilePanel' => 'applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php',
     'PhabricatorProjectEditController' => 'applications/project/controller/PhabricatorProjectEditController.php',
@@ -7332,6 +7333,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage',
     'PhabricatorProjectDAO' => 'PhabricatorLiskDAO',
     'PhabricatorProjectDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorProjectDefaultController' => 'PhabricatorProjectBoardController',
     'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField',
     'PhabricatorProjectDetailsProfilePanel' => 'PhabricatorProfilePanel',
     'PhabricatorProjectEditController' => 'PhabricatorProjectController',

src/applications/project/application/PhabricatorProjectApplication.php

@@ -94,6 +94,8 @@ final class PhabricatorProjectApplication extends PhabricatorApplication {
           => 'PhabricatorProjectSilenceController',
         'warning/(?P<id>[1-9]\d*)/'
           => 'PhabricatorProjectSubprojectWarningController',
+        'default/(?P<projectID>[1-9]\d*)/(?P<target>[^/]+)/'
+          => 'PhabricatorProjectDefaultController',
       ),
       '/tag/' => array(
         '(?P<slug>[^/]+)/' => 'PhabricatorProjectViewController',

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -8,7 +8,6 @@ final class PhabricatorProjectBoardViewController
   private $id;
   private $slug;
   private $queryKey;
-  private $filter;
   private $sortKey;
   private $showHidden;
 
@@ -56,10 +55,18 @@ final class PhabricatorProjectBoardViewController
           $search_engine->getQueryResultsPageURI($saved->getQueryKey())));
     }
 
-    $query_key = $request->getURIData('queryKey');
-    if (!$query_key) {
-      $query_key = 'open';
+    $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;
 
     $custom_query = null;
@@ -382,10 +389,12 @@ final class PhabricatorProjectBoardViewController
 
     $sort_menu = $this->buildSortMenu(
       $viewer,
+      $project,
       $this->sortKey);
 
     $filter_menu = $this->buildFilterMenu(
       $viewer,
+      $project,
       $custom_query,
       $search_engine,
       $query_key);
@@ -445,20 +454,49 @@ final class PhabricatorProjectBoardViewController
     $this->showHidden = $request->getBool('hidden');
     $this->id = $project->getID();
 
-    $sort_key = $request->getStr('order');
-    switch ($sort_key) {
+    $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 PhabricatorProjectColumn::DEFAULT_ORDER;
+  }
+
+  private function getDefaultFilter(PhabricatorProject $project) {
+    $default_filter = $project->getDefaultWorkboardFilter();
+
+    if (strlen($default_filter)) {
+      return $default_filter;
+    }
+
+    return 'open';
+  }
+
+  private function isValidSort($sort) {
+    switch ($sort) {
       case PhabricatorProjectColumn::ORDER_NATURAL:
       case PhabricatorProjectColumn::ORDER_PRIORITY:
-        break;
-      default:
-        $sort_key = PhabricatorProjectColumn::DEFAULT_ORDER;
-        break;
+        return true;
     }
-    $this->sortKey = $sort_key;
+
+    return false;
   }
 
   private function buildSortMenu(
     PhabricatorUser $viewer,
+    PhabricatorProject $project,
     $sort_key) {
 
     $sort_icon = id(new PHUIIconView())
@@ -489,6 +527,24 @@ final class PhabricatorProjectBoardViewController
       $items[] = $item;
     }
 
+    $id = $project->getID();
+
+    $save_uri = "default/{$id}/sort/";
+    $save_uri = $this->getApplicationURI($save_uri);
+    $save_uri = $this->getURIWithState($save_uri, $force = true);
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $project,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $items[] = id(new PhabricatorActionView())
+      ->setIcon('fa-floppy-o')
+      ->setName(pht('Save as Default'))
+      ->setHref($save_uri)
+      ->setWorkflow(true)
+      ->setDisabled(!$can_edit);
+
     $sort_menu = id(new PhabricatorActionListView())
       ->setUser($viewer);
     foreach ($items as $item) {
@@ -507,8 +563,10 @@ final class PhabricatorProjectBoardViewController
 
     return $sort_button;
   }
+
   private function buildFilterMenu(
     PhabricatorUser $viewer,
+    PhabricatorProject $project,
     $custom_query,
     PhabricatorApplicationSearchEngine $engine,
     $query_key) {
@@ -551,18 +609,40 @@ final class PhabricatorProjectBoardViewController
         $uri = $engine->getQueryResultsPageURI($key);
       }
 
-      $uri = $this->getURIWithState($uri);
+      $uri = $this->getURIWithState($uri)
+        ->setQueryParam('filter', null);
       $item->setHref($uri);
 
       $items[] = $item;
     }
 
+    $id = $project->getID();
+
+    $filter_uri = $this->getApplicationURI("board/{$id}/filter/");
+    $filter_uri = $this->getURIWithState($filter_uri, $force = true);
+
     $items[] = id(new PhabricatorActionView())
       ->setIcon('fa-cog')
-      ->setHref($this->getApplicationURI('board/'.$this->id.'/filter/'))
+      ->setHref($filter_uri)
       ->setWorkflow(true)
       ->setName(pht('Advanced Filter...'));
 
+    $save_uri = "default/{$id}/filter/";
+    $save_uri = $this->getApplicationURI($save_uri);
+    $save_uri = $this->getURIWithState($save_uri, $force = true);
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $project,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $items[] = id(new PhabricatorActionView())
+      ->setIcon('fa-floppy-o')
+      ->setName(pht('Save as Default'))
+      ->setHref($save_uri)
+      ->setWorkflow(true)
+      ->setDisabled(!$can_edit);
+
     $filter_menu = id(new PhabricatorActionListView())
         ->setUser($viewer);
     foreach ($items as $item) {
@@ -793,22 +873,31 @@ final class PhabricatorProjectBoardViewController
    * the rest of the board state persistent. If no URI is provided, this method
    * starts with the request URI.
    *
-   * @param string|null   URI to add state parameters to.
-   * @return PhutilURI    URI with state parameters.
+   * @param string|null URI to add state parameters to.
+   * @param bool True to explicitly include all state.
+   * @return PhutilURI URI with state parameters.
    */
-  private function getURIWithState($base = null) {
+  private function getURIWithState($base = null, $force = false) {
+    $project = $this->getProject();
+
     if ($base === null) {
       $base = $this->getRequest()->getRequestURI();
     }
 
     $base = new PhutilURI($base);
 
-    if ($this->sortKey != PhabricatorProjectColumn::DEFAULT_ORDER) {
+    if ($force || ($this->sortKey != $this->getDefaultSort($project))) {
       $base->setQueryParam('order', $this->sortKey);
     } else {
       $base->setQueryParam('order', null);
     }
 
+    if ($force || ($this->queryKey != $this->getDefaultFilter($project))) {
+      $base->setQueryParam('filter', $this->queryKey);
+    } else {
+      $base->setQueryParam('filter', null);
+    }
+
     $base->setQueryParam('hidden', $this->showHidden ? 'true' : null);
 
     return $base;

src/applications/project/controller/PhabricatorProjectDefaultController.php

@@ -0,0 +1,90 @@
+<?php
+
+final class PhabricatorProjectDefaultController
+  extends PhabricatorProjectBoardController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $request->getViewer();
+    $project_id = $request->getURIData('projectID');
+
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($viewer)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->withIDs(array($project_id))
+      ->executeOne();
+    if (!$project) {
+      return new Aphront404Response();
+    }
+    $this->setProject($project);
+
+    $target = $request->getURIData('target');
+    switch ($target) {
+      case 'filter':
+        $title = pht('Set Board Default Filter');
+        $body = pht(
+          'Make the current filter the new default filter for this board? '.
+          'All users will see the new filter as the default when they view '.
+          'the board.');
+        $button = pht('Save Default Filter');
+
+        $xaction_value = $request->getStr('filter');
+        $xaction_type = PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER;
+        break;
+      case 'sort':
+        $title = pht('Set Board Default Order');
+        $body = pht(
+          'Make the current sort order the new default order for this board? '.
+          'All users will see the new order as the default when they view '.
+          'the board.');
+        $button = pht('Save Default Order');
+
+        $xaction_value = $request->getStr('order');
+        $xaction_type = PhabricatorProjectTransaction::TYPE_DEFAULT_SORT;
+        break;
+      default:
+        return new Aphront404Response();
+    }
+
+    $id = $project->getID();
+
+    $view_uri = $this->getApplicationURI("board/{$id}/");
+    $view_uri = new PhutilURI($view_uri);
+    foreach ($request->getPassthroughRequestData() as $key => $value) {
+      $view_uri->setQueryParam($key, $value);
+    }
+
+    if ($request->isFormPost()) {
+      $xactions = array();
+
+      $xactions[] = id(new PhabricatorProjectTransaction())
+        ->setTransactionType($xaction_type)
+        ->setNewValue($xaction_value);
+
+      id(new PhabricatorProjectTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true)
+        ->applyTransactions($project, $xactions);
+
+      return id(new AphrontRedirectResponse())->setURI($view_uri);
+    }
+
+    $dialog = $this->newDialog()
+      ->setTitle($title)
+      ->appendChild($body)
+      ->setDisableWorkflowOnCancel(true)
+      ->addCancelButton($view_uri)
+      ->addSubmitButton($title);
+
+    foreach ($request->getPassthroughRequestData() as $key => $value) {
+      $dialog->addHiddenInput($key, $value);
+    }
+
+    return $dialog;
+  }
+}

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -40,6 +40,8 @@ final class PhabricatorProjectTransactionEditor
     $types[] = PhabricatorProjectTransaction::TYPE_PARENT;
     $types[] = PhabricatorProjectTransaction::TYPE_MILESTONE;
     $types[] = PhabricatorProjectTransaction::TYPE_HASWORKBOARD;
+    $types[] = PhabricatorProjectTransaction::TYPE_DEFAULT_SORT;
+    $types[] = PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER;
 
     return $types;
   }
@@ -71,6 +73,10 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_PARENT:
       case PhabricatorProjectTransaction::TYPE_MILESTONE:
         return null;
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT:
+        return $object->getDefaultWorkboardSort();
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER:
+        return $object->getDefaultWorkboardFilter();
     }
 
     return parent::getCustomTransactionOldValue($object, $xaction);
@@ -89,6 +95,8 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_LOCKED:
       case PhabricatorProjectTransaction::TYPE_PARENT:
       case PhabricatorProjectTransaction::TYPE_MILESTONE:
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT:
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER:
         return $xaction->getNewValue();
       case PhabricatorProjectTransaction::TYPE_HASWORKBOARD:
         return (int)$xaction->getNewValue();
@@ -139,6 +147,12 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_HASWORKBOARD:
         $object->setHasWorkboard($xaction->getNewValue());
         return;
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT:
+        $object->setDefaultWorkboardSort($xaction->getNewValue());
+        return;
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER:
+        $object->setDefaultWorkboardFilter($xaction->getNewValue());
+        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -181,6 +195,8 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_PARENT:
       case PhabricatorProjectTransaction::TYPE_MILESTONE:
       case PhabricatorProjectTransaction::TYPE_HASWORKBOARD:
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT:
+      case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER:
         return;
      }
 
@@ -866,8 +882,16 @@ final class PhabricatorProjectTransactionEditor
     PhabricatorLiskDAO $object,
     array $xactions) {
 
+    // Herald rules may run on behalf of other users and need to execute
+    // membership checks against ancestors.
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withPHIDs(array($object->getPHID()))
+      ->needAncestorMembers(true)
+      ->executeOne();
+
     return id(new PhabricatorProjectHeraldAdapter())
-      ->setProject($object);
+      ->setProject($project);
   }
 
 }

src/applications/project/storage/PhabricatorProject.php

@@ -36,6 +36,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO
   protected $projectDepth;
   protected $projectPathKey;
 
+  protected $properties = array();
+
   private $memberPHIDs = self::ATTACHABLE;
   private $watcherPHIDs = self::ATTACHABLE;
   private $sparseWatchers = self::ATTACHABLE;
@@ -198,6 +200,9 @@ final class PhabricatorProject extends PhabricatorProjectDAO
   protected function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
+      self::CONFIG_SERIALIZATION => array(
+        'properties' => self::SERIALIZATION_JSON,
+      ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'name' => 'sort128',
         'status' => 'text32',
@@ -549,6 +554,31 @@ final class PhabricatorProject extends PhabricatorProjectDAO
     return idx($map, $color, $color);
   }
 
+  public function getProperty($key, $default = null) {
+    return idx($this->properties, $key, $default);
+  }
+
+  public function setProperty($key, $value) {
+    $this->properties[$key] = $value;
+    return $this;
+  }
+
+  public function getDefaultWorkboardSort() {
+    return $this->getProperty('workboard.sort.default');
+  }
+
+  public function setDefaultWorkboardSort($sort) {
+    return $this->setProperty('workboard.sort.default', $sort);
+  }
+
+  public function getDefaultWorkboardFilter() {
+    return $this->getProperty('workboard.filter.default');
+  }
+
+  public function setDefaultWorkboardFilter($filter) {
+    return $this->setProperty('workboard.filter.default', $filter);
+  }
+
 
 /* -(  PhabricatorCustomFieldInterface  )------------------------------------ */
 

src/applications/project/storage/PhabricatorProjectTransaction.php

@@ -13,6 +13,8 @@ final class PhabricatorProjectTransaction
   const TYPE_PARENT = 'project:parent';
   const TYPE_MILESTONE = 'project:milestone';
   const TYPE_HASWORKBOARD = 'project:hasworkboard';
+  const TYPE_DEFAULT_SORT = 'project:sort';
+  const TYPE_DEFAULT_FILTER = 'project:filter';
 
   // NOTE: This is deprecated, members are just a normal edge now.
   const TYPE_MEMBERS    = 'project:members';
@@ -66,8 +68,29 @@ final class PhabricatorProjectTransaction
     return parent::getColor();
   }
 
-  public function getIcon() {
+  public function shouldHideForFeed() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_HASWORKBOARD:
+      case self::TYPE_DEFAULT_SORT:
+      case self::TYPE_DEFAULT_FILTER:
+        return true;
+    }
 
+    return parent::shouldHideForFeed();
+  }
+
+  public function shouldHideForMail(array $xactions) {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_HASWORKBOARD:
+      case self::TYPE_DEFAULT_SORT:
+      case self::TYPE_DEFAULT_FILTER:
+        return true;
+    }
+
+    return parent::shouldHideForMail($xactions);
+  }
+
+  public function getIcon() {
     $old = $this->getOldValue();
     $new = $this->getNewValue();
 
@@ -258,6 +281,16 @@ final class PhabricatorProjectTransaction
             '%s disabled the workboard for this project.',
             $author_handle);
         }
+
+      case self::TYPE_DEFAULT_SORT:
+        return pht(
+          '%s changed the default sort order for the project workboard.',
+          $author_handle);
+
+      case self::TYPE_DEFAULT_FILTER:
+        return pht(
+          '%s changed the default filter for the project workboard.',
+          $author_handle);
     }
 
     return parent::getTitle();
@@ -379,19 +412,6 @@ final class PhabricatorProjectTransaction
             $this->renderSlugList($rem));
         }
 
-      case self::TYPE_HASWORKBOARD:
-        if ($new) {
-          return pht(
-            '%s enabled the workboard for %s.',
-            $author_handle,
-            $object_handle);
-        } else {
-          return pht(
-            '%s disabled the workboard for %s.',
-            $author_handle,
-            $object_handle);
-        }
-
     }
 
     return parent::getTitleForFeed();