Add comment previews to Maniphest

Summary:
Moves shared code from Differential and Maniphest comment previews into
PhabricatorShapedRequest, and then implements Maniphest previews.

This doesn't implement comment drafts, I'll follow up with that but it requires
this and is completely separable.

This also always shows the preview as "commented" rather than previewing the
actual transaction. I'll follow up with that but I think it will require a
little factoring and this is useful even without transaction details.

I need to tweak the styling a bit too.

Test Plan:
Typed text in Maniphest and Differential. Toggled Differential action. Made
comments.

Reviewed By: rm
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, rm
Differential Revision: 258

src/__celerity_resource_map__.php

@@ -61,6 +61,16 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/css/aphront/headsup-action-list-view.css',
   ),
+  0 =>
+  array(
+    'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-install',
+    ),
+    'disk' => '/rsrc/js/javelin/docs/Base.js',
+  ),
   'aphront-list-filter-view-css' =>
   array(
     'uri' => '/res/e6cff171/rsrc/css/aphront/list-filter-view.css',
@@ -284,16 +294,6 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/js/application/core/behavior-tokenizer.js',
   ),
-  0 =>
-  array(
-    'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
-    'type' => 'js',
-    'requires' =>
-    array(
-      0 => 'javelin-install',
-    ),
-    'disk' => '/rsrc/js/javelin/docs/Base.js',
-  ),
   'javelin-behavior-dark-console' =>
   array(
     'uri' => '/res/044c171f/rsrc/js/application/core/behavior-dark-console.js',
@@ -350,7 +350,7 @@ celerity_register_resource_map(array(
   ),
   'javelin-behavior-differential-feedback-preview' =>
   array(
-    'uri' => '/res/79e7f18d/rsrc/js/application/differential/behavior-comment-preview.js',
+    'uri' => '/res/ab8a7d60/rsrc/js/application/differential/behavior-comment-preview.js',
     'type' => 'js',
     'requires' =>
     array(
@@ -359,6 +359,7 @@ celerity_register_resource_map(array(
       2 => 'javelin-dom',
       3 => 'javelin-request',
       4 => 'javelin-util',
+      5 => 'phabricator-shaped-request',
     ),
     'disk' => '/rsrc/js/application/differential/behavior-comment-preview.js',
   ),
@@ -462,6 +463,19 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/js/application/maniphest/behavior-transaction-controls.js',
   ),
+  'javelin-behavior-maniphest-transaction-preview' =>
+  array(
+    'uri' => '/res/b211faf2/rsrc/js/application/maniphest/behavior-transaction-preview.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-dom',
+      2 => 'javelin-util',
+      3 => 'phabricator-shaped-request',
+    ),
+    'disk' => '/rsrc/js/application/maniphest/behavior-transaction-preview.js',
+  ),
   'javelin-behavior-owners-path-editor' =>
   array(
     'uri' => '/res/9cf78ffc/rsrc/js/application/owners/owners-path-editor.js',
@@ -720,7 +734,7 @@ celerity_register_resource_map(array(
   ),
   'mainphest-task-detail-css' =>
   array(
-    'uri' => '/res/dbefc148/rsrc/css/application/maniphest/task-detail.css',
+    'uri' => '/res/fa248d0b/rsrc/css/application/maniphest/task-detail.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -851,6 +865,18 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/css/core/remarkup.css',
   ),
+  'phabricator-shaped-request' =>
+  array(
+    'uri' => '/res/1f0ef02b/rsrc/js/application/core/ShapedRequest.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-install',
+      1 => 'javelin-util',
+      2 => 'javelin-request',
+    ),
+    'disk' => '/rsrc/js/application/core/ShapedRequest.js',
+  ),
   'phabricator-standard-page-view' =>
   array(
     'uri' => '/res/9c468a70/rsrc/css/application/base/standard-page-view.css',
@@ -936,20 +962,6 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/33f413ef/typeahead.pkg.js',
       'type' => 'js',
     ),
-    '711616db' =>
-    array (
-      'name' => 'differential.pkg.js',
-      'symbols' =>
-      array (
-        0 => 'javelin-behavior-differential-feedback-preview',
-        1 => 'javelin-behavior-differential-edit-inline-comments',
-        2 => 'javelin-behavior-differential-populate',
-        3 => 'javelin-behavior-differential-show-more',
-        4 => 'javelin-behavior-differential-diff-radios',
-      ),
-      'uri' => '/res/pkg/711616db/differential.pkg.js',
-      'type' => 'js',
-    ),
     '7d23deb1' =>
     array (
       'name' => 'javelin.pkg.js',
@@ -993,6 +1005,20 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/ac70e6b7/core.pkg.css',
       'type' => 'css',
     ),
+    'b17926e1' =>
+    array (
+      'name' => 'differential.pkg.js',
+      'symbols' =>
+      array (
+        0 => 'javelin-behavior-differential-feedback-preview',
+        1 => 'javelin-behavior-differential-edit-inline-comments',
+        2 => 'javelin-behavior-differential-populate',
+        3 => 'javelin-behavior-differential-show-more',
+        4 => 'javelin-behavior-differential-diff-radios',
+      ),
+      'uri' => '/res/pkg/b17926e1/differential.pkg.js',
+      'type' => 'js',
+    ),
   ),
   'reverse' =>
   array (
@@ -1016,11 +1042,11 @@ celerity_register_resource_map(array(
     'diffusion-commit-view-css' => '03ef179e',
     'javelin-behavior' => '7d23deb1',
     'javelin-behavior-aphront-basic-tokenizer' => '33f413ef',
-    'javelin-behavior-differential-diff-radios' => '711616db',
+    'javelin-behavior-differential-diff-radios' => 'b17926e1',
-    'javelin-behavior-differential-edit-inline-comments' => '711616db',
+    'javelin-behavior-differential-edit-inline-comments' => 'b17926e1',
-    'javelin-behavior-differential-feedback-preview' => '711616db',
+    'javelin-behavior-differential-feedback-preview' => 'b17926e1',
-    'javelin-behavior-differential-populate' => '711616db',
+    'javelin-behavior-differential-populate' => 'b17926e1',
-    'javelin-behavior-differential-show-more' => '711616db',
+    'javelin-behavior-differential-show-more' => 'b17926e1',
     'javelin-behavior-workflow' => '122a6b6d',
     'javelin-dom' => '7d23deb1',
     'javelin-event' => '7d23deb1',

src/__phutil_library_map__.php

@@ -261,6 +261,7 @@ phutil_register_library_map(array(
     'ManiphestTransactionDetailView' => 'applications/maniphest/view/transactiondetail',
     'ManiphestTransactionEditor' => 'applications/maniphest/editor/transaction',
     'ManiphestTransactionListView' => 'applications/maniphest/view/transactionlist',
+    'ManiphestTransactionPreviewController' => 'applications/maniphest/controller/transactionpreview',
     'ManiphestTransactionSaveController' => 'applications/maniphest/controller/transactionsave',
     'ManiphestTransactionType' => 'applications/maniphest/constants/transactiontype',
     'Phabricator404Controller' => 'applications/base/controller/404',
@@ -684,6 +685,7 @@ phutil_register_library_map(array(
     'ManiphestTransaction' => 'ManiphestDAO',
     'ManiphestTransactionDetailView' => 'AphrontView',
     'ManiphestTransactionListView' => 'AphrontView',
+    'ManiphestTransactionPreviewController' => 'ManiphestController',
     'ManiphestTransactionSaveController' => 'ManiphestController',
     'Phabricator404Controller' => 'PhabricatorController',
     'PhabricatorAuthController' => 'PhabricatorController',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -159,6 +159,7 @@ class AphrontDefaultApplicationConfiguration
         ),
         'transaction/' => array(
           'save/' => 'ManiphestTransactionSaveController',
+          'preview/(?P<id>\d+)/$' => 'ManiphestTransactionPreviewController',
         ),
         'select/search/$' => 'ManiphestTaskSelectorSearchController',
       ),

src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php

@@ -268,7 +268,7 @@ class ManiphestTaskDetailController extends ManiphestController {
         id(new AphrontFormTextAreaControl())
           ->setLabel('Comments')
           ->setName('comments')
-          ->setValue(''))
+          ->setID('transaction-comments'))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue('Avast!'));
@@ -301,10 +301,27 @@ class ManiphestTaskDetailController extends ManiphestController {
       ),
     ));
 
+
+    Javelin::initBehavior('maniphest-transaction-preview', array(
+      'uri'     => '/maniphest/transaction/preview/'.$task->getID().'/',
+      'preview' => 'transaction-preview',
+      'comments' => 'transaction-comments',
+    ));
+
+
     $comment_panel = new AphrontPanelView();
     $comment_panel->appendChild($comment_form);
     $comment_panel->setHeader('Leap Into Action');
 
+    $preview_panel =
+      '<div class="maniphest-transaction-preview">
+        <div id="transaction-preview">
+          <div class="maniphest-loading-text">
+            Loading preview...
+          </div>
+        </div>
+      </div>';
+
     $transaction_view = new ManiphestTransactionListView();
     $transaction_view->setTransactions($transactions);
     $transaction_view->setHandles($handles);
@@ -316,6 +333,7 @@ class ManiphestTaskDetailController extends ManiphestController {
         $panel,
         $transaction_view,
         $comment_panel,
+        $preview_panel,
       ),
       array(
         'title' => 'T'.$task->getID().' '.$task->getTitle(),

src/applications/maniphest/controller/transactionpreview/ManiphestTransactionPreviewController.php

@@ -0,0 +1,59 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class ManiphestTransactionPreviewController extends ManiphestController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+  }
+
+  public function processRequest() {
+
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $comments = $request->getStr('comments');
+
+    $handles = id(new PhabricatorObjectHandleData(array($user->getPHID())))
+      ->loadHandles();
+
+    $transaction = new ManiphestTransaction();
+    $transaction->setAuthorPHID($user->getPHID());
+    $transaction->setComments($comments);
+    $transaction->setTransactionType(ManiphestTransactionType::TYPE_NONE);
+
+    $transactions = array();
+    $transactions[] = $transaction;
+
+    $factory = new DifferentialMarkupEngineFactory();
+    $engine = $factory->newDifferentialCommentMarkupEngine();
+
+    $transaction_view = new ManiphestTransactionListView();
+    $transaction_view->setTransactions($transactions);
+    $transaction_view->setHandles($handles);
+    $transaction_view->setUser($user);
+    $transaction_view->setMarkupEngine($engine);
+    $transaction_view->setPreview(true);
+
+    return id(new AphrontAjaxResponse())
+      ->setContent($transaction_view->render());
+  }
+
+}

src/applications/maniphest/controller/transactionpreview/__init__.php

@@ -0,0 +1,20 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/ajax');
+phutil_require_module('phabricator', 'applications/differential/parser/markup');
+phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
+phutil_require_module('phabricator', 'applications/maniphest/controller/base');
+phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
+phutil_require_module('phabricator', 'applications/maniphest/view/transactionlist');
+phutil_require_module('phabricator', 'applications/phid/handle/data');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('ManiphestTransactionPreviewController.php');

src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php

@@ -22,6 +22,7 @@ class ManiphestTransactionDetailView extends AphrontView {
   private $handles;
   private $markupEngine;
   private $forEmail;
+  private $preview;
 
   public function setTransactionGroup(array $transactions) {
     $this->transactions = $transactions;
@@ -38,6 +39,11 @@ class ManiphestTransactionDetailView extends AphrontView {
     return $this;
   }
 
+  public function setPreview($preview) {
+    $this->preview = $preview;
+    return $this;
+  }
+
   public function renderForEmail($with_date) {
     $this->forEmail = true;
 
@@ -107,7 +113,7 @@ class ManiphestTransactionDetailView extends AphrontView {
         if (strlen($comments)) {
           $comments = $this->markupEngine->markupText($comments);
           $comment_transaction->setCache($comments);
-          if ($comment_transaction->getID()) {
+          if ($comment_transaction->getID() && !$this->preview) {
             $comment_transaction->save();
           }
         }
@@ -120,6 +126,12 @@ class ManiphestTransactionDetailView extends AphrontView {
       $comment_block = null;
     }
 
+    if ($this->preview) {
+      $timestamp = 'COMMENT PREVIEW';
+    } else {
+      $timestamp = phabricator_format_timestamp($transaction->getDateCreated());
+    }
+
     return phutil_render_tag(
       'div',
       array(
@@ -129,7 +141,7 @@ class ManiphestTransactionDetailView extends AphrontView {
       '<div class="maniphest-transaction-detail-view '.$more_classes.'">'.
         '<div class="maniphest-transaction-header">'.
           '<div class="maniphest-transaction-timestamp">'.
-            phabricator_format_timestamp($transaction->getDateCreated()).
+            $timestamp.
           '</div>'.
           $descs.
         '</div>'.

src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php

@@ -22,6 +22,7 @@ class ManiphestTransactionListView extends AphrontView {
   private $handles;
   private $user;
   private $markupEngine;
+  private $preview;
 
   public function setTransactions(array $transactions) {
     $this->transactions = $transactions;
@@ -43,6 +44,11 @@ class ManiphestTransactionListView extends AphrontView {
     return $this;
   }
 
+  public function setPreview($preview) {
+    $this->preview = $preview;
+    return $this;
+  }
+
   public function render() {
 
     $views = array();
@@ -76,6 +82,7 @@ class ManiphestTransactionListView extends AphrontView {
       $view->setTransactionGroup($group);
       $view->setHandles($this->handles);
       $view->setMarkupEngine($this->markupEngine);
+      $view->setPreview($this->preview);
       $views[] = $view->render();
     }
 

webroot/rsrc/css/application/maniphest/task-detail.css

@@ -42,3 +42,15 @@
 .maniphest-task-detail-core {
   margin-right: 265px;
 }
+
+.maniphest-transaction-preview {
+  background: #f0f0f0;
+  border-top: 1px solid #dddddd;
+  border-bottom: 1px solid #aaaaaa;
+  margin: -1em 2em 2em;
+  padding: 15px 20px;
+}
+
+.maniphest-loading-text {
+  color: #666666;
+}

webroot/rsrc/js/application/core/ShapedRequest.js

@@ -0,0 +1,88 @@
+/**
+ * @requires javelin-install
+ *           javelin-util
+ *           javelin-request
+ * @provides phabricator-shaped-request
+ * @javelin
+ */
+
+/**
+ * Send requests with rate limiting and retries, in response to some application
+ * trigger. This is used to implement comment previews in Differential and
+ * Maniphest.
+ */
+JX.install('PhabricatorShapedRequest', {
+
+  construct : function(uri, callback, data_callback) {
+    this._uri = uri;
+    this._callback = callback;
+    this._dataCallback = data_callback;
+  },
+
+  members : {
+    _callback : null,
+    _dataCallback : null,
+    _request : null,
+    _min : null,
+    _defer : null,
+    _last : null,
+    start : function() {
+      this.trigger();
+    },
+
+    trigger : function() {
+
+      if (this._request) {
+        // Waiting on a request, rate-limit.
+        return;
+      }
+
+      if (this._min && (new Date().getTime() < this._min)) {
+        // Just got a request back, rate-limit.
+        return;
+      }
+
+      this._defer && this._defer.stop();
+
+      var data = this._dataCallback();
+
+      if (this.shouldSendRequest(this._last, data)) {
+        this._last = data;
+        var request = new JX.Request(this._uri, JX.bind(this, function(r) {
+          this._callback(r);
+
+          this._min = new Date().getTime() + this._rateLimit;
+          this._defer && this._defer.stop();
+          this._defer = JX.defer(JX.bind(this, this.trigger), this._rateLimit);
+        }));
+        request.listen('finally', JX.bind(this, function() {
+          this._request = null;
+        }));
+        request.setData(data);
+        request.setTimeout(this.getFrequency());
+        request.send();
+      } else {
+        this._defer = JX.defer(JX.bind(this, this.trigger), this._frequency);
+      }
+    },
+
+    shouldSendRequest : function(last, data) {
+      if (last === null) {
+        return true;
+      }
+
+      for (var k in last) {
+        if (data[k] !== last[k]) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+  },
+
+  properties : {
+    rateLimit : 250,
+    frequency : 750
+  }
+});

webroot/rsrc/js/application/differential/behavior-comment-preview.js

@@ -5,52 +5,33 @@
  *           javelin-dom
  *           javelin-request
  *           javelin-util
+ *           phabricator-shaped-request
  */
 
 JX.behavior('differential-feedback-preview', function(config) {
 
   var action = JX.$(config.action);
   var content = JX.$(config.content);
-  var preview = JX.$(config.preview);
 
-  var aval = null;//action.value;
+  var callback = function(r) {
-  var cval = null;//content.value;
+    JX.DOM.setContent(JX.$(config.preview), JX.$H(r));
-  var defer = null;
+  };
-  var min = null;
-  var request = null;
 
-  function check() {
+  var getdata = function() {
-    if (request || (min && (new Date().getTime() < min))) {
+    return {
-      // Waiting on an async or just got one back, rate-limit.
+      content : content.value,
-      return;
+      action : action.value
-    }
+    };
+  };
 
-    defer && defer.stop();
+  var request = new JX.PhabricatorShapedRequest(config.uri, callback, getdata);
+  var trigger = JX.bind(request, request.trigger);
 
-    if (action.value !== aval || content.value !== cval) {
+  JX.DOM.listen(content, 'keydown', null, trigger);
-      aval = action.value;
+  JX.DOM.listen(action,  'change',  null, trigger);
-      cval = content.value;
 
-      request = new JX.Request(config.uri, function(r) {
+  request.start();
-        preview && JX.DOM.setContent(preview, JX.$H(r));
-        min = new Date().getTime() + 500;
-        defer && defer.stop();
-        defer = JX.defer(check, 500);
-      });
-      request.listen('finally', function() { request = null; });
-      request.setData({action : aval, content : cval});
-      // If we don't get a response back soon, retry on the next action.
-      request.setTimeout(2000);
-      request.send();
-    } else {
-      defer = JX.defer(check, 2000);
-    }
-  }
 
-  JX.DOM.listen(content, 'keydown', null, check);
-  JX.DOM.listen(action,  'change',  null, check);
-
-  check();
 
 
   function refreshInlinePreview() {

webroot/rsrc/js/application/maniphest/behavior-transaction-preview.js

@@ -0,0 +1,29 @@
+/**
+ * @provides javelin-behavior-maniphest-transaction-preview
+ * @requires javelin-behavior
+ *           javelin-dom
+ *           javelin-util
+ *           phabricator-shaped-request
+ */
+
+JX.behavior('maniphest-transaction-preview', function(config) {
+
+  var comments = JX.$(config.comments);
+
+  var callback = function(r) {
+    JX.DOM.setContent(JX.$(config.preview), JX.$H(r));
+  };
+
+  var getdata = function() {
+    return {
+      comments : comments.value
+    };
+  }
+
+  var request = new JX.PhabricatorShapedRequest(config.uri, callback, getdata);
+  var trigger = JX.bind(request, request.trigger);
+
+  JX.DOM.listen(comments, 'keydown', null, trigger);
+
+  request.start();
+});