Modernize the Herald rule search engine

Summary: Ref T13216. Update the Herald Rule SearchEngine and Query to use a more modern style.

Test Plan: Ran various rule queries in the UI, got sensible results

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

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

src/applications/herald/query/HeraldRuleQuery.php

@@ -70,19 +70,12 @@ final class HeraldRuleQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $this;
   }
 
+  public function newResultObject() {
+    return new HeraldRule();
+  }
+
   protected function loadPage() {
-    $table = new HeraldRule();
+    return $this->loadStandardPage($this->newResultObject());
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT rule.* FROM %T rule %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
   }
 
   protected function willFilterPage(array $rules) {
@@ -175,38 +168,38 @@ final class HeraldRuleQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $rules;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    $where = array();
+    $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->authorPHIDs) {
+    if ($this->authorPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.authorPHID IN (%Ls)',
         $this->authorPHIDs);
     }
 
-    if ($this->ruleTypes) {
+    if ($this->ruleTypes !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.ruleType IN (%Ls)',
         $this->ruleTypes);
     }
 
-    if ($this->contentTypes) {
+    if ($this->contentTypes !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.contentType IN (%Ls)',
@@ -220,23 +213,21 @@ final class HeraldRuleQuery extends PhabricatorCursorPagedPolicyAwareQuery {
         (int)$this->disabled);
     }
 
-    if ($this->datasourceQuery) {
+    if ($this->datasourceQuery !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.name LIKE %>',
         $this->datasourceQuery);
     }
 
-    if ($this->triggerObjectPHIDs) {
+    if ($this->triggerObjectPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
         'rule.triggerObjectPHID IN (%Ls)',
         $this->triggerObjectPHIDs);
     }
 
-    $where[] = $this->buildPagingClause($conn);
+    return $where;
-
-    return $this->formatWhereClause($conn, $where);
   }
 
   private function validateRuleAuthors(array $rules) {
@@ -281,4 +272,8 @@ final class HeraldRuleQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return 'PhabricatorHeraldApplication';
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'rule';
+  }
+
 }

src/applications/herald/query/HeraldRuleSearchEngine.php

