Edit/Delete for inline comments

src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php

@@ -71,7 +71,13 @@ class DifferentialChangesetViewController extends DifferentialController {
     $engine = $factory->newDifferentialCommentMarkupEngine();
     $parser->setMarkupEngine($engine);
 
-    $output = $parser->render(null, $range_s, $range_e, $mask);
+    if ($request->isAjax()) {
+      // TODO: This is sort of lazy, the effect is just to not render "Edit"
+      // links on the "standalone view".
+      $parser->setUser($request->getUser());
+    }
+
+    $output = $parser->render($range_s, $range_e, $mask);
 
     if ($request->isAjax()) {
       return id(new AphrontAjaxResponse())

src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php

@@ -34,40 +34,86 @@ class DifferentialInlineCommentEditController extends DifferentialController {
     $length = $request->getInt('length');
     $text = $request->getStr('text');
     $op = $request->getStr('op');
+    $inline_id = $request->getInt('id');
 
     $user = $request->getUser();
 
     $submit_uri = '/differential/comment/inline/edit/'.$this->revisionID.'/';
 
-    switch ($op) {
+    $edit_dialog = new AphrontDialogView();
-      case 'delete':
+    $edit_dialog->setUser($user);
-        if ($request->isFormPost()) {
+    $edit_dialog->setSubmitURI($submit_uri);
-          // do the delete;
+
-          return new AphrontAjaxResponse();
+    $edit_dialog->addHiddenInput('on_right', $on_right);
+
+    $edit_dialog->addSubmitButton();
+    $edit_dialog->addCancelButton('#');
+
+    $inline = null;
+    if ($inline_id) {
+      $inline = id(new DifferentialInlineComment())
+        ->load($inline_id);
+
+      if (!$inline ||
+          $inline->getAuthorPHID() != $user->getPHID() ||
+          $inline->getCommentID() ||
+          $inline->getRevisionID() != $this->revisionID) {
+        throw new Exception("That comment is not editable!");
+      }
     }
 
-        $dialog = new AphrontDialogView();
+    switch ($op) {
-        $dialog->setTitle('Really delete this comment?');
+      case 'delete':
+        if (!$inline) {
+          return new Aphront400Response();
+        }
 
-        return id(new AphrontDialogResponse())->setDialog($dialog);
+        if ($request->isFormPost()) {
+          $inline->delete();
+          return $this->buildDeletedResponse();
+        }
+
+        $edit_dialog->setTitle('Really delete this comment?');
+        $edit_dialog->addHiddenInput('id', $inline_id);
+        $edit_dialog->addHiddenInput('op', 'delete');
+        $edit_dialog->appendChild(
+          '<p>Delete this inline comment?</p>');
+
+        return id(new AphrontDialogResponse())->setDialog($edit_dialog);
       case 'edit':
-        $dialog = new AphrontDialogView();
+        if (!$inline) {
+          return new Aphront400Response();
+        }
 
-        return id(new AphrontDialogResponse())->setDialog($dialog);
+        if ($request->isFormPost()) {
+          if (strlen($text)) {
+            $inline->setContent($text);
+            $inline->save();
+            return $this->buildRenderedCommentResponse(
+              $inline,
+              $on_right);
+          } else {
+            $inline->delete();
+            return $this->buildDeletedResponse();
+          }
+        }
+
+        $edit_dialog->setTitle('Edit Inline Comment');
+
+        $edit_dialog->addHiddenInput('id', $inline_id);
+        $edit_dialog->addHiddenInput('op', 'edit');
+
+        $edit_dialog->appendChild(
+          $this->renderTextArea(
+            $inline->getContent()));
+
+        return id(new AphrontDialogResponse())->setDialog($edit_dialog);
       case 'create':
 
         if (!$request->isFormPost() || !strlen($text)) {
           return new AphrontAjaxResponse();
         }
 
-        $factory = new DifferentialMarkupEngineFactory();
-        $engine = $factory->newDifferentialCommentMarkupEngine();
-
-        $phids = array($user->getPHID());
-
-        $handles = id(new PhabricatorObjectHandleData($phids))
-          ->loadHandles();
-
         $inline = id(new DifferentialInlineComment())
           ->setRevisionID($this->revisionID)
           ->setChangesetID($changeset)
@@ -79,12 +125,44 @@ class DifferentialInlineCommentEditController extends DifferentialController {
           ->setContent($text)
           ->save();
 
+        return $this->buildRenderedCommentResponse($inline, $on_right);
+      default:
+        $edit_dialog->setTitle('New Inline Comment');
+
+        $edit_dialog->addHiddenInput('op', 'create');
+        $edit_dialog->addHiddenInput('changeset', $changeset);
+        $edit_dialog->addHiddenInput('is_new', $is_new);
+        $edit_dialog->addHiddenInput('number', $number);
+        $edit_dialog->addHiddenInput('length', $length);
+
+        $edit_dialog->appendChild($this->renderTextArea(''));
+
+        return id(new AphrontDialogResponse())->setDialog($edit_dialog);
+    }
+  }
+
+  private function buildRenderedCommentResponse(
+    DifferentialInlineComment $inline,
+    $on_right) {
+
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $factory = new DifferentialMarkupEngineFactory();
+    $engine = $factory->newDifferentialCommentMarkupEngine();
+
+    $phids = array($user->getPHID());
+
+    $handles = id(new PhabricatorObjectHandleData($phids))
+      ->loadHandles();
+
     $view = new DifferentialInlineCommentView();
     $view->setInlineComment($inline);
     $view->setOnRight($on_right);
     $view->setBuildScaffolding(true);
     $view->setMarkupEngine($engine);
     $view->setHandles($handles);
+    $view->setEditable(true);
 
     return id(new AphrontAjaxResponse())
       ->setContent(
@@ -92,25 +170,23 @@ class DifferentialInlineCommentEditController extends DifferentialController {
           'inlineCommentID' => $inline->getID(),
           'markup'          => $view->render(),
         ));
-      default:
-        $dialog = new AphrontDialogView();
-        $dialog->setUser($user);
-        $dialog->setTitle('New Inline Comment');
-        $dialog->setSubmitURI($submit_uri);
-
-        $dialog->addHiddenInput('op', 'create');
-        $dialog->addHiddenInput('changeset', $changeset);
-        $dialog->addHiddenInput('is_new', $is_new);
-        $dialog->addHiddenInput('on_right', $on_right);
-        $dialog->addHiddenInput('number', $number);
-        $dialog->addHiddenInput('length', $length);
-
-        $dialog->addSubmitButton();
-        $dialog->addCancelButton('#');
-        $dialog->appendChild('<textarea name="text"></textarea>');
-
-        return id(new AphrontDialogResponse())->setDialog($dialog);
   }
+
+  private function buildDeletedResponse() {
+    return id(new AphrontAjaxResponse())
+      ->setContent(
+        array(
+          'markup'          => '',
+        ));
+  }
+
+  private function renderTextArea($text) {
+    return phutil_render_tag(
+      'textarea',
+      array(
+        'name' => 'text',
+      ),
+      $text);
   }
 
 }

src/applications/differential/controller/inlinecommentedit/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'aphront/response/400');
 phutil_require_module('phabricator', 'aphront/response/ajax');
 phutil_require_module('phabricator', 'aphront/response/dialog');
 phutil_require_module('phabricator', 'applications/differential/controller/base');
@@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'applications/differential/view/inlinecomme
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'view/dialog');
 
+phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/differential/parser/changeset/DifferentialChangesetParser.php

@@ -43,6 +43,7 @@ class DifferentialChangesetParser {
   protected $noHighlight;
 
   private $handles;
+  private $user;
 
   const CACHE_VERSION = 4;
 
@@ -103,6 +104,11 @@ class DifferentialChangesetParser {
     return $this;
   }
 
+  public function setUser(PhabricatorUser $user) {
+    $this->user = $user;
+    return $this;
+  }
+
   public function parseHunk(DifferentialHunk $hunk) {
     $this->parsedHunk = true;
     $lines = $hunk->getChanges();
@@ -674,7 +680,6 @@ EOSYNTHETIC;
   }
 
   public function render(
-    ViewerContext $viewer_context = null,
     $range_start  = null,
     $range_len    = null,
     $mask_force   = array()) {
@@ -829,7 +834,6 @@ EOSYNTHETIC;
       $range_len,
       $mask_force,
       $feedback_mask,
-      $viewer_context,
       $old_comments,
       $new_comments);
 
@@ -884,7 +888,6 @@ EOSYNTHETIC;
     $range_len,
     $mask_force,
     $feedback_mask,
-    $viewer_context,
     array $old_comments,
     array $new_comments) {
 
@@ -1083,9 +1086,7 @@ EOSYNTHETIC;
 
       if ($o_num && isset($old_comments[$o_num])) {
         foreach ($old_comments[$o_num] as $comment) {
-          $xhp = $this->renderInlineComment(
+          $xhp = $this->renderInlineComment($comment);
-            $comment,
-            $viewer_context);
           $html[] =
             '<tr class="inline"><th /><td>'.
               $xhp.
@@ -1094,9 +1095,7 @@ EOSYNTHETIC;
       }
       if ($n_num && isset($new_comments[$n_num])) {
         foreach ($new_comments[$n_num] as $comment) {
-          $xhp = $this->renderInlineComment(
+          $xhp = $this->renderInlineComment($comment);
-            $comment,
-            $viewer_context);
           $html[] =
             '<tr class="inline"><th /><td /><th /><td>'.
               $xhp.
@@ -1108,21 +1107,21 @@ EOSYNTHETIC;
     return implode('', $html);
   }
 
-  private function renderInlineComment(
+  private function renderInlineComment(DifferentialInlineComment $comment) {
-    DifferentialInlineComment $comment,
-    $viewer_context) {
 
-    $edit = $viewer_context &&
+    $user = $this->user;
-            ($comment->getAuthorPHID() == $viewer_context->getUserID()) &&
+    $edit = $user &&
-            (!$comment->getFeedbackID());
+            ($comment->getAuthorPHID() == $user->getPHID()) &&
+            (!$comment->getCommentID());
 
-    $is_new = $this->isCommentInNewFile($comment);
+    $on_right = $this->isCommentInNewFile($comment);
 
     return id(new DifferentialInlineCommentView())
       ->setInlineComment($comment)
-      ->setOnRight($is_new)
+      ->setOnRight($on_right)
       ->setHandles($this->handles)
       ->setMarkupEngine($this->markupEngine)
+      ->setEditable($edit)
       ->render();
   }
 

src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php

@@ -23,6 +23,7 @@ final class DifferentialInlineCommentView extends AphrontView {
   private $buildScaffolding;
   private $handles;
   private $markupEngine;
+  private $editable;
 
   public function setInlineComment(DifferentialInlineComment $comment) {
     $this->inlineComment = $comment;
@@ -49,6 +50,11 @@ final class DifferentialInlineCommentView extends AphrontView {
     return $this;
   }
 
+  public function setEditable($editable) {
+    $this->editable = $editable;
+    return $this;
+  }
+
   public function render() {
 
     $inline = $this->inlineComment;
@@ -63,22 +69,44 @@ final class DifferentialInlineCommentView extends AphrontView {
     }
 
     $metadata = array(
+      'id' => $inline->getID(),
       'number' => $inline->getLineNumber(),
       'length' => $inline->getLineLength(),
-      'on_right' => $this->onRight, // TODO
+      'on_right' => $this->onRight,
     );
 
     $sigil = 'differential-inline-comment';
 
-    $links = 'xxx';
     $content = $inline->getContent();
     $handles = $this->handles;
 
+    $links = array();
+    if ($this->editable) {
+      $links[] = javelin_render_tag(
+        'a',
+        array(
+          'href'        => '#',
+          'mustcapture' => true,
+          'sigil'       => 'differential-inline-edit',
+        ),
+        'Edit');
+      $links[] = javelin_render_tag(
+        'a',
+        array(
+          'href'        => '#',
+          'mustcapture' => true,
+          'sigil'       => 'differential-inline-delete',
+        ),
+        'Delete');
+    }
+
     if ($links) {
       $links =
         '<span class="differential-inline-comment-links">'.
-          $links.
+          implode(' &middot; ', $links).
         '</span>';
+    } else {
+      $links = null;
     }
 
     $content = $this->markupEngine->markupText($content);
@@ -93,7 +121,7 @@ final class DifferentialInlineCommentView extends AphrontView {
       '<div class="differential-inline-comment-head">'.
         $links.
         '<span class="differential-inline-comment-line">'.$line.'</span>'.
-        $handles[$inline->getAuthorPHID()]->renderLink().
+        phutil_escape_html($handles[$inline->getAuthorPHID()]->getName()).
       '</div>'.
       $content);
 

src/applications/differential/view/inlinecomment/__init__.php

@@ -9,5 +9,7 @@
 phutil_require_module('phabricator', 'infrastructure/javelin/markup');
 phutil_require_module('phabricator', 'view/base');
 
+phutil_require_module('phutil', 'markup');
+
 
 phutil_require_source('DifferentialInlineCommentView.php');

webroot/rsrc/css/application/differential/changeset-view.css

@@ -160,7 +160,7 @@
   white-space:  nowrap;
 }
 
-.differential-inline-comment-edit {
+.differential-inline-comment-links {
-  padding-left: 8px;
+  margin-left:  8px;
   font-style:   normal;
 }

webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js

@@ -210,12 +210,13 @@ JX.behavior('differential-edit-inline-comments', function(config) {
 
   JX.Stratcom.listen(
     'click',
-    [['differential-inline-comment', 'delete'],
+    [['differential-inline-comment', 'differential-inline-delete'],
-     ['differential-inline-comment', 'edit']],
+     ['differential-inline-comment', 'differential-inline-edit']],
     function(e) {
       var data = {
-        op: e.getNode('edit') ? 'edit' : 'delete',
+        op: e.getNode('differential-inline-edit') ? 'edit' : 'delete',
-        id: e.getNodeData('differential-inline-comment').id
+        id: e.getNodeData('differential-inline-comment').id,
+        on_right: e.getNodeData('differential-inline-comment').on_right,
       };
       new JX.Workflow(config.uri, data)
         .setHandler(function(r) {

webroot/rsrc/js/application/differential/behavior-show-more.js

@@ -21,7 +21,7 @@ JX.behavior('differential-show-more', function(config) {
       var container = JX.DOM.find(context, 'td');
       JX.DOM.setContent(container, 'Loading...');
       JX.DOM.alterClass(context, 'differential-show-more-loading', true);
-      var data = e.getData()['show-more'];
+      var data = e.getNodeData('show-more');
       new JX.Request(config.uri, JX.bind(null, onresponse, e))
         .setData(data)
         .send();