Save stacked actions in drafts, not just comments

Summary:
Ref T9132. Fixes T4580. Thhat might actually have been fixed a while ago or something since it describes a buggy/bad interaction which doesn't reproduce for me at HEAD.

This saves and restores all the stacked actions (subscribers, projects, etc) so that you don't lose anything if you close a window by accident.

Test Plan:
Added a bunch of actions in various states, reloaded the page, draft stuck around.

Submitted form, actions didn't stick around anymore.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4580, T9132

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

resources/celerity/map.php

@@ -8,7 +8,7 @@
 return array(
   'names' => array(
     'core.pkg.css' => '4d47b0a9',
-    'core.pkg.js' => '21eccc42',
+    'core.pkg.js' => '3c0f7f9b',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '2de124c9',
     'differential.pkg.js' => '6223dd9d',
@@ -427,7 +427,7 @@ return array(
     'rsrc/js/application/repository/repository-crossreference.js' => 'e5339c43',
     'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08',
     'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => '887ad43f',
-    'rsrc/js/application/transactions/behavior-comment-actions.js' => 'f293e8a0',
+    'rsrc/js/application/transactions/behavior-comment-actions.js' => '6de53e91',
     'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
     'rsrc/js/application/transactions/behavior-show-older-transactions.js' => 'dbbf48b6',
     'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => 'b23b49e6',
@@ -456,7 +456,7 @@ return array(
     'rsrc/js/core/KeyboardShortcutManager.js' => 'c1700f6f',
     'rsrc/js/core/MultirowRowManager.js' => 'b5d57730',
     'rsrc/js/core/Notification.js' => 'ccf1cbf8',
-    'rsrc/js/core/Prefab.js' => '2381d07a',
+    'rsrc/js/core/Prefab.js' => 'be38fe4e',
     'rsrc/js/core/ShapedRequest.js' => '7cbe244b',
     'rsrc/js/core/TextAreaUtils.js' => '5c93c52c',
     'rsrc/js/core/Title.js' => 'df5e11d2',
@@ -506,7 +506,7 @@ return array(
     'rsrc/js/phuix/PHUIXActionListView.js' => 'b5c256b8',
     'rsrc/js/phuix/PHUIXActionView.js' => '8cf6d262',
     'rsrc/js/phuix/PHUIXDropdownMenu.js' => 'bd4c8dca',
-    'rsrc/js/phuix/PHUIXFormControl.js' => '1adf0d30',
+    'rsrc/js/phuix/PHUIXFormControl.js' => '8fba1997',
     'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
   ),
   'symbols' => array(
@@ -571,7 +571,7 @@ return array(
     'javelin-behavior-audit-preview' => 'd835b03a',
     'javelin-behavior-bulk-job-reload' => 'edf8a145',
     'javelin-behavior-choose-control' => '6153c708',
-    'javelin-behavior-comment-actions' => 'f293e8a0',
+    'javelin-behavior-comment-actions' => '6de53e91',
     'javelin-behavior-config-reorder-fields' => 'b6993408',
     'javelin-behavior-conpherence-drag-and-drop-photo' => 'cf86d16a',
     'javelin-behavior-conpherence-menu' => '1d45c74d',
@@ -757,7 +757,7 @@ return array(
     'phabricator-notification-menu-css' => 'f31c0bde',
     'phabricator-object-selector-css' => '85ee8ce6',
     'phabricator-phtize' => 'd254d646',
-    'phabricator-prefab' => '2381d07a',
+    'phabricator-prefab' => 'be38fe4e',
     'phabricator-remarkup-css' => 'b1c10368',
     'phabricator-search-results-css' => '7dea472c',
     'phabricator-shaped-request' => '7cbe244b',
@@ -831,7 +831,7 @@ return array(
     'phuix-action-list-view' => 'b5c256b8',
     'phuix-action-view' => '8cf6d262',
     'phuix-dropdown-menu' => 'bd4c8dca',
-    'phuix-form-control-view' => '1adf0d30',
+    'phuix-form-control-view' => '8fba1997',
     'phuix-icon-view' => 'bff6884b',
     'policy-css' => '957ea14c',
     'policy-edit-css' => '815c66f7',
@@ -946,10 +946,6 @@ return array(
       'javelin-util',
       'javelin-reactor-node-calmer',
     ),
-    '1adf0d30' => array(
-      'javelin-install',
-      'javelin-dom',
-    ),
     '1ae869f2' => array(
       'javelin-install',
       'javelin-util',
@@ -1007,18 +1003,6 @@ return array(
       'javelin-workflow',
       'javelin-util',
     ),
-    '2381d07a' => array(
-      'javelin-install',
-      'javelin-util',
-      'javelin-dom',
-      'javelin-typeahead',
-      'javelin-tokenizer',
-      'javelin-typeahead-preloaded-source',
-      'javelin-typeahead-ondemand-source',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-util',
-    ),
     '246dc085' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1342,6 +1326,14 @@ return array(
       'phabricator-drag-and-drop-file-upload',
       'phabricator-textareautils',
     ),
+    '6de53e91' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'javelin-dom',
+      'phuix-form-control-view',
+      'phuix-icon-view',
+    ),
     '6eff08aa' => array(
       'javelin-install',
       'javelin-util',
@@ -1527,6 +1519,10 @@ return array(
       'javelin-dom',
       'javelin-stratcom',
     ),
+    '8fba1997' => array(
+      'javelin-install',
+      'javelin-dom',
+    ),
     '9007c197' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1762,6 +1758,18 @@ return array(
       'javelin-util',
       'javelin-request',
     ),
+    'be38fe4e' => array(
+      'javelin-install',
+      'javelin-util',
+      'javelin-dom',
+      'javelin-typeahead',
+      'javelin-tokenizer',
+      'javelin-typeahead-preloaded-source',
+      'javelin-typeahead-ondemand-source',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-util',
+    ),
     'bff6884b' => array(
       'javelin-install',
       'javelin-dom',
@@ -1972,14 +1980,6 @@ return array(
       'javelin-workflow',
       'javelin-json',
     ),
-    'f293e8a0' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'javelin-dom',
-      'phuix-form-control-view',
-      'phuix-icon-view',
-    ),
     'f36e01af' => array(
       'javelin-behavior',
       'javelin-behavior-device',

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -1025,6 +1025,11 @@ abstract class PhabricatorEditEngine
 
     $comment_text = $request->getStr('comment');
 
+    $actions = $request->getStr('editengine.actions');
+    if ($actions) {
+      $actions = phutil_json_decode($actions);
+    }
+
     if ($is_preview) {
       $version_key = PhabricatorVersionedDraft::KEY_VERSION;
       $request_version = $request->getInt($version_key);
@@ -1036,14 +1041,15 @@ abstract class PhabricatorEditEngine
           $current_version);
 
         // TODO: This is just a proof of concept.
-        $draft->setProperty('temporary.comment', $comment_text);
+        $draft
-        $draft->save();
+          ->setProperty('temporary.comment', $comment_text)
+          ->setProperty('actions', $actions)
+          ->save();
       }
     }
 
     $xactions = array();
 
-    $actions = $request->getStr('editengine.actions');
     if ($actions) {
       $type_map = array();
       foreach ($fields as $field) {
@@ -1056,7 +1062,6 @@ abstract class PhabricatorEditEngine
         }
       }
 
-      $actions = phutil_json_decode($actions);
       foreach ($actions as $action) {
         $type = idx($action, 'type');
         if (!$type) {

src/applications/transactions/editfield/PhabricatorSelectEditField.php

@@ -53,6 +53,7 @@ final class PhabricatorSelectEditField
       ->setPHUIXControlSpecification(
         array(
           'options' => $this->getOptions(),
+          'order' => array_keys($this->getOptions()),
           'value' => $default_value,
         ));
 

src/applications/transactions/edittype/PhabricatorEditType.php

@@ -104,4 +104,8 @@ abstract class PhabricatorEditType extends Phobject {
     return null;
   }
 
+  public function getCommentActionValueFromDraftValue($value) {
+    return $value;
+  }
+
 }

src/applications/transactions/edittype/PhabricatorPHIDListEditType.php

@@ -86,5 +86,19 @@ abstract class PhabricatorPHIDListEditType
     );
   }
 
+  public function getCommentActionValueFromDraftValue($value) {
+    $datasource = $this->getDatasource();
+
+    if (!$datasource) {
+      return array();
+    }
+
+    if (!is_array($value)) {
+      return array();
+    }
+
+    return $datasource->getWireTokens($value);
+  }
+
 
 }

src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

@@ -204,6 +204,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
     if ($edit_types) {
 
       $action_map = array();
+      $type_map = array();
       foreach ($edit_types as $edit_type) {
         $key = $edit_type->getEditType();
         $action_map[$key] = array(
@@ -212,6 +213,8 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
           'type' => $edit_type->getPHUIXControlType(),
           'spec' => $edit_type->getPHUIXControlSpecification(),
         );
+
+        $type_map[$key] = $edit_type;
       }
 
       $options = array();
@@ -248,6 +251,28 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
             'id' => $place_id,
           )));
 
+      $draft_actions = array();
+      if ($versioned_draft) {
+        $draft_actions = $versioned_draft->getProperty('actions', array());
+        foreach ($draft_actions as $key => $action) {
+          $type = idx($action, 'type');
+          if (!$type) {
+            unset($draft_actions[$key]);
+            continue;
+          }
+
+          $edit_type = idx($type_map, $type);
+          if (!$edit_type) {
+            unset($draft_actions[$key]);
+            continue;
+          }
+
+          $value = idx($action, 'value');
+          $value = $edit_type->getCommentActionValueFromDraftValue($value);
+          $draft_actions[$key]['value'] = $value;
+        }
+      }
+
       Javelin::initBehavior(
         'comment-actions',
         array(
@@ -260,6 +285,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
           'actions' => $action_map,
           'showPreview' => $this->getShowPreview(),
           'actionURI' => $this->getAction(),
+          'drafts' => $draft_actions,
         ));
     }
 

src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php

@@ -469,14 +469,14 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject {
     $tokens = array();
     foreach ($rendered as $key => $render) {
       $tokens[$key] = id(new PhabricatorTypeaheadResult())
-        ->setPHID($key)
+        ->setPHID($render->getKey())
         ->setIcon($render->getIcon())
         ->setColor($render->getColor())
         ->setDisplayName($render->getValue())
         ->setTokenType($render->getTokenType());
     }
 
-    return mpull($tokens, 'getWireFormat');
+    return mpull($tokens, 'getWireFormat', 'getPHID');
   }
 
 }

webroot/rsrc/js/application/transactions/behavior-comment-actions.js

@@ -19,20 +19,28 @@ JX.behavior('comment-actions', function(config) {
   var rows = {};
 
   JX.DOM.listen(action_node, 'change', null, function() {
+    var option = find_option(action_node.value);
+
+    action_node.value = '+';
+
+    if (option) {
+      add_row(option);
+    }
+  });
+
+  function find_option(key) {
     var options = action_node.options;
     var option;
 
-    var selected = action_node.value;
-    action_node.value = '+';
-
     for (var ii = 0; ii < options.length; ii++) {
       option = options[ii];
-      if (option.value == selected) {
+      if (option.value == key) {
-        add_row(option);
+        return option;
-        break;
       }
     }
-  });
+
+    return null;
+  }
 
   function add_row(option) {
     var action = action_map[option.value];
@@ -62,6 +70,8 @@ JX.behavior('comment-actions', function(config) {
     });
 
     place_node.parentNode.insertBefore(node, place_node);
+
+    return control;
   }
 
   function serialize_actions() {
@@ -86,6 +96,24 @@ JX.behavior('comment-actions', function(config) {
     return data;
   }
 
+  function restore_draft_actions(drafts) {
+    var draft;
+    var option;
+    var control;
+
+    for (var ii = 0; ii < drafts.length; ii++) {
+      draft = drafts[ii];
+
+      option = find_option(draft.type);
+      if (!option) {
+        continue;
+      }
+
+      control = add_row(option);
+      control.setValue(draft.value);
+    }
+  }
+
   function onresponse(response) {
     var panel = JX.$(config.panelID);
     if (!response.xactions.length) {
@@ -125,4 +153,6 @@ JX.behavior('comment-actions', function(config) {
     request.start();
   }
 
+  restore_draft_actions(config.drafts || []);
+
 });

webroot/rsrc/js/core/Prefab.js

@@ -19,15 +19,27 @@
 JX.install('Prefab', {
 
   statics : {
-    renderSelect : function(map, selected, attrs) {
+    renderSelect : function(map, selected, attrs, order) {
       var select = JX.$N('select', attrs || {});
-      for (var k in map) {
+
+      // Callers may optionally pass "order" to force options into a specific
+      // order. Although most browsers do retain order, maps in Javascript
+      // aren't technically ordered. Safari, at least, will reorder maps with
+      // numeric keys.
+
+      order = order || JX.keys(map);
+
+      var k;
+      for (var ii = 0; ii < order.length; ii++) {
+        k = order[ii];
         select.options[select.options.length] = new Option(map[k], k);
         if (k == selected) {
           select.value = k;
         }
       }
-      select.value = select.value || JX.keys(map)[0];
+
+      select.value = select.value || order[0];
+
       return select;
     },
 

webroot/rsrc/js/phuix/PHUIXFormControl.js

@@ -118,7 +118,15 @@ JX.install('PHUIXFormControl', {
         spec.config);
       build.tokenizer.start();
 
+      function get_value() {
+        return JX.keys(build.tokenizer.getTokens());
+      }
+
       function set_value(map) {
+        var tokens = get_value();
+        for (var ii = 0; ii < tokens.length; ii++) {
+          build.tokenizer.removeToken(tokens[ii]);
+        }
         for (var k in map) {
           var v = JX.Prefab.transformDatasourceResults(map[k]);
           build.tokenizer.addToken(k, v);
@@ -129,15 +137,17 @@ JX.install('PHUIXFormControl', {
 
       return {
         node: build.node,
-        get: function() {
+        get: get_value,
-          return JX.keys(build.tokenizer.getTokens());
-        },
         set: set_value
       };
     },
 
     _newSelect: function(spec) {
-      var node = JX.Prefab.renderSelect(spec.options, spec.value);
+      var node = JX.Prefab.renderSelect(
+        spec.options,
+        spec.value,
+        {},
+        spec.order);
 
       return {
         node: node,