Move Repositories to SearchField

Summary: Ref T8441. Ref T7715. Internalizes `setQueryOrder()`. Removes `appendOrderFieldsToForm()`.

Test Plan: Used all Repository search fields.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

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

src/applications/repository/query/PhabricatorRepositoryQuery.php

@@ -150,22 +150,13 @@ final class PhabricatorRepositoryQuery
     $this->identifierMap = array();
   }
 
+  public function newResultObject() {
+    return new PhabricatorRepository();
+  }
+
   protected function loadPage() {
-    $table = new PhabricatorRepository();
+    $table = $this->newResultObject();
-    $conn_r = $table->establishConnection('r');
+    $data = $this->loadStandardPageRows($table);
-
-    $data = queryfx_all(
-      $conn_r,
-      '%Q FROM %T r %Q %Q %Q %Q %Q %Q',
-      $this->buildSelectClause($conn_r),
-      $table->getTableName(),
-      $this->buildJoinClause($conn_r),
-      $this->buildWhereClause($conn_r),
-      $this->buildGroupClause($conn_r),
-      $this->buildHavingClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
     $repositories = $table->loadAllFromArray($data);
 
     if ($this->needCommitCounts) {
@@ -386,25 +377,27 @@ final class PhabricatorRepositoryQuery
     return $map;
   }
 
-  protected function buildSelectClause(AphrontDatabaseConnection $conn) {
+  protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
-    $parts = $this->buildSelectClauseParts($conn);
+    $parts = parent::buildSelectClauseParts($conn);
+
     if ($this->shouldJoinSummaryTable()) {
       $parts[] = 's.*';
     }
-    return $this->formatSelectClause($parts);
+
+    return $parts;
   }
 
-  protected function buildJoinClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
-    $joins = $this->buildJoinClauseParts($conn_r);
+    $joins = parent::buildJoinClauseParts($conn);
 
     if ($this->shouldJoinSummaryTable()) {
       $joins[] = qsprintf(
-        $conn_r,
+        $conn,
         'LEFT JOIN %T s ON r.id = s.repositoryID',
         PhabricatorRepository::TABLE_SUMMARY);
     }
 
-    return $this->formatJoinClause($joins);
+    return $joins;
   }
 
   private function shouldJoinSummaryTable() {
@@ -428,26 +421,26 @@ final class PhabricatorRepositoryQuery
     return false;
   }
 
-  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    $where = parent::buildWhereClauseParts($conn_r);
+    $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->callsigns) {
+    if ($this->callsigns !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.callsign IN (%Ls)',
         $this->callsigns);
     }
@@ -459,21 +452,21 @@ final class PhabricatorRepositoryQuery
 
       if ($this->numericIdentifiers) {
         $identifier_clause[] = qsprintf(
-          $conn_r,
+          $conn,
           'r.id IN (%Ld)',
           $this->numericIdentifiers);
       }
 
       if ($this->callsignIdentifiers) {
         $identifier_clause[] = qsprintf(
-          $conn_r,
+          $conn,
           'r.callsign IN (%Ls)',
           $this->callsignIdentifiers);
       }
 
       if ($this->phidIdentifiers) {
         $identifier_clause[] = qsprintf(
-          $conn_r,
+          $conn,
           'r.phid IN (%Ls)',
           $this->phidIdentifiers);
       }
@@ -483,21 +476,21 @@ final class PhabricatorRepositoryQuery
 
     if ($this->types) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.versionControlSystem IN (%Ls)',
         $this->types);
     }
 
     if ($this->uuids) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.uuid IN (%Ls)',
         $this->uuids);
     }
 
     if (strlen($this->nameContains)) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'name LIKE %~',
         $this->nameContains);
     }
@@ -511,7 +504,7 @@ final class PhabricatorRepositoryQuery
         $callsign = $query;
       }
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'r.name LIKE %> OR r.callsign LIKE %>',
         $query,
         $callsign);

src/applications/repository/query/PhabricatorRepositorySearchEngine.php

@@ -11,122 +11,66 @@ final class PhabricatorRepositorySearchEngine
     return 'PhabricatorDiffusionApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
+  public function newQuery() {
-    $saved = new PhabricatorSavedQuery();
+    return id(new PhabricatorRepositoryQuery())
-
-    $saved->setParameter('callsigns', $request->getStrList('callsigns'));
-    $saved->setParameter('status', $request->getStr('status'));
-    $saved->setParameter('order', $request->getStr('order'));
-    $saved->setParameter('hosted', $request->getStr('hosted'));
-    $saved->setParameter('types', $request->getArr('types'));
-    $saved->setParameter('name', $request->getStr('name'));
-
-    $saved->setParameter(
-      'projects',
-      $this->readProjectsFromRequest($request, 'projects'));
-
-    return $saved;
-  }
-
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new PhabricatorRepositoryQuery())
       ->needProjectPHIDs(true)
       ->needCommitCounts(true)
       ->needMostRecentCommits(true);
