From c5ce156e71039ba084a76e2b9f8230e0f54fdbe8 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Feb 2011 10:10:25 -0800 Subject: [PATCH] Edit/Delete for inline comments --- .../DifferentialChangesetViewController.php | 8 +- ...ifferentialInlineCommentEditController.php | 162 +++++++++++++----- .../controller/inlinecommentedit/__init__.php | 2 + .../changeset/DifferentialChangesetParser.php | 33 ++-- .../DifferentialInlineCommentView.php | 36 +++- .../view/inlinecomment/__init__.php | 2 + .../differential/changeset-view.css | 4 +- .../behavior-edit-inline-comments.js | 9 +- .../differential/behavior-show-more.js | 2 +- 9 files changed, 186 insertions(+), 72 deletions(-) diff --git a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php index c9692c6fb2..558f747d7c 100644 --- a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php +++ b/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()) diff --git a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php index 8199613f52..1eb44e9c07 100644 --- a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php +++ b/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.'/'; + $edit_dialog = new AphrontDialogView(); + $edit_dialog->setUser($user); + $edit_dialog->setSubmitURI($submit_uri); + + $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!"); + } + } + switch ($op) { case 'delete': - if ($request->isFormPost()) { - // do the delete; - return new AphrontAjaxResponse(); + if (!$inline) { + return new Aphront400Response(); } - $dialog = new AphrontDialogView(); - $dialog->setTitle('Really delete this comment?'); + if ($request->isFormPost()) { + $inline->delete(); + return $this->buildDeletedResponse(); + } - return id(new AphrontDialogResponse())->setDialog($dialog); + $edit_dialog->setTitle('Really delete this comment?'); + $edit_dialog->addHiddenInput('id', $inline_id); + $edit_dialog->addHiddenInput('op', 'delete'); + $edit_dialog->appendChild( + '

Delete this inline comment?

'); + + 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,38 +125,68 @@ class DifferentialInlineCommentEditController extends DifferentialController { ->setContent($text) ->save(); - $view = new DifferentialInlineCommentView(); - $view->setInlineComment($inline); - $view->setOnRight($on_right); - $view->setBuildScaffolding(true); - $view->setMarkupEngine($engine); - $view->setHandles($handles); - - return id(new AphrontAjaxResponse()) - ->setContent( - array( - 'inlineCommentID' => $inline->getID(), - 'markup' => $view->render(), - )); + return $this->buildRenderedCommentResponse($inline, $on_right); default: - $dialog = new AphrontDialogView(); - $dialog->setUser($user); - $dialog->setTitle('New Inline Comment'); - $dialog->setSubmitURI($submit_uri); + $edit_dialog->setTitle('New Inline Comment'); - $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); + $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); - $dialog->addSubmitButton(); - $dialog->addCancelButton('#'); - $dialog->appendChild(''); + $edit_dialog->appendChild($this->renderTextArea('')); - return id(new AphrontDialogResponse())->setDialog($dialog); + 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( + array( + 'inlineCommentID' => $inline->getID(), + 'markup' => $view->render(), + )); + } + + private function buildDeletedResponse() { + return id(new AphrontAjaxResponse()) + ->setContent( + array( + 'markup' => '', + )); + } + + private function renderTextArea($text) { + return phutil_render_tag( + 'textarea', + array( + 'name' => 'text', + ), + $text); + } + } diff --git a/src/applications/differential/controller/inlinecommentedit/__init__.php b/src/applications/differential/controller/inlinecommentedit/__init__.php index 80f7296260..5b2d0020ee 100644 --- a/src/applications/differential/controller/inlinecommentedit/__init__.php +++ b/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'); diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index 29ece8b849..7c13efa57d 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/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( - $comment, - $viewer_context); + $xhp = $this->renderInlineComment($comment); $html[] = ''. $xhp. @@ -1094,9 +1095,7 @@ EOSYNTHETIC; } if ($n_num && isset($new_comments[$n_num])) { foreach ($new_comments[$n_num] as $comment) { - $xhp = $this->renderInlineComment( - $comment, - $viewer_context); + $xhp = $this->renderInlineComment($comment); $html[] = ''. $xhp. @@ -1108,21 +1107,21 @@ EOSYNTHETIC; return implode('', $html); } - private function renderInlineComment( - DifferentialInlineComment $comment, - $viewer_context) { + private function renderInlineComment(DifferentialInlineComment $comment) { - $edit = $viewer_context && - ($comment->getAuthorPHID() == $viewer_context->getUserID()) && - (!$comment->getFeedbackID()); + $user = $this->user; + $edit = $user && + ($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(); } diff --git a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php index d255a54065..a51dd7663a 100644 --- a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php +++ b/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 = ''. - $links. + implode(' · ', $links). ''; + } else { + $links = null; } $content = $this->markupEngine->markupText($content); @@ -93,7 +121,7 @@ final class DifferentialInlineCommentView extends AphrontView { '
'. $links. ''.$line.''. - $handles[$inline->getAuthorPHID()]->renderLink(). + phutil_escape_html($handles[$inline->getAuthorPHID()]->getName()). '
'. $content); diff --git a/src/applications/differential/view/inlinecomment/__init__.php b/src/applications/differential/view/inlinecomment/__init__.php index 08aaec2ca1..71452b2ed7 100644 --- a/src/applications/differential/view/inlinecomment/__init__.php +++ b/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'); diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 00f316712d..de9fa3004d 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -160,7 +160,7 @@ white-space: nowrap; } -.differential-inline-comment-edit { - padding-left: 8px; +.differential-inline-comment-links { + margin-left: 8px; font-style: normal; } diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js index 0dbfda475e..a071abf7e8 100644 --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/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', 'edit']], + [['differential-inline-comment', 'differential-inline-delete'], + ['differential-inline-comment', 'differential-inline-edit']], function(e) { var data = { - op: e.getNode('edit') ? 'edit' : 'delete', - id: e.getNodeData('differential-inline-comment').id + op: e.getNode('differential-inline-edit') ? 'edit' : 'delete', + 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) { diff --git a/webroot/rsrc/js/application/differential/behavior-show-more.js b/webroot/rsrc/js/application/differential/behavior-show-more.js index 25748ba6af..18c53575cc 100644 --- a/webroot/rsrc/js/application/differential/behavior-show-more.js +++ b/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();