Support "Select" custom fields in Herald rules

Summary: Fixes T5016. Ref T655. Ref T8434. Ref T8726. For "select" custom fields, permit construction of Herald rules.

Test Plan: {F602997}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T655, T5016, T8434, T8726

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

src/__phutil_library_map__.php

@@ -2777,6 +2777,7 @@ phutil_register_library_map(array(
     'PhabricatorStandardCustomFieldText' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php',
     'PhabricatorStandardCustomFieldUsers' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php',
     'PhabricatorStandardPageView' => 'view/page/PhabricatorStandardPageView.php',
+    'PhabricatorStandardSelectCustomFieldDatasource' => 'infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php',
     'PhabricatorStatusController' => 'applications/system/controller/PhabricatorStatusController.php',
     'PhabricatorStatusUIExample' => 'applications/uiexample/examples/PhabricatorStatusUIExample.php',
     'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/PhabricatorStorageFixtureScopeGuard.php',
@@ -6678,6 +6679,7 @@ phutil_register_library_map(array(
     'PhabricatorStandardCustomFieldText' => 'PhabricatorStandardCustomField',
     'PhabricatorStandardCustomFieldUsers' => 'PhabricatorStandardCustomFieldPHIDs',
     'PhabricatorStandardPageView' => 'PhabricatorBarePageView',
+    'PhabricatorStandardSelectCustomFieldDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorStatusController' => 'PhabricatorController',
     'PhabricatorStatusUIExample' => 'PhabricatorUIExample',
     'PhabricatorStorageFixtureScopeGuard' => 'Phobject',

src/applications/herald/adapter/HeraldAdapter.php

@@ -882,23 +882,12 @@ abstract class HeraldAdapter extends Phobject {
     HeraldCondition $condition,
     array $handles) {
 
-    $impl = $this->getFieldImplementation($condition->getFieldName());
-    if ($impl) {
-      return $impl->getEditorValue(
-        $viewer,
-        $condition->getValue());
-    }
+    $field = $this->requireFieldImplementation($condition->getFieldName());
 
-    $value = $condition->getValue();
-    if (is_array($value)) {
-      $value_map = array();
-      foreach ($value as $k => $phid) {
-        $value_map[$phid] = $handles[$phid]->getName();
-      }
-      $value = $value_map;
-    }
-
-    return $value;
+    return $field->getEditorValue(
+      $viewer,
+      $condition->getFieldCondition(),
+      $condition->getValue());
   }
 
   public function renderRuleAsText(
@@ -1020,28 +1009,12 @@ abstract class HeraldAdapter extends Phobject {
     PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
-    $impl = $this->getFieldImplementation($condition->getFieldName());
-    if ($impl) {
-      return $impl->renderConditionValue(
-        $viewer,
-        $condition->getFieldCondition(),
-        $condition->getValue());
-    }
+    $field = $this->requireFieldImplementation($condition->getFieldName());
 
-    $value = $condition->getValue();
-    if (!is_array($value)) {
-      $value = array($value);
-    }
-
-    foreach ($value as $index => $val) {
-      $handle = $handles->getHandleIfExists($val);
-      if ($handle) {
-        $value[$index] = $handle->renderLink();
-      }
-    }
-
-    $value = phutil_implode_html(', ', $value);
-    return $value;
+    return $field->renderConditionValue(
+      $viewer,
+      $condition->getFieldCondition(),
+      $condition->getValue());
   }
 
   private function renderActionTargetAsText(

src/applications/herald/controller/HeraldTranscriptController.php

@@ -116,6 +116,9 @@ final class HeraldTranscriptController extends HeraldController {
   }
 
   protected function renderConditionTestValue($condition, $handles) {
+    // TODO: This is all a hacky mess and should be driven through FieldValue
+    // eventually.
+
     switch ($condition->getFieldName()) {
       case HeraldAnotherRuleField::FIELDCONST:
         $value = array($condition->getTestValue());
@@ -128,14 +131,12 @@ final class HeraldTranscriptController extends HeraldController {
     if (!is_scalar($value) && $value !== null) {
       foreach ($value as $key => $phid) {
         $handle = idx($handles, $phid);
-        if ($handle) {
+        if ($handle && $handle->isComplete()) {
           $value[$key] = $handle->getName();
         } else {
-          // This shouldn't ever really happen as we are supposed to have
-          // grabbed handles for everything, but be super liberal in what
-          // we accept here since we expect all sorts of weird issues as we
-          // version the system.
-          $value[$key] = pht('Unknown Object #%s', $phid);
+          // This happens for things like task priorities, statuses, and
+          // custom fields.
+          $value[$key] = $phid;
         }
       }
       sort($value);

src/applications/herald/field/HeraldField.php

@@ -24,6 +24,10 @@ abstract class HeraldField extends Phobject {
     throw new PhutilMethodNotImplementedException();
   }
 
+  protected function getDatasourceValueMap() {
+    return null;
+  }
+
   public function getHeraldFieldConditions() {
     $standard_type = $this->getHeraldFieldStandardType();
     switch ($standard_type) {
@@ -103,9 +107,16 @@ abstract class HeraldField extends Phobject {
           case HeraldAdapter::CONDITION_NOT_EXISTS:
             return new HeraldEmptyFieldValue();
           default:
-            return id(new HeraldTokenizerFieldValue())
+            $tokenizer = id(new HeraldTokenizerFieldValue())
               ->setKey($this->getHeraldFieldName())
               ->setDatasource($this->getDatasource());
+
+            $value_map = $this->getDatasourceValueMap();
+            if ($value_map !== null) {
+              $tokenizer->setValueMap($value_map);
+            }
+
+            return $tokenizer;
         }
         break;
 
@@ -130,50 +141,18 @@ abstract class HeraldField extends Phobject {
     $value) {
 
     $value_type = $this->getHeraldFieldValueType($condition);
-    if ($value_type instanceof HeraldFieldValue) {
-      $value_type->setViewer($viewer);
-      return $value_type->renderFieldValue($value);
-    }
-
-    // TODO: While this is less of a mess than it used to be, it would still
-    // be nice to push this down into individual fields better eventually and
-    // stop guessing which values are PHIDs and which aren't.
-
-    if (!is_array($value)) {
-      return $value;
-    }
-
-    $type_unknown = PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN;
-
-    foreach ($value as $key => $val) {
-      if (is_string($val)) {
-        if (phid_get_type($val) !== $type_unknown) {
-          $value[$key] = $viewer->renderHandle($val);
-        }
-      }
-    }
-
-    return phutil_implode_html(', ', $value);
+    $value_type->setViewer($viewer);
+    return $value_type->renderFieldValue($value);
   }
 
   public function getEditorValue(
     PhabricatorUser $viewer,
+    $condition,
     $value) {
 
-    // TODO: This should be better structured and pushed down into individual
-    // fields. As it is used to manually build tokenizer tokens, it can
-    // probably be removed entirely.
-
-    if (is_array($value)) {
-      $handles = $viewer->loadHandles($value);
-      $value_map = array();
-      foreach ($value as $k => $phid) {
-        $value_map[$phid] = $handles[$phid]->getName();
-      }
-      $value = $value_map;
-    }
-
-    return $value;
+    $value_type = $this->getHeraldFieldValueType($condition);
+    $value_type->setViewer($viewer);
+    return $value_type->renderEditorValue($value);
   }
 
   final public function setAdapter(HeraldAdapter $adapter) {

src/applications/herald/value/HeraldEmptyFieldValue.php

@@ -15,4 +15,8 @@ final class HeraldEmptyFieldValue
     return null;
   }
 
+  public function renderEditorValue($value) {
+    return null;
+  }
+
 }

src/applications/herald/value/HeraldFieldValue.php

@@ -12,6 +12,7 @@ abstract class HeraldFieldValue extends Phobject {
   abstract public function getFieldValueKey();
   abstract public function getControlType();
   abstract public function renderFieldValue($value);
+  abstract public function renderEditorValue($value);
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;

src/applications/herald/value/HeraldSelectFieldValue.php

@@ -61,4 +61,8 @@ final class HeraldSelectFieldValue
     return idx($options, $value, $value);
   }
 
+  public function renderEditorValue($value) {
+    return $value;
+  }
+
 }

src/applications/herald/value/HeraldTextFieldValue.php

@@ -11,9 +11,12 @@ final class HeraldTextFieldValue
     return self::CONTROL_TEXT;
   }
 
-
   public function renderFieldValue($value) {
     return $value;
   }
 
+  public function renderEditorValue($value) {
+    return $value;
+  }
+
 }

src/applications/herald/value/HeraldTokenizerFieldValue.php

@@ -5,6 +5,7 @@ final class HeraldTokenizerFieldValue
 
   private $key;
   private $datasource;
+  private $valueMap;
 
   public function setKey($key) {
     $this->key = $key;
@@ -24,6 +25,15 @@ final class HeraldTokenizerFieldValue
     return $this->datasource;
   }
 
+  public function setValueMap(array $value_map) {
+    $this->valueMap = $value_map;
+    return $this;
+  }
+
+  public function getValueMap() {
+    return $this->valueMap;
+  }
+
   public function getFieldValueKey() {
     if ($this->getKey() === null) {
       throw new PhutilInvalidStateException('setKey');
@@ -54,7 +64,40 @@ final class HeraldTokenizerFieldValue
 
   public function renderFieldValue($value) {
     $viewer = $this->getViewer();
+    $value = (array)$value;
+
+    if ($this->valueMap !== null) {
+      foreach ($value as $k => $v) {
+        $value[$k] = idx($this->valueMap, $v, $v);
+      }
+      return implode(', ', $value);
+    }
+
     return $viewer->renderHandleList((array)$value)->setAsInline(true);
   }
 
+  public function renderEditorValue($value) {
+    $viewer = $this->getViewer();
+    $value = (array)$value;
+
+    // TODO: This should eventually render properly through the datasource
+    // to get icons and colors.
+
+    if ($this->valueMap !== null) {
+      $map = array();
+      foreach ($value as $v) {
+        $map[$v] = idx($this->valueMap, $v, $v);
+      }
+      return $map;
+    }
+
+    $handles = $viewer->loadHandles($value);
+
+    $map = array();
+    foreach ($value as $v) {
+      $map[$v] = $handles[$v]->getName();
+    }
+    return $map;
+  }
+
 }

src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php

@@ -21,36 +21,8 @@ final class ManiphestTaskPriorityHeraldField
     return new ManiphestTaskPriorityDatasource();
   }
 
-  public function renderConditionValue(
-    PhabricatorUser $viewer,
-    $condition,
-    $value) {
-
-    $priority_map = ManiphestTaskPriority::getTaskPriorityMap();
-
-    $value = (array)$value;
-    foreach ($value as $index => $val) {
-      $name = idx($priority_map, $val);
-      if ($name !== null) {
-        $value[$index] = $name;
-      }
-    }
-
-    return implode(', ', $value);
-  }
-
-  public function getEditorValue(
-    PhabricatorUser $viewer,
-    $value) {
-
-    $priority_map = ManiphestTaskPriority::getTaskPriorityMap();
-
-    $value_map = array();
-    foreach ($value as $priority) {
-      $value_map[$priority] = idx($priority_map, $priority, $priority);
-    }
-
-    return $value_map;
+  protected function getDatasourceValueMap() {
+    return ManiphestTaskPriority::getTaskPriorityMap();
   }
 
 }

src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php

@@ -21,36 +21,8 @@ final class ManiphestTaskStatusHeraldField
     return new ManiphestTaskStatusDatasource();
   }
 
-  public function renderConditionValue(
-    PhabricatorUser $viewer,
-    $condition,
-    $value) {
-
-    $status_map = ManiphestTaskStatus::getTaskStatusMap();
-
-    $value = (array)$value;
-    foreach ($value as $index => $val) {
-      $name = idx($status_map, $val);
-      if ($name !== null) {
-        $value[$index] = $name;
-      }
-    }
-
-    return implode(', ', $value);
-  }
-
-  public function getEditorValue(
-    PhabricatorUser $viewer,
-    $value) {
-
-    $status_map = ManiphestTaskStatus::getTaskStatusMap();
-
-    $value_map = array();
-    foreach ($value as $status) {
-      $value_map[$status] = idx($status_map, $status, $status);
-    }
-
-    return $value_map;
+  protected function getDatasourceValueMap() {
+    return ManiphestTaskStatus::getTaskStatusMap();
   }
 
 }

src/infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php

@@ -0,0 +1,93 @@
+<?php
+
+final class PhabricatorStandardSelectCustomFieldDatasource
+  extends PhabricatorTypeaheadDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Values');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type a field value...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return null;
+  }
+
+  public function loadResults() {
+    $viewer = $this->getViewer();
+
+    $class = $this->getParameter('object');
+    if (!class_exists($class)) {
+      throw new Exception(
+        pht(
+          'Custom field class "%s" does not exist.',
+          $class));
+    }
+
+    $reflection = new ReflectionClass($class);
+    $interface = 'PhabricatorCustomFieldInterface';
+    if (!$reflection->implementsInterface($interface)) {
+      throw new Exception(
+        pht(
+          'Custom field class "%s" does not implement interface "%s".',
+          $class,
+          $interface));
+    }
+
+    $role = $this->getParameter('role');
+    if (!strlen($role)) {
+      throw new Exception(pht('No custom field role specified.'));
+    }
+
+    $object = newv($class, array());
+    $field_list = PhabricatorCustomField::getObjectFields($object, $role);
+
+    $field_key = $this->getParameter('key');
+    if (!strlen($field_key)) {
+      throw new Exception(pht('No custom field key specified.'));
+    }
+
+    $field = null;
+    foreach ($field_list->getFields() as $candidate_field) {
+      if ($candidate_field->getFieldKey() == $field_key) {
+        $field = $candidate_field;
+        break;
+      }
+    }
+
+    if ($field === null) {
+      throw new Exception(
+        pht(
+          'No field with field key "%s" exists for objects of class "%s" with '.
+          'custom field role "%s".',
+          $field_key,
+          $class,
+          $role));
+    }
+
+    if (!($field instanceof PhabricatorStandardCustomFieldSelect)) {
+      $field = $field->getProxy();
+      if (!($field instanceof PhabricatorStandardCustomFieldSelect)) {
+        throw new Exception(
+          pht(
+            'Field "%s" is not a standard select field, nor a proxy of a '.
+            'standard select field.',
+            $field_key));
+      }
+    }
+
+    $options = $field->getOptions();
+
+    $results = array();
+    foreach ($options as $key => $option) {
+      $results[] = id(new PhabricatorTypeaheadResult())
+        ->setName($option)
+        ->setPHID($key);
+    }
+
+    return $this->filterResultsAgainstTokens($results);
+  }
+
+}

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php

@@ -59,7 +59,7 @@ final class PhabricatorStandardCustomFieldSelect
     $form->appendChild($control);
   }
 
-  private function getOptions() {
+  public function getOptions() {
     return $this->getFieldConfigValue('options', array());
   }
 
@@ -79,7 +79,6 @@ final class PhabricatorStandardCustomFieldSelect
     return idx($this->getOptions(), $this->getFieldValue());
   }
 
-
   public function getApplicationTransactionTitle(
     PhabricatorApplicationTransaction $xaction) {
     $author_phid = $xaction->getAuthorPHID();
@@ -110,4 +109,31 @@ final class PhabricatorStandardCustomFieldSelect
     }
   }
 
+  public function shouldAppearInHerald() {
+    return true;
+  }
+
+  public function getHeraldFieldConditions() {
+    return array(
+      HeraldAdapter::CONDITION_IS_ANY,
+      HeraldAdapter::CONDITION_IS_NOT_ANY,
+    );
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    $parameters = array(
+      'object' => get_class($this->getObject()),
+      'role' => PhabricatorCustomField::ROLE_HERALD,
+      'key' => $this->getFieldKey(),
+    );
+
+    $datasource = id(new PhabricatorStandardSelectCustomFieldDatasource())
+      ->setParameters($parameters);
+
+    return id(new HeraldTokenizerFieldValue())
+      ->setKey('custom.'.$this->getFieldKey())
+      ->setDatasource($datasource)
+      ->setValueMap($this->getOptions());
+  }
+
 }