@@ -10,90 +10,68 @@ final class HeraldRuleSearchEngine extends PhabricatorApplicationSearchEngine {
     return 'PhabricatorHeraldApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
+  public function newQuery() {
-    $saved = new PhabricatorSavedQuery();
+    return new HeraldRuleQuery();
-
-    $saved->setParameter(
-      'authorPHIDs',
-      $this->readUsersFromRequest($request, 'authors'));
-
-    $saved->setParameter('contentType', $request->getStr('contentType'));
-    $saved->setParameter('ruleType', $request->getStr('ruleType'));
-    $saved->setParameter(
-      'disabled',
-      $this->readBoolFromRequest($request, 'disabled'));
-
-    return $saved;
   }
 
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
+  protected function buildCustomSearchFields() {
-    $query = id(new HeraldRuleQuery());
+    $viewer = $this->requireViewer();
 
-    $author_phids = $saved->getParameter('authorPHIDs');
+    $rule_types = HeraldRuleTypeConfig::getRuleTypeMap();
-    if ($author_phids) {
+    $content_types = HeraldAdapter::getEnabledAdapterMap($viewer);
-      $query->withAuthorPHIDs($author_phids);
+
+    return array(
+      id(new PhabricatorUsersSearchField())
+        ->setLabel(pht('Authors'))
+        ->setKey('authorPHIDs')
+        ->setAliases(array('author', 'authors', 'authorPHID'))
+        ->setDescription(
+          pht('Search for rules with given authors.')),
+      id(new PhabricatorSearchCheckboxesField())
+        ->setKey('ruleTypes')
+        ->setAliases(array('ruleType'))
+        ->setLabel(pht('Rule Type'))
+        ->setDescription(
+          pht('Search for rules of given types.'))
+        ->setOptions($rule_types),
+      id(new PhabricatorSearchCheckboxesField())
+        ->setKey('contentTypes')
+        ->setLabel(pht('Content Type'))
+        ->setDescription(
+          pht('Search for rules affecting given types of content.'))
+        ->setOptions($content_types),
+      id(new PhabricatorSearchThreeStateField())
+        ->setLabel(pht('Rule Status'))
+        ->setKey('disabled')
+        ->setOptions(
+          pht('(Show All)'),
+          pht('Show Only Disabled Rules'),
+          pht('Show Only Enabled Rules')),
+    );
+  }
+
+  protected function buildQueryFromParameters(array $map) {
+    $query = $this->newQuery();
+
+    if ($map['authorPHIDs']) {
+      $query->withAuthorPHIDs($map['authorPHIDs']);
     }
 
-    $content_type = $saved->getParameter('contentType');
+    if ($map['contentTypes']) {
-    $content_type = idx($this->getContentTypeValues(), $content_type);
+      $query->withContentTypes($map['contentTypes']);
-    if ($content_type) {
-      $query->withContentTypes(array($content_type));
     }
 
-    $rule_type = $saved->getParameter('ruleType');
+    if ($map['ruleTypes']) {
-    $rule_type = idx($this->getRuleTypeValues(), $rule_type);
+      $query->withRuleTypes($map['ruleTypes']);
-    if ($rule_type) {
-      $query->withRuleTypes(array($rule_type));
     }
 
-    $disabled = $saved->getParameter('disabled');
+    if ($map['disabled'] !== null) {
-    if ($disabled !== null) {
+      $query->withDisabled($map['disabled']);
-      $query->withDisabled($disabled);
     }
 
     return $query;
   }
 
-  public function buildSearchForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved_query) {
-
-    $author_phids = $saved_query->getParameter('authorPHIDs', array());
-    $content_type = $saved_query->getParameter('contentType');
-    $rule_type = $saved_query->getParameter('ruleType');
-
-    $form
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('authors')
-          ->setLabel(pht('Authors'))
-          ->setValue($author_phids))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setName('contentType')
-          ->setLabel(pht('Content Type'))
-          ->setValue($content_type)
-          ->setOptions($this->getContentTypeOptions()))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setName('ruleType')
-          ->setLabel(pht('Rule Type'))
-          ->setValue($rule_type)
-          ->setOptions($this->getRuleTypeOptions()))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setName('disabled')
-          ->setLabel(pht('Rule Status'))
-          ->setValue($this->getBoolFromQuery($saved_query, 'disabled'))
-          ->setOptions(
-            array(
-              '' => pht('Show Enabled and Disabled Rules'),
-              'false' => pht('Show Only Enabled Rules'),
-              'true' => pht('Show Only Disabled Rules'),
-            )));
-  }
-
   protected function getURI($path) {
     return '/herald/'.$path;
   }
@@ -131,35 +109,6 @@ final class HeraldRuleSearchEngine extends PhabricatorApplicationSearchEngine {
     return parent::buildSavedQueryFromBuiltin($query_key);
   }
 
-  private function getContentTypeOptions() {
-    return array(
-      '' => pht('(All Content Types)'),
-    ) + HeraldAdapter::getEnabledAdapterMap($this->requireViewer());
-  }
-
-  private function getContentTypeValues() {
-    return array_fuse(
-      array_keys(
-        HeraldAdapter::getEnabledAdapterMap($this->requireViewer())));
-  }
-
-  private function getRuleTypeOptions() {
-    return array(
-      '' => pht('(All Rule Types)'),
-    ) + HeraldRuleTypeConfig::getRuleTypeMap();
-  }
-
-  private function getRuleTypeValues() {
-    return array_fuse(array_keys(HeraldRuleTypeConfig::getRuleTypeMap()));
-  }
-
-  protected function getRequiredHandlePHIDsForResultList(
-    array $rules,
-    PhabricatorSavedQuery $query) {
-
-    return mpull($rules, 'getAuthorPHID');
-  }
-
   protected function renderResultList(
     array $rules,
     PhabricatorSavedQuery $query,
@@ -167,6 +116,7 @@ final class HeraldRuleSearchEngine extends PhabricatorApplicationSearchEngine {
     assert_instances_of($rules, 'HeraldRule');
 
     $viewer = $this->requireViewer();
+    $handles = $viewer->loadHandles(mpull($rules, 'getAuthorPHID'));
 
     $content_type_map = HeraldAdapter::getEnabledAdapterMap($viewer);