Move handle fetching into tokenizer Datasources

Summary: Ref T7689. This serves two goals: - I want to remove Controller->loadViewerHandles(). A nontrivial number of these callsites are loading handles to pass to tokenizers. Since tokenizers need to take strings eventually anyway, we can do less work by letting them take PHIDs now. - A few changes out, I want tokenizers to accept parameterized tokens (like `viewer()`, `members(differential)`, etc.), so the `setValues()` signature needs to change eventually anyway. I made this work and converted a handful of callsites as an example; upcoming changes will convert more. Test Plan: - Viewed Almanac binding editor; used "Interface". - Edited Almanac services; used "Projects". - Edited Almanac devices; used "Projects". - Searched for commits; used "Auditors"; "Commit Authors", "Repositories". - Searched for calendar events; used "Created By"; used "Invited". Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7689 Differential Revision: https://secure.phabricator.com/D12218
2024-11-19 05:12:41 +01:00 · 2015-03-31 14:10:32 -07:00 · 2015-03-31 14:10:32 -07:00 · 7711ea9855
commit 7711ea9855
parent ba10e8ed41
7 changed files with 79 additions and 73 deletions
--- a/src/applications/almanac/controller/AlmanacBindingEditController.php
+++ b/src/applications/almanac/controller/AlmanacBindingEditController.php
@ -82,20 +82,15 @@ final class AlmanacBindingEditController
      }
    }

-    $interface_handles = array();
-    if ($v_interface) {
-      $interface_handles = $this->loadViewerHandles($v_interface);
-    }
-
    $form = id(new AphrontFormView())
      ->setUser($viewer)
-      ->appendChild(
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setName('interfacePHIDs')
          ->setLabel('Interface')
          ->setLimit(1)
          ->setDatasource(new AlmanacInterfaceDatasource())
-          ->setValue($interface_handles)
+          ->setValue($v_interface)
          ->setError($e_interface))
      ->appendChild(
        id(new AphrontFormSubmitControl())
--- a/src/applications/almanac/controller/AlmanacDeviceEditController.php
+++ b/src/applications/almanac/controller/AlmanacDeviceEditController.php
@ -107,12 +107,6 @@ final class AlmanacDeviceEditController
      ->setObject($device)
      ->execute();

-    if ($v_projects) {
-      $project_handles = $this->loadViewerHandles($v_projects);
-    } else {
-      $project_handles = array();
-    }
-
    $form = id(new AphrontFormView())
      ->setUser($viewer)
      ->appendChild(
@ -133,11 +127,11 @@ final class AlmanacDeviceEditController
          ->setPolicyObject($device)
          ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
          ->setPolicies($policies))
-      ->appendChild(
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setLabel(pht('Projects'))
          ->setName('projects')
-          ->setValue($project_handles)
+          ->setValue($v_projects)
          ->setDatasource(new PhabricatorProjectDatasource()))
      ->appendChild(
        id(new AphrontFormSubmitControl())
--- a/src/applications/almanac/controller/AlmanacServiceEditController.php
+++ b/src/applications/almanac/controller/AlmanacServiceEditController.php
@ -122,12 +122,6 @@ final class AlmanacServiceEditController
      ->setObject($service)
      ->execute();

-    if ($v_projects) {
-      $project_handles = $this->loadViewerHandles($v_projects);
-    } else {
-      $project_handles = array();
-    }
-
    $form = id(new AphrontFormView())
      ->setUser($viewer)
      ->addHiddenInput('edit', true)
@ -150,11 +144,11 @@ final class AlmanacServiceEditController
          ->setPolicyObject($service)
          ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
          ->setPolicies($policies))
-      ->appendChild(
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setLabel(pht('Projects'))
          ->setName('projects')
-          ->setValue($project_handles)
+          ->setValue($v_projects)
          ->setDatasource(new PhabricatorProjectDatasource()))
      ->appendChild(
        id(new AphrontFormSubmitControl())
--- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php
+++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
@ -82,43 +82,31 @@ final class PhabricatorCommitSearchEngine
    $audit_status = $saved->getParameter('auditStatus', null);
    $repository_phids = $saved->getParameter('repositoryPHIDs', array());

-    $phids = array_mergev(
-      array(
-        $auditor_phids,
-        $commit_author_phids,
-        $repository_phids,
-      ));
-
-    $handles = id(new PhabricatorHandleQuery())
-      ->setViewer($this->requireViewer())
-      ->withPHIDs($phids)
-      ->execute();
-
    $form
-      ->appendChild(
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setDatasource(new DiffusionAuditorDatasource())
          ->setName('auditorPHIDs')
          ->setLabel(pht('Auditors'))
-          ->setValue(array_select_keys($handles, $auditor_phids)))
-      ->appendChild(
+          ->setValue($auditor_phids))
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setDatasource(new PhabricatorPeopleDatasource())
          ->setName('authors')
          ->setLabel(pht('Commit Authors'))
-          ->setValue(array_select_keys($handles, $commit_author_phids)))
+          ->setValue($commit_author_phids))
       ->appendChild(
         id(new AphrontFormSelectControl())
-         ->setName('auditStatus')
-         ->setLabel(pht('Audit Status'))
-         ->setOptions($this->getAuditStatusOptions())
-         ->setValue($audit_status))
-       ->appendChild(
+           ->setName('auditStatus')
+           ->setLabel(pht('Audit Status'))
+           ->setOptions($this->getAuditStatusOptions())
+           ->setValue($audit_status))
+       ->appendControl(
         id(new AphrontFormTokenizerControl())
-         ->setLabel(pht('Repositories'))
-         ->setName('repositoryPHIDs')
-         ->setDatasource(new DiffusionRepositoryDatasource())
-         ->setValue(array_select_keys($handles, $repository_phids)));
+           ->setLabel(pht('Repositories'))
+           ->setName('repositoryPHIDs')
+           ->setDatasource(new DiffusionRepositoryDatasource())
+           ->setValue($repository_phids));

  }

--- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
+++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
@ -87,35 +87,19 @@ final class PhabricatorCalendarEventSearchEngine
    $invited_phids = $saved->getParameter('invitedPHIDs', array());
    $creator_phids = $saved->getParameter('creatorPHIDs', array());

-    $all_phids = array_merge(
-      $invited_phids,
-      $creator_phids);
-
-    if ($all_phids) {
-      $handles = id(new PhabricatorHandleQuery())
-        ->setViewer($this->requireViewer())
-        ->withPHIDs($all_phids)
-        ->execute();
-    } else {
-      $handles = array();
-    }
-
-    $invited_handles = array_select_keys($handles, $invited_phids);
-    $creator_handles = array_select_keys($handles, $creator_phids);
-
    $form
-      ->appendChild(
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setDatasource(new PhabricatorPeopleDatasource())
          ->setName('creators')
          ->setLabel(pht('Created By'))
-          ->setValue($creator_handles))
-      ->appendChild(
+          ->setValue($creator_phids))
+      ->appendControl(
        id(new AphrontFormTokenizerControl())
          ->setDatasource(new PhabricatorPeopleDatasource())
          ->setName('invited')
          ->setLabel(pht('Invited'))
-          ->setValue($invited_handles))
+          ->setValue($invited_phids))
      ->appendChild(
        id(new AphrontFormDateControl())
          ->setLabel(pht('Occurs After'))
--- a/src/view/form/AphrontFormView.php
+++ b/src/view/form/AphrontFormView.php
@ -12,7 +12,7 @@ final class AphrontFormView extends AphrontView {
  private $shaded = false;
  private $sigils = array();
  private $metadata;
-
+  private $controls = array();

  public function setMetadata($metadata) {
    $this->metadata = $metadata;
@ -87,9 +87,30 @@ final class AphrontFormView extends AphrontView {
      ->appendChild($this->renderChildren());
  }

+
+  /**
+   * Append a control to the form.
+   *
+   * This method behaves like @{method:appendChild}, but it only takes
+   * controls. It will propagate some information from the form to the
+   * control to simplify rendering.
+   *
+   * @param AphrontFormControl Control to append.
+   * @return this
+   */
+  public function appendControl(AphrontFormControl $control) {
+    $this->controls[] = $control;
+    return $this->appendChild($control);
+  }
+
+
  public function render() {
    require_celerity_resource('phui-form-view-css');

+    foreach ($this->controls as $control) {
+      $control->setUser($this->getUser());
+    }
+
    $layout = $this->buildLayoutView();

    if (!$this->user) {
--- a/src/view/form/control/AphrontFormTokenizerControl.php
+++ b/src/view/form/control/AphrontFormTokenizerControl.php
@ -35,7 +35,37 @@ final class AphrontFormTokenizerControl extends AphrontFormControl {
    $name = $this->getName();
    $values = nonempty($this->getValue(), array());

-    assert_instances_of($values, 'PhabricatorObjectHandle');
+    // Values may either be handles (which are now legacy/deprecated) or
+    // strings. Load handles for any PHIDs.
+    $load = array();
+    $handles = array();
+    $select = array();
+    foreach ($values as $value) {
+      if ($value instanceof PhabricatorObjectHandle) {
+        $handles[$value->getPHID()] = $value;
+        $select[] = $value->getPHID();
+      } else {
+        $load[] = $value;
+        $select[] = $value;
+      }
+    }
+
+    // TODO: Once this code path simplifies, move this prefetch to setValue()
+    // so we can bulk load across multiple controls.
+
+    if ($load) {
+      $viewer = $this->getUser();
+      if (!$viewer) {
+        // TODO: Clean this up when handles go away.
+        throw new Exception(
+          pht('Call setUser() before rendering tokenizer string values.'));
+      }
+      $loaded_handles = $viewer->loadHandles($load);
+      $handles = $handles + iterator_to_array($loaded_handles);
+    }
+
+    // Reorder the list into input order.
+    $handles = array_select_keys($handles, $select);

    if ($this->getID()) {
      $id = $this->getID();
@ -55,7 +85,7 @@ final class AphrontFormTokenizerControl extends AphrontFormControl {
    $template = new AphrontTokenizerTemplateView();
    $template->setName($name);
    $template->setID($id);
-    $template->setValue($values);
+    $template->setValue($handles);

    $username = null;
    if ($this->user) {
@ -71,8 +101,8 @@ final class AphrontFormTokenizerControl extends AphrontFormControl {
      Javelin::initBehavior('aphront-basic-tokenizer', array(
        'id'          => $id,
        'src'         => $datasource_uri,
-        'value'       => mpull($values, 'getFullName', 'getPHID'),
-        'icons'       => mpull($values, 'getIcon', 'getPHID'),
+        'value'       => mpull($handles, 'getFullName', 'getPHID'),
+        'icons'       => mpull($handles, 'getIcon', 'getPHID'),
        'limit'       => $this->limit,
        'username'    => $username,
        'placeholder' => $placeholder,