Use SearchFields in Maniphest

Summary:
Ref T8441. Ref T7715. Move Maniphest to SearchFields.

The only new tech here is hiding fields, which we use to hide some fields on the dashboard query UI.

Test Plan:
  - Queried by each field, including custom fields.
  - Used some standrad queries.
  - Used dashboards, used standard + custom queries.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

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

src/applications/maniphest/query/ManiphestTaskSearchEngine.php

@@ -43,129 +43,152 @@ final class ManiphestTaskSearchEngine
       ->needProjectPHIDs(true);
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
+  public function buildCustomSearchFields() {
-    $saved = new PhabricatorSavedQuery();
+    return array(
-
+      id(new PhabricatorSearchOwnersField())
-    $saved->setParameter(
+        ->setLabel(pht('Assigned To'))
-      'assignedPHIDs',
+        ->setKey('assignedPHIDs')
-      $this->readUsersFromRequest($request, 'assigned'));
+        ->setAliases(array('assigned')),
-
+      id(new PhabricatorSearchUsersField())
-    $saved->setParameter(
+        ->setLabel(pht('Authors'))
-      'authorPHIDs',
+        ->setKey('authorPHIDs')
-      $this->readUsersFromRequest($request, 'authors'));
+        ->setAliases(array('author', 'authors')),
-
+      id(new PhabricatorSearchDatasourceField())
-    $saved->setParameter(
+        ->setLabel(pht('Statuses'))
-      'subscriberPHIDs',
+        ->setKey('statuses')
-      $this->readSubscribersFromRequest($request, 'subscribers'));
+        ->setAliases(array('status'))
-
+        ->setDatasource(new ManiphestTaskStatusFunctionDatasource()),
-    $saved->setParameter(
+      id(new PhabricatorSearchDatasourceField())
-      'statuses',
+        ->setLabel(pht('Priorities'))
-      $this->readListFromRequest($request, 'statuses'));
+        ->setKey('priorities')
-
+        ->setAliases(array('priority'))
-    $saved->setParameter(
+        ->setDatasource(new ManiphestTaskPriorityDatasource()),
-      'priorities',
+      id(new PhabricatorSearchTextField())
-      $this->readListFromRequest($request, 'priorities'));
+        ->setLabel(pht('Contains Words'))
-
+        ->setKey('fulltext'),
-    $saved->setParameter(
+      id(new PhabricatorSearchThreeStateField())
-      'blocking',
+        ->setLabel(pht('Blocking'))
-      $this->readBoolFromRequest($request, 'blocking'));
+        ->setKey('blocking')
-    $saved->setParameter(
+        ->setOptions(
-      'blocked',
+          pht('(Show All)'),
-      $this->readBoolFromRequest($request, 'blocked'));
+          pht('Show Only Tasks Blocking Other Tasks'),
-
+          pht('Hide Tasks Blocking Other Tasks')),
-    $saved->setParameter('group', $request->getStr('group'));
+      id(new PhabricatorSearchThreeStateField())
-    $saved->setParameter('order', $request->getStr('order'));
+        ->setLabel(pht('Blocked'))
-
+        ->setKey('blocked')
-    $ids = $request->getStrList('ids');
+        ->setOptions(
-    foreach ($ids as $key => $id) {
+          pht('(Show All)'),
-      $id = trim($id, ' Tt');
+          pht('Show Only Task Blocked By Other Tasks'),
-      if (!$id || !is_numeric($id)) {
+          pht('Hide Tasks Blocked By Other Tasks')),
-        unset($ids[$key]);
+      id(new PhabricatorSearchSelectField())
-      } else {
+        ->setLabel(pht('Group By'))
-        $ids[$key] = $id;
+        ->setKey('group')
-      }
+        ->setOptions($this->getGroupOptions()),
-    }
+      id(new PhabricatorSearchStringListField())
-    $saved->setParameter('ids', $ids);
+        ->setLabel(pht('Task IDs'))
-
+        ->setKey('ids'),
-    $saved->setParameter('fulltext', $request->getStr('fulltext'));
+      id(new PhabricatorSearchDateField())
-
+        ->setLabel(pht('Created After'))
-    $saved->setParameter(
+        ->setKey('createdStart'),
-      'projects',
+      id(new PhabricatorSearchDateField())
-      $this->readProjectsFromRequest($request, 'projects'));
+        ->setLabel(pht('Created Before'))
-
+        ->setKey('createdEnd'),
-    $saved->setParameter('createdStart', $request->getStr('createdStart'));
+      id(new PhabricatorSearchDateField())
-    $saved->setParameter('createdEnd', $request->getStr('createdEnd'));
+        ->setLabel(pht('Updated After'))
-    $saved->setParameter('modifiedStart', $request->getStr('modifiedStart'));
+        ->setKey('modifiedStart'),
-    $saved->setParameter('modifiedEnd', $request->getStr('modifiedEnd'));
+      id(new PhabricatorSearchDateField())
-
+        ->setLabel(pht('Updated Before'))
-    $limit = $request->getInt('limit');
+        ->setKey('modifiedEnd'),
-    if ($limit > 0) {
+      id(new PhabricatorSearchTextField())
-      $saved->setParameter('limit', $limit);
+        ->setLabel(pht('Page Size'))
-    }
+        ->setKey('limit'),
-
+    );
-    $this->readCustomFieldsFromRequest($request, $saved);
-
-    return $saved;
   }
 
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
+  public function getDefaultFieldOrder() {
-    $query = $this->newQuery();
+    return array(
+      'assignedPHIDs',
+      'projectPHIDs',
+      'authorPHIDs',
+      'subscriberPHIDs',
+      'statuses',
+      'priorities',
+      'fulltext',
+      'blocking',
+      'blocked',
+      'group',
+      'order',
+      'ids',
+      '...',
+      'createdStart',
+      'createdEnd',
+      'modifiedStart',
+      'modifiedEnd',
+      'limit',
+    );
+  }
 
