From fbfcc375316123815ca8aed12bc8211ebb3861fa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 19 Jan 2018 14:43:20 -0800
Subject: [PATCH] Respect token limits for "Assign to" and custom datasource
 fields in Herald

Summary:
See PHI173. Currently, Herald has an "Assign to" action for tasks, and you can specify custom fields with datasource values (like users or projects) that have a limit (like 1 "Owner", or 12 "Jury Members").

Herald doesn't support these limits right now, so you can write `[ Assign to ][ X, Y, Z ]`. This just means "Assign to X", but make it more clear by actually enforcing the limit in the UI.

Test Plan:
  - Created a "projects" custom field with limit 1.
  - Tried to create actions that 'assign to' or 'set custom field to' more than one thing, got helpfully rebuffed by the UI.
  - Created an "add subscribers" action with more than one value.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18887
---
 resources/celerity/map.php                    | 22 +++++++++----------
 .../value/HeraldTokenizerFieldValue.php       |  1 +
 .../ManiphestTaskAssignOtherHeraldAction.php  |  5 ++---
 ...habricatorStandardCustomFieldTokenizer.php |  9 +++++++-
 .../js/application/herald/HeraldRuleEditor.js |  3 ++-
 5 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index f671233f69..1d8c9f4b64 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -416,7 +416,7 @@ return array(
     'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef',
     'rsrc/js/application/files/behavior-icon-composer.js' => '8499b6ab',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888',
-    'rsrc/js/application/herald/HeraldRuleEditor.js' => '2dff5579',
+    'rsrc/js/application/herald/HeraldRuleEditor.js' => 'dca75c0e',
     'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec',
     'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3',
     'rsrc/js/application/maniphest/behavior-batch-selector.js' => 'ad54037e',
@@ -580,7 +580,7 @@ return array(
     'global-drag-and-drop-css' => 'b556a948',
     'harbormaster-css' => 'f491c9f4',
     'herald-css' => 'cd8d0134',
-    'herald-rule-editor' => '2dff5579',
+    'herald-rule-editor' => 'dca75c0e',
     'herald-test-css' => 'a52e323e',
     'inline-comment-summary-css' => 'f23d4e8f',
     'javelin-aphlict' => 'e1d4b11a',
@@ -1106,15 +1106,6 @@ return array(
       'javelin-install',
       'javelin-event',
     ),
-    '2dff5579' => array(
-      'multirow-row-manager',
-      'javelin-install',
-      'javelin-util',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-json',
-      'phabricator-prefab',
-    ),
     '2ee659ce' => array(
       'javelin-install',
     ),
@@ -2030,6 +2021,15 @@ return array(
       'javelin-util',
       'phabricator-shaped-request',
     ),
+    'dca75c0e' => array(
+      'multirow-row-manager',
+      'javelin-install',
+      'javelin-util',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-json',
+      'phabricator-prefab',
+    ),
     'de2e896f' => array(
       'javelin-behavior',
       'javelin-dom',
diff --git a/src/applications/herald/value/HeraldTokenizerFieldValue.php b/src/applications/herald/value/HeraldTokenizerFieldValue.php
index bb3ceef1e7..215df29693 100644
--- a/src/applications/herald/value/HeraldTokenizerFieldValue.php
+++ b/src/applications/herald/value/HeraldTokenizerFieldValue.php
@@ -58,6 +58,7 @@ final class HeraldTokenizerFieldValue
         'datasourceURI' => $datasource->getDatasourceURI(),
         'browseURI' => $datasource->getBrowseURI(),
         'placeholder' => $datasource->getPlaceholderText(),
+        'limit' => $datasource->getLimit(),
       ),
     );
   }
diff --git a/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php b/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php
index 5a2591058e..27a5477623 100644
--- a/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php
+++ b/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php
@@ -22,9 +22,8 @@ final class ManiphestTaskAssignOtherHeraldAction
   }
 
   protected function getDatasource() {
-    // TODO: Eventually, it would be nice to get "limit = 1" exported from here
-    // up to the UI.
-    return new ManiphestAssigneeDatasource();
+    return id(new ManiphestAssigneeDatasource())
+      ->setLimit(1);
   }
 
   public function renderActionDescription($value) {
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
index 5c5f422375..65c112c684 100644
--- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
+++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
@@ -94,7 +94,14 @@ abstract class PhabricatorStandardCustomFieldTokenizer
   }
 
   public function getHeraldActionDatasource() {
-    return $this->getDatasource();
+    $datasource = $this->getDatasource();
+
+    $limit = $this->getFieldConfigValue('limit');
+    if ($limit) {
+      $datasource->setLimit($limit);
+    }
+
+    return $datasource;
   }
 
   private function renderHeraldHandleList($value) {
diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
index 69eeea4a26..443fe9811e 100644
--- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
+++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
@@ -283,7 +283,8 @@ JX.install('HeraldRuleEditor', {
       var tokenizerConfig = {
         src: spec.datasourceURI,
         placeholder: spec.placeholder,
-        browseURI: spec.browseURI
+        browseURI: spec.browseURI,
+        limit: spec.limit
       };
 
       var build = JX.Prefab.newTokenizerFromTemplate(