From 9532bfbb32c83085725ab405c7160be4b45eb9a0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 13 Apr 2019 09:02:30 -0700
Subject: [PATCH] Improve trigger editor behavior when switching to/from
 tokenizers

Summary:
Ref T13269. See D20329. When we switch trigger rule control types, reset the rule value.

Also, pick slightly nicer defaults for status/priority.

Test Plan:
  - Created a "Change Status To: X" rule.
  - Saved it.
  - Edited it.
  - Selected "Assign to" for the existing action's dropdown.
  - Before: tokenizer filled with nonsense.
  - After: tokenizer cleared.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13269

Differential Revision: https://secure.phabricator.com/D20416
---
 resources/celerity/map.php                    |  4 ++--
 ...torProjectTriggerManiphestPriorityRule.php |  2 +-
 ...catorProjectTriggerManiphestStatusRule.php |  2 +-
 .../js/application/trigger/TriggerRule.js     | 19 ++++++++++++++++++-
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 9f41f0584c..a9a62b86e2 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -433,7 +433,7 @@ return array(
     'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '600f440c',
     'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => '2bdadf1a',
     'rsrc/js/application/transactions/behavior-transaction-list.js' => '9cec214e',
-    'rsrc/js/application/trigger/TriggerRule.js' => '1c60c3fc',
+    'rsrc/js/application/trigger/TriggerRule.js' => '41b7b4f6',
     'rsrc/js/application/trigger/TriggerRuleControl.js' => '5faf27b9',
     'rsrc/js/application/trigger/TriggerRuleEditor.js' => 'b49fd60c',
     'rsrc/js/application/trigger/TriggerRuleType.js' => '4feea7d3',
@@ -894,7 +894,7 @@ return array(
     'syntax-default-css' => '055fc231',
     'syntax-highlighting-css' => '4234f572',
     'tokens-css' => 'ce5a50bd',
-    'trigger-rule' => '1c60c3fc',
+    'trigger-rule' => '41b7b4f6',
     'trigger-rule-control' => '5faf27b9',
     'trigger-rule-editor' => 'b49fd60c',
     'trigger-rule-type' => '4feea7d3',
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
index 98a03a1393..c8453b3d7e 100644
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestPriorityRule.php
@@ -54,7 +54,7 @@ final class PhabricatorProjectTriggerManiphestPriorityRule
   }
 
   protected function getDefaultValue() {
-    return head_key(ManiphestTaskPriority::getTaskPriorityMap());
+    return ManiphestTaskPriority::getDefaultPriority();
   }
 
   protected function getPHUIXControlType() {
diff --git a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
index b11d7567de..a2fda831a0 100644
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
@@ -53,7 +53,7 @@ final class PhabricatorProjectTriggerManiphestStatusRule
   }
 
   protected function getDefaultValue() {
-    return head_key(ManiphestTaskStatus::getTaskStatusMap());
+    return ManiphestTaskStatus::getDefaultClosedStatus();
   }
 
   protected function getPHUIXControlType() {
diff --git a/webroot/rsrc/js/application/trigger/TriggerRule.js b/webroot/rsrc/js/application/trigger/TriggerRule.js
index cf117e24d9..86f9ecb966 100644
--- a/webroot/rsrc/js/application/trigger/TriggerRule.js
+++ b/webroot/rsrc/js/application/trigger/TriggerRule.js
@@ -98,7 +98,24 @@ JX.install('TriggerRule', {
     },
 
     _onTypeChange: function(control) {
-      this.setType(control.value);
+      var new_type = control.value;
+
+      this.setType(new_type);
+
+      // Before we build a new control, change the rule value to be appropriate
+      // for the new rule type.
+
+      // TODO: Currently, we set the rule value to the default value for the
+      // type. This works most of the time, but if the user selects type "A",
+      // makes a change to the value, selects type "B", then changes back to
+      // type "A", it would be better to retain their edit. This is currently
+      // difficult because of tokenizers: if you save their value, you get a
+      // list of PHIDs which do not restore cleanly into tokens later.
+
+      var editor = this.getEditor();
+      var type = editor.getType(new_type);
+      this.setValue(type.getDefaultValue());
+
       this._rebuildValueControl();
     },