-    $viewer = $this->requireViewer();
+  public function getHiddenFields() {
+    $keys = array();
 
-    $datasource = id(new PhabricatorPeopleUserFunctionDatasource())
+    if ($this->getIsBoardView()) {
-      ->setViewer($viewer);
+      $keys[] = 'group';
-
+      $keys[] = 'order';
-    $author_phids = $saved->getParameter('authorPHIDs', array());
+      $keys[] = 'limit';
-    $author_phids = $datasource->evaluateTokens($author_phids);
-    if ($author_phids) {
-      $query->withAuthors($author_phids);
     }
 
-    $datasource = id(new PhabricatorMetaMTAMailableFunctionDatasource())
+    return $keys;
-      ->setViewer($viewer);
+  }
-    $subscriber_phids = $saved->getParameter('subscriberPHIDs', array());
+
-    $subscriber_phids = $datasource->evaluateTokens($subscriber_phids);
+  public function buildQueryFromParameters(array $map) {
-    if ($subscriber_phids) {
+    $query = id(new ManiphestTaskQuery())
-      $query->withSubscribers($subscriber_phids);
+      ->needProjectPHIDs(true);
+
+    if ($map['assignedPHIDs']) {
+      $query->withOwners($map['assignedPHIDs']);
     }
 
-    $datasource = id(new PhabricatorPeopleOwnerDatasource())
+    if ($map['authorPHIDs']) {
-      ->setViewer($this->requireViewer());
+      $query->withAuthors($map['authorPHIDs']);
-
-    $assigned_phids = $this->readAssignedPHIDs($saved);
-    $assigned_phids = $datasource->evaluateTokens($assigned_phids);
-    if ($assigned_phids) {
-      $query->withOwners($assigned_phids);
     }
 
-    $datasource = id(new ManiphestTaskStatusFunctionDatasource())
+    if ($map['statuses']) {
-      ->setViewer($this->requireViewer());
+      $query->withStatuses($map['statuses']);
-    $statuses = $saved->getParameter('statuses', array());
-    $statuses = $datasource->evaluateTokens($statuses);
-    if ($statuses) {
-      $query->withStatuses($statuses);
     }
 
-    $priorities = $saved->getParameter('priorities', array());
+    if ($map['priorities']) {
-    if ($priorities) {
+      $query->withPriorities($map['priorities']);
-      $query->withPriorities($priorities);
     }
 
-
+    if ($map['createdStart']) {
-    $query->withBlockingTasks($saved->getParameter('blocking'));
+      $query->withDateCreatedAfter($map['createdStart']);
-    $query->withBlockedTasks($saved->getParameter('blocked'));
-
-    // TODO: This is glue that will be obsolete soon.
-    $order = $saved->getParameter('order');
-    $builtin = $query->getBuiltinOrderAliasMap();
-    if (strlen($order) && isset($builtin[$order])) {
-      $query->setOrder($order);
-    } else {
-      $query->setOrder(head_key($builtin));
     }
 
-    $group = $saved->getParameter('group');
+    if ($map['createdEnd']) {
+      $query->withDateCreatedBefore($map['createdEnd']);
+    }
+
+    if ($map['modifiedStart']) {
+      $query->withDateModifiedAfter($map['modifiedStart']);
+    }
+
+    if ($map['modifiedEnd']) {
+      $query->withDateModifiedBefore($map['modifiedEnd']);
+    }
+
+    if ($map['blocking'] !== null) {
+      $query->withBlockingTasks($map['blocking']);
+    }
+
+    if ($map['blocked'] !== null) {
+      $query->withBlockedTasks($map['blocked']);
+    }
+
+    if (strlen($map['fulltext'])) {
+      $query->withFullTextSearch($map['fulltext']);
+    }
+
+    $group = idx($map, 'group');
     $group = idx($this->getGroupValues(), $group);
     if ($group) {
       $query->setGroupBy($group);
@@ -173,181 +196,25 @@ final class ManiphestTaskSearchEngine
       $query->setGroupBy(head($this->getGroupValues()));
     }
 
-    $ids = $saved->getParameter('ids');
+    if ($map['ids']) {
-    if ($ids) {
+      $ids = $map['ids'];
-      $query->withIDs($ids);
+      foreach ($ids as $key => $id) {
+        $id = trim($id, ' Tt');
+        if (!$id || !is_numeric($id)) {
+          unset($ids[$key]);
+        } else {
+          $ids[$key] = $id;
+        }
+      }
+
+      if ($ids) {
+        $query->withIDs($ids);
+      }
     }
 
-    $fulltext = $saved->getParameter('fulltext');
-    if (strlen($fulltext)) {
-      $query->withFullTextSearch($fulltext);
-    }
-
-    $projects = $this->readProjectTokens($saved);
-    $adjusted = id(clone $saved)->setParameter('projects', $projects);
-    $this->setQueryProjects($query, $adjusted);
-
-    $start = $this->parseDateTime($saved->getParameter('createdStart'));
-    $end = $this->parseDateTime($saved->getParameter('createdEnd'));
-
-    if ($start) {
-      $query->withDateCreatedAfter($start);
-    }
-
-    if ($end) {
-      $query->withDateCreatedBefore($end);
-    }
-
-    $mod_start = $this->parseDateTime($saved->getParameter('modifiedStart'));
-    $mod_end = $this->parseDateTime($saved->getParameter('modifiedEnd'));
-
-    if ($mod_start) {
-      $query->withDateModifiedAfter($mod_start);
-    }
-
-    if ($mod_end) {
-      $query->withDateModifiedBefore($mod_end);
-    }
-
-    $this->applyCustomFieldsToQuery($query, $saved);
-
     return $query;
   }
 
-  public function buildSearchForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved) {
-
-    $assigned_phids = $this->readAssignedPHIDs($saved);
-
-    $author_phids = $saved->getParameter('authorPHIDs', array());
-    $projects = $this->readProjectTokens($saved);
-
-    $subscriber_phids = $saved->getParameter('subscriberPHIDs', array());
-
-    $statuses = $saved->getParameter('statuses', array());
-    $priorities = $saved->getParameter('priorities', array());
-
-    $blocking_control = id(new AphrontFormSelectControl())
-      ->setLabel(pht('Blocking'))
-      ->setName('blocking')
-      ->setValue($this->getBoolFromQuery($saved, 'blocking'))
-      ->setOptions(array(
-        '' => pht('Show All Tasks'),
-        'true' => pht('Show Tasks Blocking Other Tasks'),
-        'false' => pht('Show Tasks Not Blocking Other Tasks'),
-      ));
-
-    $blocked_control = id(new AphrontFormSelectControl())
-      ->setLabel(pht('Blocked'))
-      ->setName('blocked')
-      ->setValue($this->getBoolFromQuery($saved, 'blocked'))
-      ->setOptions(array(
-        '' => pht('Show All Tasks'),
-        'true' => pht('Show Tasks Blocked By Other Tasks'),
-        'false' => pht('Show Tasks Not Blocked By Other Tasks'),
-      ));
-
-    $ids = $saved->getParameter('ids', array());
-
-    $all_orders = ipull($this->newQuery()->getBuiltinOrders(), 'name');
-
-    $form
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleOwnerDatasource())
-          ->setName('assigned')
-          ->setLabel(pht('Assigned To'))
-          ->setValue($assigned_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorProjectLogicalDatasource())
-          ->setName('projects')
-          ->setLabel(pht('Projects'))
-          ->setValue($projects))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleUserFunctionDatasource())
-          ->setName('authors')
-          ->setLabel(pht('Authors'))
-          ->setValue($author_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorMetaMTAMailableFunctionDatasource())
-          ->setName('subscribers')
-          ->setLabel(pht('Subscribers'))
-          ->setValue($subscriber_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new ManiphestTaskStatusFunctionDatasource())
-          ->setLabel(pht('Statuses'))
-          ->setName('statuses')
-          ->setValue($statuses))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new ManiphestTaskPriorityDatasource())
-          ->setLabel(pht('Priorities'))
-          ->setName('priorities')
-          ->setValue($priorities))
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setName('fulltext')
-          ->setLabel(pht('Contains Words'))
-          ->setValue($saved->getParameter('fulltext')))
-      ->appendChild($blocking_control)
-      ->appendChild($blocked_control);
-
-    if (!$this->getIsBoardView()) {
-      $form
-        ->appendChild(
-          id(new AphrontFormSelectControl())
-            ->setName('group')
-            ->setLabel(pht('Group By'))
-            ->setValue($saved->getParameter('group'))
-            ->setOptions($this->getGroupOptions()))
-        ->appendChild(
-          id(new AphrontFormSelectControl())
-            ->setName('order')
-            ->setLabel(pht('Order By'))
-            ->setValue($saved->getParameter('order'))
-            ->setOptions($all_orders));
-    }
-
-    $form
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setName('ids')
-          ->setLabel(pht('Task IDs'))
-          ->setValue(implode(', ', $ids)));
-
-    $this->appendCustomFieldsToForm($form, $saved);
-
-    $this->buildDateRange(
-      $form,
-      $saved,
-      'createdStart',
-      pht('Created After'),
-      'createdEnd',
-      pht('Created Before'));
-
-    $this->buildDateRange(
-      $form,
-      $saved,
-      'modifiedStart',
-      pht('Updated After'),
-      'modifiedEnd',
-      pht('Updated Before'));
-
-    if (!$this->getIsBoardView()) {
-      $form
-        ->appendChild(
-          id(new AphrontFormTextControl())
-            ->setName('limit')
-            ->setLabel(pht('Page Size'))
-            ->setValue($saved->getParameter('limit', 100)));
-    }
-  }
-
   protected function getURI($path) {
     if ($this->baseURI) {
       return $this->baseURI.$path;
@@ -458,48 +325,52 @@ final class ManiphestTaskSearchEngine
       ->setShowBatchControls($this->showBatchControls);
   }
 
-  private function readAssignedPHIDs(PhabricatorSavedQuery $saved) {
+  protected function willUseSavedQuery(PhabricatorSavedQuery $saved) {
-    $assigned_phids = $saved->getParameter('assignedPHIDs', array());
 
-    // This may be present in old saved queries from before parameterized
+    // The 'withUnassigned' parameter may be present in old saved queries from
-    // typeaheads, and is retained for compatibility. We could remove it by
+    // before parameterized typeaheads, and is retained for compatibility. We
-    // migrating old saved queries.
+    // could remove it by migrating old saved queries.
+    $assigned_phids = $saved->getParameter('assignedPHIDs', array());
     if ($saved->getParameter('withUnassigned')) {
       $assigned_phids[] = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN;
     }
+    $saved->setParameter('assignedPHIDs', $assigned_phids);
 
-    return $assigned_phids;
+    // The 'projects' and other parameters may be present in old saved queries
-  }
+    // from before parameterized typeaheads.
+    $project_phids = $saved->getParameter('projectPHIDs', array());
 
-  private function readProjectTokens(PhabricatorSavedQuery $saved) {
+    $old = $saved->getParameter('projects', array());
-    $projects = $saved->getParameter('projects', array());
+    foreach ($old as $phid) {
+      $project_phids[] = $phid;
+    }
 
     $all = $saved->getParameter('allProjectPHIDs', array());
     foreach ($all as $phid) {
-      $projects[] = $phid;
+      $project_phids[] = $phid;
     }
 
     $any = $saved->getParameter('anyProjectPHIDs', array());
     foreach ($any as $phid) {
-      $projects[] = 'any('.$phid.')';
+      $project_phids[] = 'any('.$phid.')';
     }
 
     $not = $saved->getParameter('excludeProjectPHIDs', array());
     foreach ($not as $phid) {
-      $projects[] = 'not('.$phid.')';
+      $project_phids[] = 'not('.$phid.')';
     }
 
     $users = $saved->getParameter('userProjectPHIDs', array());
     foreach ($users as $phid) {
-      $projects[] = 'projects('.$phid.')';
+      $project_phids[] = 'projects('.$phid.')';
     }
 
     $no = $saved->getParameter('withNoProject');
     if ($no) {
-      $projects[] = 'null()';
+      $project_phids[] = 'null()';
     }
 
-    return $projects;
+    $saved->setParameter('projectPHIDs', $project_phids);
   }
 
 }

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -205,6 +205,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
     $this->willUseSavedQuery($saved);
 
     $fields = $this->buildSearchFields();
+    $fields = $this->adjustFieldsForDisplay($fields);
     $viewer = $this->requireViewer();
 
     foreach ($fields as $field) {
@@ -266,7 +267,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
       $orders = ipull($orders, 'name');
 
       $fields[] = id(new PhabricatorSearchOrderField())
-        ->setLabel(pht('Order'))
+        ->setLabel(pht('Order By'))
         ->setKey('order')
         ->setOptions($orders);
     }
@@ -284,7 +285,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
       $field_map[$key] = $field;
     }
 
