Make some Differential comment actions (like "Accept" and "Reject") conflict with one another

Summary:
Ref T11114. When a user selects "Accept", and then selects "Reject", remove the "Accept". It does not make sense to both accept and reject a revision.

For now, every one of the "actions" conflicts: accept, reject, resign, claim, close, commandeer, etc, etc. I couldn't come up with any combinations that it seems like users are reasonably likely to want to try, and we haven't received combo-action requests in the past that I can recall.

Test Plan:
  - Selected "Accept", then selected "Reject". One replaced the other.
  - Selected "Accept", then selected "Change Subscribers". Both co-existed happily.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

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

resources/celerity/map.php

@@ -396,6 +396,7 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '019f36c4',
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375',
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
+    'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
     'rsrc/js/application/differential/ChangesetViewManager.js' => 'a2828756',
     'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '2e3f9738',
     'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18',
@@ -453,7 +454,7 @@ return array(
     'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072',
     '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' => 'c23ecb0b',
+    'rsrc/js/application/transactions/behavior-comment-actions.js' => '8fd8b2b1',
     'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243',
     'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
     'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '94c65b72',
@@ -609,7 +610,7 @@ return array(
     'javelin-behavior-bulk-job-reload' => 'edf8a145',
     'javelin-behavior-calendar-month-view' => 'fe33e256',
     'javelin-behavior-choose-control' => '327a00d1',
-    'javelin-behavior-comment-actions' => 'c23ecb0b',
+    'javelin-behavior-comment-actions' => '8fd8b2b1',
     'javelin-behavior-config-reorder-fields' => 'b6993408',
     'javelin-behavior-conpherence-menu' => '7524fcfa',
     'javelin-behavior-conpherence-participant-pane' => '8604caa8',
@@ -625,6 +626,7 @@ return array(
     'javelin-behavior-desktop-notifications-control' => 'edd1ba66',
     'javelin-behavior-detect-timezone' => '4c193c96',
     'javelin-behavior-device' => 'bb1dd507',
+    'javelin-behavior-diff-preview-link' => '051c7832',
     'javelin-behavior-differential-add-reviewers-and-ccs' => 'e10f8e18',
     'javelin-behavior-differential-comment-jump' => '4fdb476d',
     'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
@@ -944,6 +946,11 @@ return array(
       'javelin-dom',
       'phabricator-keyboard-shortcut',
     ),
+    '051c7832' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+    ),
     '05270951' => array(
       'javelin-util',
       'javelin-magical-init',
@@ -1640,6 +1647,15 @@ return array(
       'javelin-stratcom',
       'javelin-util',
     ),
+    '8fd8b2b1' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'javelin-dom',
+      'phuix-form-control-view',
+      'phuix-icon-view',
+      'javelin-behavior-phabricator-gesture',
+    ),
     '8ff5e24c' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1947,15 +1963,6 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
-    'c23ecb0b' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'javelin-dom',
-      'phuix-form-control-view',
-      'phuix-icon-view',
-      'javelin-behavior-phabricator-gesture',
-    ),
     'c587b80f' => array(
       'javelin-install',
     ),

src/applications/differential/xaction/DifferentialRevisionActionTransaction.php

@@ -73,6 +73,18 @@ abstract class DifferentialRevisionActionTransaction
 
         $group_key = $this->getRevisionActionGroupKey();
         $field->setCommentActionGroupKey($group_key);
+
+        // Currently, every revision action conflicts with every other
+        // revision action: for example, you can not simultaneously Accept and
+        // Reject a revision.
+
+        // Under some configurations, some combinations of actions are sort of
+        // technically permissible. For example, you could reasonably Reject
+        // and Abandon a revision if "anyone can abandon anything" is enabled.
+
+        // It's not clear that these combinations are actually useful, so just
+        // keep things simple for now.
+        $field->setActionConflictKey('revision.action');
       }
     }
 

src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php

