Remember action in Differential comment draft

Summary:
It happens to me quite often that I leave the window with revision (by closing it or by visiting a link from it).
When I return then the comment draft is there so I clowncopterize it but forget that I wanted to take some other action than Comment.

Test Plan: Selected "Add Reviewers", added some reviewers, closed the window, opened it - the action and reviewers were still there.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Korvin

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

resources/sql/patches/draft-metadata.sql

@@ -0,0 +1,4 @@
+ALTER TABLE `{$NAMESPACE}_draft`.`draft`
+ADD `metadata` longtext NOT NULL DEFAULT '' AFTER `draft`;
+
+UPDATE `{$NAMESPACE}_draft`.`draft` SET `metadata` = '[]';

src/applications/differential/constants/DifferentialAction.php

@@ -85,4 +85,12 @@ final class DifferentialAction {
     }
   }
 
+  public static function allowReviewers($action) {
+    if ($action == DifferentialAction::ACTION_ADDREVIEWERS ||
+        $action == DifferentialAction::ACTION_REQUEST) {
+      return true;
+    }
+    return false;
+  }
+
 }

src/applications/differential/controller/DifferentialCommentPreviewController.php

@@ -42,18 +42,15 @@ final class DifferentialCommentPreviewController
 
     $handles = array($author_phid);
 