-
-    $callsigns = $saved->getParameter('callsigns');
-    if ($callsigns) {
-      $query->withCallsigns($callsigns);
-    }
-
-    $status = $saved->getParameter('status');
-    $status = idx($this->getStatusValues(), $status);
-    if ($status) {
-      $query->withStatus($status);
-    }
-
-    $this->setQueryOrder($query, $saved);
-
-    $hosted = $saved->getParameter('hosted');
-    $hosted = idx($this->getHostedValues(), $hosted);
-    if ($hosted) {
-      $query->withHosted($hosted);
-    }
-
-    $types = $saved->getParameter('types');
-    if ($types) {
-      $query->withTypes($types);
-    }
-
-    $name = $saved->getParameter('name');
-    if (strlen($name)) {
-      $query->withNameContains($name);
-    }
-
-    $adjusted = clone $saved;
-    $adjusted->setParameter('projects', $this->readProjectTokens($saved));
-    $this->setQueryProjects($query, $adjusted);
-
-    return $query;
   }
 
-  public function buildSearchForm(
+  protected function buildCustomSearchFields() {
-    AphrontFormView $form,
+    return array(
-    PhabricatorSavedQuery $saved_query) {
+      id(new PhabricatorSearchStringListField())
+        ->setLabel(pht('Callsigns'))
+        ->setKey('callsigns'),
+      id(new PhabricatorSearchTextField())
+        ->setLabel(pht('Name Contains'))
+        ->setKey('name'),
+      id(new PhabricatorSearchSelectField())
+        ->setLabel(pht('Status'))
+        ->setKey('status')
+        ->setOptions($this->getStatusOptions()),
+      id(new PhabricatorSearchSelectField())
+        ->setLabel(pht('Hosted'))
+        ->setKey('hosted')
+        ->setOptions($this->getHostedOptions()),
+      id(new PhabricatorSearchCheckboxesField())
+        ->setLabel(pht('Types'))
+        ->setKey('types')
+        ->setOptions(PhabricatorRepositoryType::getAllRepositoryTypes()),
+    );
+  }
 
-    $callsigns = $saved_query->getParameter('callsigns', array());
+  public function buildQueryFromParameters(array $map) {
-    $types = $saved_query->getParameter('types', array());
+    $query = $this->newQuery();
-    $types = array_fuse($types);
-    $name = $saved_query->getParameter('name');
-    $projects = $this->readProjectTokens($saved_query);
 
-    $form
+    if ($map['callsigns']) {
-      ->appendChild(
+      $query->withCallsigns($map['callsigns']);
-        id(new AphrontFormTextControl())
-          ->setName('callsigns')
-          ->setLabel(pht('Callsigns'))
-          ->setValue(implode(', ', $callsigns)))
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setName('name')
-          ->setLabel(pht('Name Contains'))
-          ->setValue($name))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorProjectLogicalDatasource())
-          ->setName('projects')
-          ->setLabel(pht('Projects'))
-          ->setValue($projects))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setName('status')
-          ->setLabel(pht('Status'))
-          ->setValue($saved_query->getParameter('status'))
-          ->setOptions($this->getStatusOptions()))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setName('hosted')
-          ->setLabel(pht('Hosted'))
-          ->setValue($saved_query->getParameter('hosted'))
-          ->setOptions($this->getHostedOptions()));
-
-    $type_control = id(new AphrontFormCheckboxControl())
-      ->setLabel(pht('Types'));
-
-    $all_types = PhabricatorRepositoryType::getAllRepositoryTypes();
-    foreach ($all_types as $key => $name) {
-      $type_control->addCheckbox(
-        'types[]',
-        $key,
-        $name,
-        isset($types[$key]));
     }
-    $form->appendChild($type_control);
 
