Implement "user" and "users" Maniphest custom fields

Summary:
Ref T2575. Implements "user" (zero or one users) and "users" (zero or more users) field types.

Also allows custom fields to participate in the handle pipeline.

Test Plan: {F35071}

Reviewers: hach-que, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2575

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

src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php

@@ -21,6 +21,8 @@ class ManiphestAuxiliaryFieldDefaultSpecification
   const TYPE_BOOL     = 'bool';
   const TYPE_DATE     = 'date';
   const TYPE_REMARKUP = 'remarkup';
+  const TYPE_USER     = 'user';
+  const TYPE_USERS    = 'users';
 
   public function getFieldType() {
     return $this->fieldType;
@@ -102,6 +104,14 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         $control = new PhabricatorRemarkupControl();
         $control->setUser($this->getUser());
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        $control = new AphrontFormTokenizerControl();
+        $control->setDatasource('/typeahead/common/users/');
+        if ($type == self::TYPE_USER) {
+          $control->setLimit(1);
+        }
+        break;
       default:
         $label = $this->getLabel();
         throw new ManiphestAuxiliaryFieldTypeException(
@@ -121,6 +131,15 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         $control->setValue($this->getValue());
         $control->setName('auxiliary_date_'.$this->getAuxiliaryKey());
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        $control->setName('auxiliary_tokenizer_'.$this->getAuxiliaryKey());
+        $value = array();
+        foreach ($this->getValue() as $phid) {
+          $value[$phid] = $this->getHandle($phid)->getFullName();
+        }
+        $control->setValue($value);
+        break;
       default:
         $control->setValue($this->getValue());
         $control->setName('auxiliary['.$this->getAuxiliaryKey().']');
@@ -135,11 +154,20 @@ class ManiphestAuxiliaryFieldDefaultSpecification
   }
 
   public function setValueFromRequest(AphrontRequest $request) {
-    switch ($this->getFieldType()) {
+    $type = $this->getFieldType();
+    switch ($type) {
       case self::TYPE_DATE:
         $control = $this->renderControl();
         $value = $control->readValueFromRequest($request);
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        $name = 'auxiliary_tokenizer_'.$this->getAuxiliaryKey();
+        $value = $request->getArr($name);
+        if ($type == self::TYPE_USER) {
+          $value = array_slice($value, 0, 1);
+        }
+        break;
       default:
         $aux_post_values = $request->getArr('auxiliary');
         $value = idx($aux_post_values, $this->getAuxiliaryKey(), '');
@@ -149,17 +177,34 @@ class ManiphestAuxiliaryFieldDefaultSpecification
   }
 
   public function getValueForStorage() {
+    switch ($this->getFieldType()) {
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        return json_encode($this->getValue());
+      default:
         return $this->getValue();
     }
+  }
 
   public function setValueFromStorage($value) {
+    switch ($this->getFieldType()) {
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        $value = json_decode($value, true);
+        if (!is_array($value)) {
+          $value = array();
+        }
+        break;
+      default:
+        break;
+    }
     return $this->setValue($value);
   }
 
   public function validate() {
     switch ($this->getFieldType()) {
       case self::TYPE_INT:
-        if (!is_numeric($this->getValue())) {
+        if ($this->getValue() && !is_numeric($this->getValue())) {
           throw new ManiphestAuxiliaryFieldValidationException(
             pht(
               '%s must be an integer value.',
@@ -173,13 +218,22 @@ class ManiphestAuxiliaryFieldDefaultSpecification
       case self::TYPE_SELECT:
         return true;
       case self::TYPE_DATE:
-        if ($this->getValue() <= 0) {
+        if ((int)$this->getValue() <= 0) {
           throw new ManiphestAuxiliaryFieldValidationException(
             pht(
               '%s must be a valid date.',
               $this->getLabel()));
         }
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        if (!is_array($this->getValue())) {
+          throw new ManiphestAuxiliaryFieldValidationException(
+            pht(
+              '%s is not a valid list of user PHIDs.',
+              $this->getLabel()));
+        }
+        break;
     }
   }
 
@@ -192,6 +246,15 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         }
         $this->setValue($value);
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        if (!is_array($value)) {
+          $value = array();
+        } else {
+          $value = array_values($value);
+        }
+        $this->setValue($value);
+        break;
       default:
         $this->setValue((string)$value);
         break;
@@ -222,10 +285,30 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         return $this->getMarkupEngine()->getOutput(
           $this,
           'default');
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        return $this->renderHandleList($this->getValue());
     }
     return parent::renderForDetailView();
   }
 
+  public function getRequiredHandlePHIDs() {
+    switch ($this->getFieldType()) {
+      case self::TYPE_USER;
+      case self::TYPE_USERS:
+        return $this->getValue();
+    }
+    return parent::getRequiredHandlePHIDs();
+  }
+
+  protected function renderHandleList(array $phids) {
+    $links = array();
+    foreach ($phids as $phid) {
+      $links[] = $this->getHandle($phid)->renderLink();
+    }
+    return phutil_implode_html(', ', $links);
+  }
+
   public function renderTransactionDescription(
     ManiphestTransaction $transaction,
     $target) {
@@ -266,6 +349,12 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         // TODO: After we get ApplicationTransactions, straighten this out.
         $desc = "updated field '{$label}'";
         break;
+      case self::TYPE_USER:
+      case self::TYPE_USERS:
+        // TODO: As above, this is a mess that should get straightened out,
+        // but it will be easier after T2217.
+        $desc = "updated field '{$label}'";
+        break;
       default:
         if (!strlen($old)) {
           if (!strlen($new)) {

src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php

@@ -16,6 +16,7 @@ abstract class ManiphestAuxiliaryFieldSpecification
   private $user;
   private $task;
   private $markupEngine;
+  private $handles;
 
   public function setTask(ManiphestTask $task) {
     $this->task = $task;
@@ -159,6 +160,25 @@ abstract class ManiphestAuxiliaryFieldSpecification
     return 'updated a custom field';
   }
 
+  public function getRequiredHandlePHIDs() {
+    return array();
+  }
+
+  public function setHandles(array $handles) {
+    assert_instances_of($handles, 'PhabricatorObjectHandle');
+    $this->handles = array_select_keys(
+      $handles,
+      $this->getRequiredHandlePHIDs());
+    return $this;
+  }
+
+  public function getHandle($phid) {
+    if (empty($this->handles[$phid])) {
+      throw new Exception(
+        "Field is requesting a handle ('{$phid}') it did not require.");
+    }
+    return $this->handles[$phid];
+  }
 
   public function getMarkupFields() {
     return array();

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -35,6 +35,9 @@ final class ManiphestTaskDetailController extends ManiphestController {
       'taskID = %d ORDER BY id ASC',
       $task->getID());
 
+    $extensions = ManiphestTaskExtensions::newExtensions();
+    $aux_fields = $extensions->loadFields($task, $user);
+
     $e_commit = PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT;
     $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK;
     $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK;
@@ -82,8 +85,18 @@ final class ManiphestTaskDetailController extends ManiphestController {
     }
 
     $phids = array_keys($phids);
+
+    $phids = array_merge(
+      $phids,
+      array_mergev(mpull($aux_fields, 'getRequiredHandlePHIDs')));
+
     $this->loadHandles($phids);
 
+    $handles = $this->getLoadedHandles();
+    foreach ($aux_fields as $aux_field) {
+      $aux_field->setHandles($handles);
+    }
+
     $context_bar = null;
 
     if ($parent_task) {
@@ -127,9 +140,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
       }
     }
 
-    $extensions = ManiphestTaskExtensions::newExtensions();
-    $aux_fields = $extensions->loadFields($task, $user);
-
     foreach ($aux_fields as $aux_field) {
       foreach ($aux_field->getMarkupFields() as $markup_field) {
         $engine->addObject($aux_field, $markup_field);

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -111,12 +111,11 @@ final class ManiphestTaskEditController extends ManiphestController {
       foreach ($aux_fields as $aux_field) {
         $aux_field->setValueFromRequest($request);
 
-        if ($aux_field->isRequired() && !strlen($aux_field->getValue())) {
+        if ($aux_field->isRequired() && !$aux_field->getValue()) {
           $errors[] = $aux_field->getLabel() . ' is required.';
           $aux_field->setError('Required');
         }
 
-        if (strlen($aux_field->getValue())) {
         try {
           $aux_field->validate();
         } catch (Exception $e) {
@@ -124,7 +123,6 @@ final class ManiphestTaskEditController extends ManiphestController {
           $aux_field->setError('Invalid');
         }
       }
-      }
 
       if ($errors) {
         $task->setPriority($request->getInt('priority'));
@@ -283,7 +281,8 @@ final class ManiphestTaskEditController extends ManiphestController {
     $phids = array_merge(
       array($task->getOwnerPHID()),
       $task->getCCPHIDs(),
-      $task->getProjectPHIDs());
+      $task->getProjectPHIDs(),
+      array_mergev(mpull($aux_fields, 'getRequiredHandlePHIDs')));
 
     if ($parent_task) {
       $phids[] = $parent_task->getPHID();
@@ -294,6 +293,10 @@ final class ManiphestTaskEditController extends ManiphestController {
 
     $handles = $this->loadViewerHandles($phids);
 
+    foreach ($aux_fields as $aux_field) {
+      $aux_field->setHandles($handles);
+    }
+
     $tvalues = mpull($handles, 'getFullName', 'getPHID');
 
     $error_view = null;
@@ -424,7 +427,6 @@ final class ManiphestTaskEditController extends ManiphestController {
               pht('Create New Project')))
           ->setDatasource('/typeahead/common/projects/'));
 
-    if ($aux_fields) {
     foreach ($aux_fields as $aux_field) {
       if ($aux_field->isRequired() &&
           !$aux_field->getError() &&
@@ -435,7 +437,6 @@ final class ManiphestTaskEditController extends ManiphestController {
       $aux_control = $aux_field->renderControl();
       $form->appendChild($aux_control);
     }
-    }
 
     require_celerity_resource('aphront-error-view-css');
 

src/docs/userguide/maniphest_custom.diviner

@@ -44,8 +44,8 @@ Each array key must be unique, and is used to organize the internal storage of
 the field. These options are available:
 
   - **label**: Display label for the field on the edit and detail interfaces.
-  - **type**: Field type, one of **int**, **string**, **bool**, **select**,
-    **remarkup**, or **date**.
+  - **type**: Field type: one of **int**, **string**, **bool**, **select**,
+    **remarkup**, **user**, **users**, or **date**.
   - **caption**: A caption to display underneath the field (optional).
   - **required**: True if the user should be required to provide a value.
   - **options**: If type is set to **select**, provide options for the dropdown
@@ -54,8 +54,10 @@ the field. These options are available:
     show next to the checkbox.
   - **checkbox-value**: If type is set to **bool**, the value to show on
     the detail view when the checkbox is selected.
-  - **default**: Default field value. For **date**, you can use a string like
-    `"July 4, 1990"`, `"5PM today"`, or any other valid input to `strtotime()`.
+  - **default**: Default field value.
+    - For **date**, you can use a string like `"July 4, 1990"`, `"5PM today"`,
+      or any other valid input to `strtotime()`.
+    - For **user** and **users**, you can use an array of user PHIDs.
   - **copy**: When a user creates a task, the UI gives them an option to
     "Create Another Similar Task". Some fields from the original task are copied
     into the new task, while others are not; by default, fields are not copied.