Make typeahead results more structured

Summary: Get rid of this positional array garbage.

Test Plan: Used typeaheads in menu, herald, maniphest, differential, repository/projects.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1569

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

src/__phutil_library_map__.php

@@ -1013,6 +1013,7 @@ phutil_register_library_map(array(
     'PhabricatorTrivialTestCase' => 'infrastructure/testing/__tests__/PhabricatorTrivialTestCase.php',
     'PhabricatorTypeaheadCommonDatasourceController' => 'applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php',
     'PhabricatorTypeaheadDatasourceController' => 'applications/typeahead/controller/PhabricatorTypeaheadDatasourceController.php',
+    'PhabricatorTypeaheadResult' => 'applications/typeahead/storage/PhabricatorTypeaheadResult.php',
     'PhabricatorUIExample' => 'applications/uiexample/examples/PhabricatorUIExample.php',
     'PhabricatorUIExampleController' => 'applications/uiexample/controller/PhabricatorUIExampleController.php',
     'PhabricatorUIExampleRenderController' => 'applications/uiexample/controller/PhabricatorUIExampleRenderController.php',

src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php

@@ -88,48 +88,29 @@ final class PhabricatorTypeaheadCommonDatasourceController
         break;
     }
 
-    // TODO: We transfer these fields without keys as an opitimization, but this
+    $results = array();
-    // function is hard to read as a result. Until we can sort it out, here's
-    // what the position arguments mean:
-    //
-    //   0: (required) name to match against what the user types
-    //   1: (optional) URI
-    //   2: (required) PHID
-    //   3: (optional) priority matching string
-    //   4: (optional) display name [overrides position 0]
-    //   5: (optional) display type
-    //   6: (optional) image URI
-
-    $data = array();
 
     if ($need_upforgrabs) {
-      $data[] = array(
+      $results[] = id(new PhabricatorTypeaheadResult())
-        'upforgrabs (Up For Grabs)',
+        ->setName('upforgrabs (Up For Grabs)')
-        null,
+        ->setPHID(ManiphestTaskOwner::OWNER_UP_FOR_GRABS);
-        ManiphestTaskOwner::OWNER_UP_FOR_GRABS,
-      );
     }
 
     if ($need_noproject) {
-      $data[] = array(
+      $results[] = id(new PhabricatorTypeaheadResult())
-        'noproject (No Project)',
+        ->setName('noproject (No Project)')
-        null,
+        ->setPHID(ManiphestTaskOwner::PROJECT_NO_PROJECT);
-        ManiphestTaskOwner::PROJECT_NO_PROJECT,
-      );
     }
 
     if ($need_users) {
       $columns = array(
         'isSystemAgent',
+        'isAdmin',
         'isDisabled',
         'userName',
         'realName',
         'phid');
 
-      if ($need_rich_data) {
-        $columns[] = 'profileImagePHID';
-      }
-
       if ($query) {
         $conn_r = id(new PhabricatorUser())->establishConnection('r');
         $ids = queryfx_all(
@@ -164,29 +145,33 @@ final class PhabricatorTypeaheadCommonDatasourceController
             continue;
           }
         }
-        $spec = array(
+
-          $user->getUsername().' ('.$user->getRealName().')',
+        $result = id(new PhabricatorTypeaheadResult())
-          '/p/'.$user->getUsername(),
+          ->setName($user->getFullName())
-          $user->getPHID(),
+          ->setURI('/p/'.$user->getUsername())
-          $user->getUsername(),
+          ->setPHID($user->getPHID())
-          null,
+          ->setPriorityString($user->getUsername());
-          'User',
+
-        );
         if ($need_rich_data) {
-          $spec[] = $handles[$user->getPHID()]->getImageURI();
+          $display_type = 'User';
+          if ($user->getIsAdmin()) {
+            $display_type = 'Administrator';
           }
-        $data[] = $spec;
+          $result->setDisplayType($display_type);
+          $result->setImageURI($handles[$user->getPHID()]->getImageURI());
+          $result->setPriorityType('user');
+        }
+        $results[] = $result;
       }
     }
 
     if ($need_lists) {
       $lists = id(new PhabricatorMetaMTAMailingList())->loadAll();
       foreach ($lists as $list) {
-        $data[] = array(
+        $results[] = id(new PhabricatorTypeaheadResult())
-          $list->getName(),
+          ->setName($list->getName())
-          $list->getURI(),
+          ->setURI($list->getURI())
-          $list->getPHID(),
+          ->setPHID($list->getPHID());
-        );
       }
     }
 