-    return $this->adjustFieldsForDisplay($field_map);
+    return $field_map;
   }
 
   private function adjustFieldsForDisplay(array $field_map) {
@@ -310,7 +311,13 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
     $body = array_diff_key($field_map, array_fuse($tail_keys));
     $tail = array_select_keys($field_map, $tail_keys);
 
-    return $head + $body + $tail;
+    $result = $head + $body + $tail;
+
+    foreach ($this->getHiddenFields() as $hidden_key) {
+      unset($result[$hidden_key]);
+    }
+
+    return $result;
   }
 
   protected function buildCustomSearchFields() {
@@ -346,6 +353,15 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
     return array();
   }
 
+  /**
+   * Return a list of field keys which should be hidden from the viewer.
+   *
+    * @return list<string> Fields to hide.
+   */
+  protected function getHiddenFields() {
+    return array();
+  }
+
   public function getErrors() {
     return $this->errors;
   }
@@ -924,7 +940,13 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
 
 
   public function getPageSize(PhabricatorSavedQuery $saved) {
-    return $saved->getParameter('limit', 100);
+    $limit = (int)$saved->getParameter('limit');
+
+    if ($limit > 0) {
+      return $limit;
+    }
+
+    return 100;
   }
 
 
@@ -1061,33 +1083,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
   }
 
 