-    $reviewers = $request->getStr('reviewers');
+    $reviewers = $request->getStrList('reviewers');
-    if (($action == DifferentialAction::ACTION_ADDREVIEWERS
+    if (DifferentialAction::allowReviewers($action) && $reviewers) {
-        || $action == DifferentialAction::ACTION_REQUEST) && $reviewers) {
-      $reviewers = explode(',', $reviewers);
       $comment->setMetadata(array(
         DifferentialComment::METADATA_ADDED_REVIEWERS => $reviewers));
       $handles = array_merge($handles, $reviewers);
     }
 
-    $ccs = $request->getStr('ccs');
+    $ccs = $request->getStrList('ccs');
     if ($action == DifferentialAction::ACTION_ADDCCS && $ccs) {
-      $ccs = explode(',', $ccs);
       $comment->setMetadata(array(
         DifferentialComment::METADATA_ADDED_CCS => $ccs));
       $handles = array_merge($handles, $ccs);
@@ -74,6 +71,11 @@ final class DifferentialCommentPreviewController
       ->setAuthorPHID($author_phid)
       ->setDraftKey('differential-comment-'.$this->id)
       ->setDraft($comment->getContent())
+      ->setMetadata(array(
+        'action' => $action,
+        'reviewers' => $reviewers,
+        'ccs' => $ccs,
+      ))
       ->replace();
 
     return id(new AphrontAjaxResponse())

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -353,10 +353,17 @@ final class DifferentialRevisionViewController extends DifferentialController {
         'authorPHID = %s AND draftKey = %s',
         $user->getPHID(),
         'differential-comment-'.$revision->getID());
+
+      $reviewers = array();
+      $ccs = array();
       if ($draft) {
-        $draft = $draft->getDraft();
+        $reviewers = idx($draft->getMetadata(), 'reviewers', array());
-      } else {
+        $ccs = idx($draft->getMetadata(), 'ccs', array());
-        $draft = null;
+        if ($reviewers || $ccs) {
+          $handles = $this->loadViewerHandles(array_merge($reviewers, $ccs));
+          $reviewers = array_select_keys($handles, $reviewers);
+          $ccs = array_select_keys($handles, $ccs);
+        }
       }
 
       $comment_form = new DifferentialAddCommentView();
@@ -366,6 +373,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $comment_form->setActionURI('/differential/comment/save/');
       $comment_form->setUser($user);
       $comment_form->setDraft($draft);
+      $comment_form->setReviewers(mpull($reviewers, 'getFullName', 'getPHID'));
+      $comment_form->setCCs(mpull($ccs, 'getFullName', 'getPHID'));
     }
 
     $pane_id = celerity_generate_unique_node_id();

src/applications/differential/view/DifferentialAddCommentView.php

@@ -24,6 +24,8 @@ final class DifferentialAddCommentView extends AphrontView {
   private $user;
   private $draft;
   private $auxFields;
+  private $reviewers = array();
+  private $ccs = array();
 
   public function setRevision($revision) {
     $this->revision = $revision;
@@ -51,11 +53,21 @@ final class DifferentialAddCommentView extends AphrontView {
     return $this;
   }
 
-  public function setDraft($draft) {
+  public function setDraft(PhabricatorDraft $draft = null) {
     $this->draft = $draft;
     return $this;
   }
 
+  public function setReviewers(array $names) {
+    $this->reviewers = $names;
+    return $this;
+  }
+
+  public function setCCs(array $names) {
+    $this->ccs = $names;
+    return $this;
+  }
+
   private function generateWarningView(
     $status,
     array $titles,
@@ -79,6 +91,14 @@ final class DifferentialAddCommentView extends AphrontView {
 
     $revision = $this->revision;
 
+    $action = null;
+    if ($this->draft) {
+      $action = idx($this->draft->getMetadata(), 'action');
+    }
+
+    $enable_reviewers = DifferentialAction::allowReviewers($action);
+    $enable_ccs = ($action == DifferentialAction::ACTION_ADDCCS);
+
     $form = new AphrontFormView();
     $form
       ->setWorkflow(true)
@@ -89,6 +109,7 @@ final class DifferentialAddCommentView extends AphrontView {
         id(new AphrontFormSelectControl())
           ->setLabel('Action')
           ->setName('action')
+          ->setValue($action)
           ->setID('comment-action')
           ->setOptions($this->actions))
       ->appendChild(
@@ -96,7 +117,7 @@ final class DifferentialAddCommentView extends AphrontView {
           ->setLabel('Add Reviewers')
           ->setName('reviewers')
           ->setControlID('add-reviewers')
-          ->setControlStyle('display: none')
+          ->setControlStyle($enable_reviewers ? null : 'display: none')
           ->setID('add-reviewers-tokenizer')
           ->setDisableBehavior(true))
       ->appendChild(
@@ -104,7 +125,7 @@ final class DifferentialAddCommentView extends AphrontView {
           ->setLabel('Add CCs')
           ->setName('ccs')
           ->setControlID('add-ccs')
-          ->setControlStyle('display: none')
+          ->setControlStyle($enable_ccs ? null : 'display: none')
           ->setID('add-ccs-tokenizer')
           ->setDisableBehavior(true))
       ->appendChild(
@@ -113,7 +134,7 @@ final class DifferentialAddCommentView extends AphrontView {
           ->setID('comment-content')
           ->setLabel('Comment')
           ->setEnableDragAndDropFileUploads(true)
-          ->setValue($this->draft))
+          ->setValue($this->draft->getDraft()))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue($is_serious ? 'Submit' : 'Clowncopterize'));
@@ -125,6 +146,7 @@ final class DifferentialAddCommentView extends AphrontView {
           'add-reviewers-tokenizer' => array(
             'actions' => array('request_review' => 1, 'add_reviewers' => 1),
             'src' => '/typeahead/common/users/',
+            'value' => $this->reviewers,
             'row' => 'add-reviewers',
             'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
             'placeholder' => 'Type a user name...',
@@ -132,6 +154,7 @@ final class DifferentialAddCommentView extends AphrontView {
           'add-ccs-tokenizer' => array(
             'actions' => array('add_ccs' => 1),
             'src' => '/typeahead/common/mailable/',
+            'value' => $this->ccs,
             'row' => 'add-ccs',
             'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
             'placeholder' => 'Type a user or mailing list...',

src/applications/draft/storage/PhabricatorDraft.php

@@ -21,5 +21,14 @@ final class PhabricatorDraft extends PhabricatorDraftDAO {
   protected $authorPHID;
   protected $draftKey;
   protected $draft;
+  protected $metadata = array();
+
+  public function getConfiguration() {
+    return array(
+      self::CONFIG_SERIALIZATION => array(
+        'metadata' => self::SERIALIZATION_JSON,
+      ),
+    ) + parent::getConfiguration();
+  }
 
 }

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -984,6 +984,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type'    => 'sql',
         'name'    => $this->getPatchPath('xhprof.sql'),
       ),
+      'draft-metadata.sql' => array(
+        'type'    => 'sql',
+        'name'    => $this->getPatchPath('draft-metadata.sql'),
+      ),
     );
   }