@@ -195,45 +180,40 @@ final class PhabricatorTypeaheadCommonDatasourceController
         'status != %d',
         PhabricatorProjectStatus::STATUS_ARCHIVED);
       foreach ($projs as $proj) {
-        $data[] = array(
+        $results[] = id(new PhabricatorTypeaheadResult())
-          $proj->getName(),
+          ->setName($proj->getName())
-          '/project/view/'.$proj->getID().'/',
+          ->setURI('/project/view/'.$proj->getID().'/')
-          $proj->getPHID(),
+          ->setPHID($proj->getPHID());
-        );
       }
     }
 
     if ($need_repos) {
       $repos = id(new PhabricatorRepository())->loadAll();
       foreach ($repos as $repo) {
-        $data[] = array(
+        $results[] = id(new PhabricatorTypeaheadResult())
-          'r'.$repo->getCallsign().' ('.$repo->getName().')',
+          ->setName('r'.$repo->getCallsign().' ('.$repo->getName().')')
-          '/diffusion/'.$repo->getCallsign().'/',
+          ->setURI('/diffusion/'.$repo->getCallsign().'/')
-          $repo->getPHID(),
+          ->setPHID($repo->getPHID())
-          'r'.$repo->getCallsign(),
+          ->setPriorityString('r'.$repo->getCallsign());
-        );
       }
     }
 
     if ($need_packages) {
       $packages = id(new PhabricatorOwnersPackage())->loadAll();
       foreach ($packages as $package) {
-        $data[] = array(
+        $results[] = id(new PhabricatorTypeaheadResult())
-          $package->getName(),
+          ->setName($package->getName())
-          '/owners/package/'.$package->getID().'/',
+          ->setURI('/owners/package/'.$package->getID().'/')
-          $package->getPHID(),
+          ->setPHID($package->getPHID());
-        );
       }
     }
 
     if ($need_arcanist_projects) {
       $arcprojs = id(new PhabricatorRepositoryArcanistProject())->loadAll();
       foreach ($arcprojs as $proj) {
-        $data[] = array(
+        $results[] = id(new PhabricatorTypeaheadResult())
-          $proj->getName(),
+          ->setName($proj->getName())
-          null,
+          ->setPHID($proj->getPHID());
-          $proj->getPHID(),
-        );
       }
     }
 
@@ -244,28 +224,23 @@ final class PhabricatorTypeaheadCommonDatasourceController
         if (!$uri) {
           continue;
         }
-        $data[] = array(
+        $name = $application->getName().' '.$application->getShortDescription();
-          $application->getName().' '.$application->getShortDescription(),
+
-          $uri,
+        $results[] = id(new PhabricatorTypeaheadResult())
-          $application->getPHID(),
+          ->setName($name)
-          $application->getName(),
+          ->setURI($uri)
-          $application->getName(),
+          ->setPHID($application->getPHID())
-          $application->getShortDescription(),
+          ->setPriorityString($application->getName())
-          $application->getIconURI(),
+          ->setDisplayName($application->getName())
-        );
+          ->setDisplayType($application->getShortDescription())
+          ->setImageuRI($application->getIconURI())
+          ->setPriorityType('apps');
       }
     }
 
-    if (!$need_rich_data) {
+    $content = mpull($results, 'getWireFormat');
-      foreach ($data as $key => $info) {
-        unset($data[$key][4]);
-        unset($data[$key][5]);
-        unset($data[$key][6]);
-      }
-    }
 
-    return id(new AphrontAjaxResponse())
+    return id(new AphrontAjaxResponse())->setContent($content);
-      ->setContent($data);
   }
 
 }

src/applications/typeahead/storage/PhabricatorTypeaheadResult.php

@@ -0,0 +1,88 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorTypeaheadResult {
+
+  private $name;
+  private $uri;
+  private $phid;
+  private $priorityString;
+  private $displayName;
+  private $displayType;
+  private $imageURI;
+  private $priorityType;
+
+  public function setName($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function setURI($uri) {
+    $this->uri = $uri;
+    return $this;
+  }
+
+  public function setPHID($phid) {
+    $this->phid = $phid;
+    return $this;
+  }
+
+  public function setPriorityString($priority_string) {
+    $this->priorityString = $priority_string;
+    return $this;
+  }
+
+  public function setDisplayName($display_name) {
+    $this->displayName = $display_name;
+    return $this;
+  }
+
+  public function setDisplayType($display_type) {
+    $this->displayType = $display_type;
+    return $this;
+  }
+
+  public function setImageURI($image_uri) {
+    $this->imageURI = $image_uri;
+    return $this;
+  }
+
+  public function setPriorityType($priority_type) {
+    $this->priorityType = $priority_type;
+    return $this;
+  }
+
+  public function getWireFormat() {
+    $data = array(
+      $this->name,
+      $this->uri,
+      $this->phid,
+      $this->priorityString,
+      $this->displayName,
+      $this->displayType,
+      $this->imageURI,
+      $this->priorityType,
+    );
+    while (end($data) === null) {
+      array_pop($data);
+    }
+    return $data;
+  }
+
+}
+