Add a one-click "Scuttle Task" button to Maniphest

Summary: Fixes T4657. See that task for discussion of edge cases.

Test Plan: {F132941}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: chad, carl, epriestley

Maniphest Tasks: T4657

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

resources/celerity/map.php

@@ -7,8 +7,8 @@
 return array(
   'names' =>
   array(
-    'core.pkg.css' => '5e574aa1',
-    'core.pkg.js' => '264721e1',
+    'core.pkg.css' => '6d16f22a',
+    'core.pkg.js' => 'd3fecc57',
     'darkconsole.pkg.js' => 'ca8671ce',
     'differential.pkg.css' => 'cb97e095',
     'differential.pkg.js' => '11a5b750',
@@ -132,7 +132,7 @@ return array(
     'rsrc/css/phui/phui-document.css' => '10f59385',
     'rsrc/css/phui/phui-feed-story.css' => '3a59c2cf',
     'rsrc/css/phui/phui-fontkit.css' => 'de84aa4a',
-    'rsrc/css/phui/phui-form-view.css' => '0efd3326',
+    'rsrc/css/phui/phui-form-view.css' => '867463b4',
     'rsrc/css/phui/phui-form.css' => 'b78ec020',
     'rsrc/css/phui/phui-header-view.css' => '5b79f0ef',
     'rsrc/css/phui/phui-icon.css' => '7a5771a9',
@@ -475,7 +475,7 @@ return array(
     'rsrc/js/core/behavior-tokenizer.js' => 'b3a4b884',
     'rsrc/js/core/behavior-tooltip.js' => '48db4145',
     'rsrc/js/core/behavior-watch-anchor.js' => '06e05112',
-    'rsrc/js/core/behavior-workflow.js' => '82947dda',
+    'rsrc/js/core/behavior-workflow.js' => 'fee00761',
     'rsrc/js/core/phtize.js' => 'd254d646',
     'rsrc/js/phui/behavior-phui-object-box-tabs.js' => 'a3e2244e',
     'rsrc/swf/aphlict.swf' => 'abac967d',
@@ -627,7 +627,7 @@ return array(
     'javelin-behavior-test-payment-form' => 'b3e5ee60',
     'javelin-behavior-toggle-class' => 'a82a7769',
     'javelin-behavior-view-placeholder' => '2fa810fc',
-    'javelin-behavior-workflow' => '82947dda',
+    'javelin-behavior-workflow' => 'fee00761',
     'javelin-color' => '7e41274a',
     'javelin-cookie' => '6b3dcf44',
     'javelin-dom' => '5054855f',
@@ -748,7 +748,7 @@ return array(
     'phui-feed-story-css' => '3a59c2cf',
     'phui-fontkit-css' => 'de84aa4a',
     'phui-form-css' => 'b78ec020',
-    'phui-form-view-css' => '0efd3326',
+    'phui-form-view-css' => '867463b4',
     'phui-header-view-css' => '5b79f0ef',
     'phui-icon-view-css' => '7a5771a9',
     'phui-info-panel-css' => '27ea50a1',
@@ -1330,13 +1330,6 @@ return array(
       0 => 'javelin-behavior',
       1 => 'javelin-history',
     ),
-    '82947dda' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-stratcom',
-      2 => 'javelin-workflow',
-      3 => 'javelin-dom',
-    ),
     '82f568cd' =>
     array(
       0 => 'javelin-install',
@@ -1972,6 +1965,13 @@ return array(
       4 => 'multirow-row-manager',
       5 => 'javelin-json',
     ),
+    'fee00761' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-stratcom',
+      2 => 'javelin-workflow',
+      3 => 'javelin-dom',
+    ),
     28497740 =>
     array(
       0 => 'javelin-behavior',

src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php

@@ -149,8 +149,9 @@ The keys you can provide in a specification are:
     - `default` This is the default status for newly created tasks. You must
       designate one status as default, and it must be an open status.
     - `closed` This is the default status for closed tasks (for example, tasks
-      closed via the "!close" action in email). You must designate one status
-      as the default closed status, and it must be a closed status.
+      closed via the "!close" action in email or via the quick close button in
+      Maniphest). You must designate one status as the default closed status,
+      and it must be a closed status.
     - `duplicate` This is the status used when tasks are merged into one
       another as duplicates. You must designate one status for duplicates,
       and it must be a closed status.

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -208,6 +208,29 @@ final class ManiphestTaskDetailController extends ManiphestController {
 
     $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
 
+    $submit_text = $is_serious
+      ? pht('Submit')
+      : pht('Avast!');
+
+    $close_text = $is_serious
+      ? pht('Close Task')
+      : pht('Scuttle Task');
+
+    $submit_control = id(new PHUIFormMultiSubmitControl());
+    if (!$task->isClosed()) {
+      $close_image = id(new PHUIIconView())
+          ->setSpriteSheet(PHUIIconView::SPRITE_ICONS)
+          ->setSpriteIcon('check');
+      $submit_control->addButtonView(
+        id(new PHUIButtonView())
+          ->setColor(PHUIButtonView::GREY)
+          ->setIcon($close_image)
+          ->setText($close_text)
+          ->setName('scuttle')
+          ->addSigil('alternate-submit-button'));
+    }
+    $submit_control->addSubmitButton($submit_text);
+
     $comment_form = new AphrontFormView();
     $comment_form
       ->setUser($user)
@@ -273,9 +296,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
           ->setValue($draft_text)
           ->setID('transaction-comments')
           ->setUser($user))
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-          ->setValue($is_serious ? pht('Submit') : pht('Avast!')));
+      ->appendChild($submit_control);
 
     $control_map = array(
       ManiphestTransaction::TYPE_STATUS   => 'resolution',

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

@@ -130,6 +130,18 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       $transactions[] = $transaction;
     }
 
+    $resolution = $request->getStr('resolution');
+    $did_scuttle = false;
+    if ($action !== ManiphestTransaction::TYPE_STATUS) {
+      if ($request->getStr('scuttle')) {
+        $transactions[] = id(new ManiphestTransaction())
+          ->setTransactionType(ManiphestTransaction::TYPE_STATUS)
+          ->setNewValue(ManiphestTaskStatus::getDefaultClosedStatus());
+        $did_scuttle = true;
+        $resolution = ManiphestTaskStatus::getDefaultClosedStatus();
+      }
+    }
+
     // When you interact with a task, we add you to the CC list so you get
     // further updates, and possibly assign the task to you if you took an
     // ownership action (closing it) but it's currently unowned. We also move
@@ -137,31 +149,28 @@ final class ManiphestTransactionSaveController extends ManiphestController {
     // and create side-effect transactions for them.
 
     $implicitly_claimed = false;
-    switch ($action) {
-      case ManiphestTransaction::TYPE_OWNER:
-        if ($task->getOwnerPHID() == $transaction->getNewValue()) {
-          // If this is actually no-op, don't generate the side effect.
-          break;
-        }
-        // Otherwise, when a task is reassigned, move the previous owner to CC.
-        $added_ccs[] = $task->getOwnerPHID();
-        break;
-      case ManiphestTransaction::TYPE_STATUS:
-        $resolution = $request->getStr('resolution');
-        if (!$task->getOwnerPHID() &&
-            ManiphestTaskStatus::isClosedStatus($resolution)) {
-          // Closing an unassigned task. Assign the user as the owner of
-          // this task.
-          $assign = new ManiphestTransaction();
-          $assign->setTransactionType(ManiphestTransaction::TYPE_OWNER);
-          $assign->setNewValue($user->getPHID());
-          $transactions[] = $assign;
-
-          $implicitly_claimed = true;
-        }
+    if ($action == ManiphestTransaction::TYPE_OWNER) {
+      if ($task->getOwnerPHID() == $transaction->getNewValue()) {
+        // If this is actually no-op, don't generate the side effect.
         break;
+      }
+      // Otherwise, when a task is reassigned, move the previous owner to CC.
+      $added_ccs[] = $task->getOwnerPHID();
     }
 
+    if ($did_scuttle || ($action == ManiphestTransaction::TYPE_STATUS)) {
+      if (!$task->getOwnerPHID() &&
+          ManiphestTaskStatus::isClosedStatus($resolution)) {
+        // Closing an unassigned task. Assign the user as the owner of
+        // this task.
+        $assign = new ManiphestTransaction();
+        $assign->setTransactionType(ManiphestTransaction::TYPE_OWNER);
+        $assign->setNewValue($user->getPHID());
+        $transactions[] = $assign;
+
+        $implicitly_claimed = true;
+      }
+    }
 
     $user_owns_task = false;
     if ($implicitly_claimed) {

src/view/form/control/AphrontFormSubmitControl.php

@@ -2,7 +2,7 @@
 
 final class AphrontFormSubmitControl extends AphrontFormControl {
 
-  protected $cancelButton;
+  private $cancelButton;
 
   public function addCancelButton($href, $label = null) {
     if (!$label) {
@@ -35,7 +35,11 @@ final class AphrontFormSubmitControl extends AphrontFormControl {
         ),
         $this->getValue());
     }
-    return hsprintf('%s%s', $submit_button, $this->cancelButton);
+
+    return array(
+      $submit_button,
+      $this->cancelButton,
+    );
   }
 
 }

src/view/form/control/PHUIFormMultiSubmitControl.php

@@ -31,14 +31,20 @@ final class PHUIFormMultiSubmitControl extends AphrontFormControl {
     return $this;
   }
 
+  public function addButtonView(PHUIButtonView $button) {
+    $this->buttons[] = $button;
+    return $this;
+  }
+
   public function addButton($name, $label, $class = null) {
-    $this->buttons[] = phutil_tag(
+    $this->buttons[] = javelin_tag(
       'input',
       array(
         'type'  => 'submit',
         'name'  => $name,
         'value' => $label,
         'class' => $class,
+        'sigil' => 'alternate-submit-button',
         'disabled' => $this->getDisabled() ? 'disabled' : null,
       ));
     return $this;

src/view/phui/PHUIButtonView.php

@@ -21,6 +21,16 @@ final class PHUIButtonView extends AphrontTagView {
   private $href = null;
   private $title = null;
   private $disabled;
+  private $name;
+
+  public function setName($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function getName() {
+    return $this->name;
+  }
 
   public function setText($text) {
     $this->text = $text;
@@ -103,9 +113,12 @@ final class PHUIButtonView extends AphrontTagView {
       $classes[] = 'disabled';
     }
 
-    return array('class'  => $classes,
-                 'href'   => $this->href,
-                 'title'  => $this->title);
+    return array(
+      'class'  => $classes,
+      'href'   => $this->href,
+      'name'   => $this->name,
+      'title'  => $this->title,
+    );
   }
 
   protected function getTagContent() {

webroot/rsrc/css/phui/phui-form-view.css

@@ -108,9 +108,10 @@
 }
 
 .phui-form-control-multi-submit input,
+.phui-form-control-multi-submit button,
 .phui-form-control-multi-submit a {
   float: right;
-  margin: 0.5em 0 0em 2%;
+  margin: 4px 0 0 8px;
   width: auto;
 }
 

webroot/rsrc/js/core/behavior-workflow.js

@@ -8,6 +8,22 @@
 
 JX.behavior('workflow', function() {
 
+  // If a user clicks an alternate submit button, make sure it gets marshalled
+  // into the workflow.
+  JX.Stratcom.listen(
+    'click',
+    ['workflow', 'tag:form', 'alternate-submit-button'],
+    function(e) {
+      e.prevent();
+
+      var target = e.getNode('alternate-submit-button');
+      var form = e.getNode('tag:form');
+      var button = {};
+      button[target.name] = target.value || true;
+
+      JX.DOM.invoke(form, 'didSyntheticSubmit', {extra: button});
+    });
+
   // Listen for both real and synthetic submit events.
   JX.Stratcom.listen(
     ['submit', 'didSyntheticSubmit'],
@@ -17,11 +33,14 @@ JX.behavior('workflow', function() {
         return;
       }
 
+      var data = e.getData();
+      var extra = (data && data.extra) || {};
+
       // NOTE: We activate workflow if any parent node has the "workflow" sigil,
       // not just the <form /> itself.
 
       e.prevent();
-      JX.Workflow.newFromForm(e.getNode('tag:form')).start();
+      JX.Workflow.newFromForm(e.getNode('tag:form'), extra).start();
     });
 
   JX.Stratcom.listen(