@@ -8,6 +8,7 @@ abstract class PhabricatorEditEngineCommentAction extends Phobject {
   private $initialValue;
   private $order;
   private $groupKey;
+  private $conflictKey;
 
   abstract public function getPHUIXControlType();
   abstract public function getPHUIXControlSpecification();
@@ -30,6 +31,15 @@ abstract class PhabricatorEditEngineCommentAction extends Phobject {
     return $this->groupKey;
   }
 
+  public function setConflictKey($conflict_key) {
+    $this->conflictKey = $conflict_key;
+    return $this;
+  }
+
+  public function getConflictKey() {
+    return $this->conflictKey;
+  }
+
   public function setLabel($label) {
     $this->label = $label;
     return $this;

src/applications/transactions/editfield/PhabricatorApplyEditField.php

@@ -4,6 +4,7 @@ final class PhabricatorApplyEditField
   extends PhabricatorEditField {
 
   private $actionDescription;
+  private $actionConflictKey;
 
   protected function newControl() {
     return null;
@@ -18,6 +19,15 @@ final class PhabricatorApplyEditField
     return $this->actionDescription;
   }
 
+  public function setActionConflictKey($action_conflict_key) {
+    $this->actionConflictKey = $action_conflict_key;
+    return $this;
+  }
+
+  public function getActionConflictKey() {
+    return $this->actionConflictKey;
+  }
+
   protected function newHTTPParameterType() {
     return new AphrontBoolHTTPParameterType();
   }
@@ -34,7 +44,8 @@ final class PhabricatorApplyEditField
 
   protected function newCommentAction() {
     return id(new PhabricatorEditEngineStaticCommentAction())
-      ->setDescription($this->getActionDescription());
+      ->setDescription($this->getActionDescription())
+      ->setConflictKey($this->getActionConflictKey());
   }
 
 }

src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

@@ -301,6 +301,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
           'spec' => $comment_action->getPHUIXControlSpecification(),
           'initialValue' => $comment_action->getInitialValue(),
           'groupKey' => $comment_action->getGroupKey(),
+          'conflictKey' => $comment_action->getConflictKey(),
         );
 
         $type_map[$key] = $comment_action;

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

@@ -43,59 +43,13 @@ JX.behavior('comment-actions', function(config) {
     return null;
   }
 
-  function add_row(option) {
-    var action = action_map[option.value];
-    if (!action) {
-      return;
+  function remove_action(key) {
+    var row = rows[key];
+    if (row) {
+      JX.DOM.remove(row.node);
+      row.option.disabled = false;
+      delete rows[key];
     }
-
-    option.disabled = true;
-
-    var icon = new JX.PHUIXIconView()
-      .setIcon('fa-times-circle');
-    var remove = JX.$N('a', {href: '#'}, icon.getNode());
-
-    var control = new JX.PHUIXFormControl()
-      .setLabel(action.label)
-      .setError(remove)
-      .setControl(action.type, action.spec)
-      .setClass('phui-comment-action');
-    var node = control.getNode();
-
-    JX.Stratcom.addSigil(node, 'touchable');
-
-    var remove_action = function() {
-      JX.DOM.remove(node);
-      delete rows[action.key];
-      option.disabled = false;
-    };
-
-    JX.DOM.listen(node, 'gesture.swipe.end', null, function(e) {
-      var data = e.getData();
-
-      if (data.direction != 'left') {
-        // Didn't swipe left.
-        return;
-      }
-
-      if (data.length <= (JX.Vector.getDim(node).x / 2)) {
-        // Didn't swipe far enough.
-        return;
-      }
-
-      remove_action();
-    });
-
-    rows[action.key] = control;
-
-    JX.DOM.listen(remove, 'click', null, function(e) {
-      e.kill();
-      remove_action();
-    });
-
-    place_node.parentNode.insertBefore(node, place_node);
-
-    return control;
   }
 
   function serialize_actions() {
@@ -104,7 +58,7 @@ JX.behavior('comment-actions', function(config) {
     for (var k in rows) {
       data.push({
         type: k,
-        value: rows[k].getValue(),
+        value: rows[k].control.getValue(),
         initialValue: action_map[k].initialValue || null
       });
     }
@@ -171,6 +125,91 @@ JX.behavior('comment-actions', function(config) {
     }
   }
 
+  function force_preview() {
+    if (!config.shouldPreview) {
+      return;
+    }
+
+    new JX.Request(config.actionURI, onresponse)
+      .setData(get_data())
+      .send();
+  }
+
+  function add_row(option) {
+    var action = action_map[option.value];
+    if (!action) {
+      return;
+    }
+
+    // Remove any conflicting actions. For example, "Accept Revision" conflicts
+    // with "Reject Revision".
+    var conflict_key = action.conflictKey || null;
+    if (conflict_key !== null) {
+      for (var k in action_map) {
+        if (k === action) {
+          continue;
+        }
+        if (action_map[k].conflictKey !== conflict_key) {
+          continue;
+        }
+
+        if (!(k in rows)) {
+          continue;
+        }
+
+        remove_action(k);
+      }
+    }
+
+    option.disabled = true;
+
+    var icon = new JX.PHUIXIconView()
+      .setIcon('fa-times-circle');
+    var remove = JX.$N('a', {href: '#'}, icon.getNode());
+
+    var control = new JX.PHUIXFormControl()
+      .setLabel(action.label)
+      .setError(remove)
+      .setControl(action.type, action.spec)
+      .setClass('phui-comment-action');
+    var node = control.getNode();
+
+    JX.Stratcom.addSigil(node, 'touchable');
+
+    JX.DOM.listen(node, 'gesture.swipe.end', null, function(e) {
+      var data = e.getData();
+
+      if (data.direction != 'left') {
+        // Didn't swipe left.
+        return;
+      }
+
+      if (data.length <= (JX.Vector.getDim(node).x / 2)) {
+        // Didn't swipe far enough.
+        return;
+      }
+
+      remove_action(action);
+    });
+
+    rows[action.key] = {
+      control: control,
+      node: node,
+      option: option
+    };
+
+    JX.DOM.listen(remove, 'click', null, function(e) {
+      e.kill();
+      remove_action(action);
+    });
+
+    place_node.parentNode.insertBefore(node, place_node);
+
+    force_preview();
+
+    return control;
+  }
+
   JX.DOM.listen(form_node, ['submit', 'didSyntheticSubmit'], null, function() {
     input_node.value = serialize_actions();
   });
@@ -185,13 +224,7 @@ JX.behavior('comment-actions', function(config) {
 
     JX.DOM.listen(form_node, 'keydown', null, trigger);
 
-    var always_trigger = function() {
-      new JX.Request(config.actionURI, onresponse)
-        .setData(get_data())
-        .send();
-    };
-
-    JX.DOM.listen(form_node, 'shouldRefresh', null, always_trigger);
+    JX.DOM.listen(form_node, 'shouldRefresh', null, force_preview);
     request.start();
 
     var old_device = JX.Device.getDevice();
@@ -206,7 +239,7 @@ JX.behavior('comment-actions', function(config) {
         // Force an immediate refresh if we switched from another device type
         // to desktop.
         if (old_device != new_device) {
-          always_trigger();
+          force_preview();
         }
       } else {
         // On mobile, don't show live previews and only save drafts every