-  /**
-   * Moves data from the request into a saved query.
-   *
-   * @param AphrontRequest Request to read.
-   * @param PhabricatorSavedQuery Query to write to.
-   * @return void
-   * @task appsearch
-   */
-  protected function readCustomFieldsFromRequest(
-    AphrontRequest $request,
-    PhabricatorSavedQuery $saved) {
-
-    $list = $this->getCustomFieldList();
-    if (!$list) {
-      return;
-    }
-
-    foreach ($list->getFields() as $field) {
-      $key = $this->getKeyForCustomField($field);
-      $value = $field->readApplicationSearchValueFromRequest(
-        $this,
-        $request);
-      $saved->setParameter($key, $value);
-    }
-  }
-
-
   /**
    * Applies data from a saved query to an executable query.
    *
@@ -1105,24 +1100,13 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
     }
 
     foreach ($list->getFields() as $field) {
-      $key = $this->getKeyForCustomField($field);
       $value = $field->applyApplicationSearchConstraintToQuery(
         $this,
         $query,
-        $saved->getParameter($key));
+        $saved->getParameter('custom:'.$field->getFieldIndex()));
     }
   }
 
-  /**
-   * Get a unique key identifying a field.
-   *
-   * @param PhabricatorCustomField Field to identify.
-   * @return string Unique identifier, suitable for use as an input name.
-   */
-  public function getKeyForCustomField(PhabricatorCustomField $field) {
-    return 'custom:'.$field->getFieldIndex();
-  }
-
   private function buildCustomFieldSearchFields() {
     $list = $this->getCustomFieldList();
     if (!$list) {
@@ -1135,24 +1119,8 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
         ->setSearchEngine($this)
         ->setCustomField($field);
     }
+
     return $fields;
   }
 
-  // TODO: Remove.
-  protected function appendCustomFieldsToForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved) {
-
-    $list = $this->getCustomFieldList();
-    if (!$list) {
-      return;
-    }
-
-    foreach ($list->getFields() as $field) {
-      $key = $this->getKeyForCustomField($field);
-      $value = $saved->getParameter($key);
-      $field->appendToApplicationSearchForm($this, $form, $value);
-    }
-  }
-
 }