-    $this->appendOrderFieldsToForm(
+    if ($map['status']) {
-      $form,
+      $status = idx($this->getStatusValues(), $map['status']);
-      $saved_query,
+      if ($status) {
-      new PhabricatorRepositoryQuery());
+        $query->withStatus($status);
+      }
+    }
+
+    if ($map['hosted']) {
+      $hosted = idx($this->getHostedValues(), $map['hosted']);
+      if ($hosted) {
+        $query->withHosted($hosted);
+      }
+    }
+
+    if ($map['types']) {
+      $query->withTypes($map['types']);
+    }
+
+    if (strlen($map['name'])) {
+      $query->withNameContains($map['name']);
+    }
+
+    return $query;
   }
 
   protected function getURI($path) {
@@ -268,15 +212,20 @@ final class PhabricatorRepositorySearchEngine
     return $list;
   }
 
-  private function readProjectTokens(PhabricatorSavedQuery $saved) {
+  protected function willUseSavedQuery(PhabricatorSavedQuery $saved) {
-    $projects = $saved->getParameter('projects', array());
+    $project_phids = $saved->getParameter('projectPHIDs', array());
+
+    $old = $saved->getParameter('projects', array());
+    foreach ($old as $phid) {
+      $project_phids[] = $phid;
+    }
 
     $any = $saved->getParameter('anyProjectPHIDs', array());
     foreach ($any as $project) {
-      $projects[] = 'any('.$project.')';
+      $project_phids[] = 'any('.$project.')';
     }
 
-    return $projects;
+    $saved->setParameter('projectPHIDs', $project_phids);
   }
 
 }

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -108,6 +108,9 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
    * @return The result of the query.
    */
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
+    $saved = clone $saved;
+    $this->willUseSavedQuery($saved);
+
     $fields = $this->buildSearchFields();
     $viewer = $this->requireViewer();
 
@@ -153,11 +156,36 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
       $this->applyCustomFieldsToQuery($query, $saved);
     }
 
-    $this->setQueryOrder($query, $saved);
+    $order = $saved->getParameter('order');
+    $builtin = $query->getBuiltinOrders();
+    if (strlen($order) && isset($builtin[$order])) {
+      $query->setOrder($order);
+    } else {
+      // If the order is invalid or not available, we choose the first
+      // builtin order. This isn't always the default order for the query,
+      // but is the first value in the "Order" dropdown, and makes the query
+      // behavior more consistent with the UI. In queries where the two
+      // orders differ, this order is the preferred order for humans.
+      $query->setOrder(head_key($builtin));
+    }
 
     return $query;
   }
 
+  /**
+   * Hook for subclasses to adjust saved queries prior to use.
+   *
+   * If an application changes how queries are saved, it can implement this
+   * hook to keep old queries working the way users expect, by reading,
+   * adjusting, and overwriting parameters.
+   *
+   * @param PhabricatorSavedQuery Saved query which will be executed.
+   * @return void
+   */
+  protected function willUseSavedQuery(PhabricatorSavedQuery $saved) {
+    return;
+  }
+
   protected function buildQueryFromParameters(array $parameters) {
     throw new PhutilMethodNotImplementedException();
   }
@@ -173,6 +201,9 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
     AphrontFormView $form,
     PhabricatorSavedQuery $saved) {
 
+    $saved = clone $saved;
+    $this->willUseSavedQuery($saved);
+
     $fields = $this->buildSearchFields();
     $viewer = $this->requireViewer();
 
@@ -889,50 +920,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
   }
 
 
-/* -(  Result Ordering  )---------------------------------------------------- */
-
-  /**
-   * Set query ordering from a saved value.
-   */
-  protected function setQueryOrder(
-    PhabricatorCursorPagedPolicyAwareQuery $query,
-    PhabricatorSavedQuery $saved) {
-
-    $order = $saved->getParameter('order');
-    $builtin = $query->getBuiltinOrders();
-
-    if (strlen($order) && isset($builtin[$order])) {
-      $query->setOrder($order);
-    } else {
-      // If the order is invalid or not available, we choose the first
-      // builtin order. This isn't always the default order for the query,
-      // but is the first value in the "Order" dropdown, and makes the query
-      // behavior more consistent with the UI. In queries where the two
-      // orders differ, this order is the preferred order for humans.
-      $query->setOrder(head_key($builtin));
-    }
-
-    return $this;
-  }
-
-
-
-  protected function appendOrderFieldsToForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved,
-    PhabricatorCursorPagedPolicyAwareQuery $query) {
-
-    $orders = $query->getBuiltinOrders();
-    $orders = ipull($orders, 'name');
-
-    $form->appendControl(
-      id(new AphrontFormSelectControl())
-        ->setLabel(pht('Order'))
-        ->setName('order')
-        ->setOptions($orders)
-        ->setValue($saved->getParameter('order')));
-  }
-
 /* -(  Paging and Executing Queries  )--------------------------------------- */