revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-19 16:58:48 +02:00
Code Issues Releases Wiki Activity

Make "editing" state persistent for inline comments

Browse source 
Summary:
Ref T13513. This is mostly an infrastructure cleanup change.

In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change.

---

Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes.

On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another.

Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it).

To simplify this:

  - Make `PhabricatorInlineCommentInterface` an abstract base class instead.
  - Lift as much code out of the `Audit` and `Differential` subclasses as possible.
  - Delete methods which no longer have callers, or have only trivial callers.

---

Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code.

Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`.

These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code.

The `EditView` can not fully render its content. Move the content rendering code into the view.

---

Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately.

This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`.

---

Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level.

Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle.

This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further.

---

Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row".

Test Plan:
  - Created comments on either side of a diff.
  - Edited a comment, reloaded, saw edit stick.
  - Saved comments, reloaded, saw save stick.
  - Edited a comment, typed text, cancelled, "unedited" to get state back.
  - Created a comment, typed text, cancelled, "unedited" to get state back.
  - Deleted a comment, "undeleted" to get state back.

Weirdness / known issues:

  - Drafts don't autosave yet.
  - Fixed in D21187:
    - When you create an empty comment then reload, you get an empty editor. This is a bit silly.
    - "Cancel" does not save state, but should, once drafts autosave.
  - Mostly fixed in D21188:
    - "Editing" comments aren't handled specially by the overall submission flow.
    - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work.

Subscribers: jmeador

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21186
This commit is contained in:
epriestley 2020-04-28 14:45:54 -07:00
parent 5ff0ae7d48
commit b48a22bf50
27 changed files with 696 additions and 953 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
resources/celerity/map.php
View file

@ -13,7 +13,7 @@ return array(
'core.pkg.js' => '632fb8f5',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '2d70b7b9',
'differential.pkg.js' => 'c8f88d74',
'differential.pkg.js' => 'b289f75d',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
@ -380,8 +380,8 @@ return array(
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
'rsrc/js/application/diff/DiffChangeset.js' => '9a713ba5',
'rsrc/js/application/diff/DiffChangesetList.js' => 'adf069cd',
'rsrc/js/application/diff/DiffInline.js' => '16e97ebc',
'rsrc/js/application/diff/DiffChangesetList.js' => '10726e6a',
'rsrc/js/application/diff/DiffInline.js' => '7b0bdd6d',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17',
@ -777,8 +777,8 @@ return array(
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => '9a713ba5',
'phabricator-diff-changeset-list' => 'adf069cd',
'phabricator-diff-inline' => '16e97ebc',
'phabricator-diff-changeset-list' => '10726e6a',
'phabricator-diff-inline' => '7b0bdd6d',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
'phabricator-drag-and-drop-file-upload' => '4370900d',
@ -1022,6 +1022,11 @@ return array(
'javelin-workflow',
'phuix-icon-view',
),
'10726e6a' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'111bfd2d' => array(
'javelin-install',
),
@ -1036,9 +1041,6 @@ return array(
'javelin-stratcom',
'javelin-util',
),
'16e97ebc' => array(
'javelin-dom',
),
'1a844c06' => array(
'javelin-install',
'javelin-util',
@ -1614,6 +1616,9 @@ return array(
'phabricator-drag-and-drop-file-upload',
'phabricator-textareautils',
),
'7b0bdd6d' => array(
'javelin-dom',
),
'7b139193' => array(
'javelin-behavior',
'javelin-stratcom',
@ -1930,11 +1935,6 @@ return array(
'javelin-typeahead-ondemand-source',
'javelin-util',
),
'adf069cd' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'aec8e38c' => array(
'javelin-dom',
'javelin-util',

17
src/__phutil_library_map__.php
View file

@ -3592,8 +3592,8 @@ phutil_register_library_map(array(
'PhabricatorIndexEngineExtensionModule' => 'applications/search/index/PhabricatorIndexEngineExtensionModule.php',
'PhabricatorIndexableInterface' => 'applications/search/interface/PhabricatorIndexableInterface.php',
'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php',
'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php',
'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php',
'PhabricatorInlineCommentInterface' => 'infrastructure/diff/interface/PhabricatorInlineCommentInterface.php',
'PhabricatorInlineCommentPreviewController' => 'infrastructure/diff/PhabricatorInlineCommentPreviewController.php',
'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php',
'PhabricatorInstructionsEditField' => 'applications/transactions/editfield/PhabricatorInstructionsEditField.php',
@ -6623,10 +6623,7 @@ phutil_register_library_map(array(
'DifferentialHunkParserTestCase' => 'PhabricatorTestCase',
'DifferentialHunkQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DifferentialHunkTestCase' => 'PhutilTestCase',
'DifferentialInlineComment' => array(
'Phobject',
'PhabricatorInlineCommentInterface',
),
'DifferentialInlineComment' => 'PhabricatorInlineComment',
'DifferentialInlineCommentEditController' => 'PhabricatorInlineCommentController',
'DifferentialInlineCommentMailView' => 'DifferentialMailView',
'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery',
@ -8595,10 +8592,7 @@ phutil_register_library_map(array(
'PhabricatorAuditCommentEditor' => 'PhabricatorEditor',
'PhabricatorAuditController' => 'PhabricatorController',
'PhabricatorAuditEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorAuditInlineComment' => array(
'Phobject',
'PhabricatorInlineCommentInterface',
),
'PhabricatorAuditInlineComment' => 'PhabricatorInlineComment',
'PhabricatorAuditListView' => 'AphrontView',
'PhabricatorAuditMailReceiver' => 'PhabricatorObjectMailReceiver',
'PhabricatorAuditManagementDeleteWorkflow' => 'PhabricatorAuditManagementWorkflow',
@ -10115,8 +10109,11 @@ phutil_register_library_map(array(
'PhabricatorIndexEngineExtension' => 'Phobject',
'PhabricatorIndexEngineExtensionModule' => 'PhabricatorConfigModule',
'PhabricatorInfrastructureTestCase' => 'PhabricatorTestCase',
'PhabricatorInlineComment' => array(
'Phobject',
'PhabricatorMarkupInterface',
),
'PhabricatorInlineCommentController' => 'PhabricatorController',
'PhabricatorInlineCommentInterface' => 'PhabricatorMarkupInterface',
'PhabricatorInlineCommentPreviewController' => 'PhabricatorController',
'PhabricatorInlineSummaryView' => 'AphrontView',
'PhabricatorInstructionsEditField' => 'PhabricatorEditField',

234
src/applications/audit/storage/PhabricatorAuditInlineComment.php
View file

@ -1,27 +1,16 @@
<?php
final class PhabricatorAuditInlineComment
extends Phobject
implements PhabricatorInlineCommentInterface {
extends PhabricatorInlineComment {
private $proxy;
private $syntheticAuthor;
private $isGhost;
public function __construct() {
$this->proxy = new PhabricatorAuditTransactionComment();
protected function newStorageObject() {
return new PhabricatorAuditTransactionComment();
}
public function __clone() {
$this->proxy = clone $this->proxy;
}
public function getTransactionPHID() {
return $this->proxy->getTransactionPHID();
}
public function getTransactionComment() {
return $this->proxy;
public function getControllerURI() {
return urisprintf(
'/diffusion/inline/edit/%s/',
$this->getCommitPHID());
}
public function supportsHiding() {
@ -36,13 +25,13 @@ final class PhabricatorAuditInlineComment
$content_source = PhabricatorContentSource::newForSource(
PhabricatorOldWorldContentSource::SOURCECONST);
$this->proxy
$this->getStorageObject()
->setViewPolicy('public')
->setEditPolicy($this->getAuthorPHID())
->setContentSource($content_source)
->setCommentVersion(1);
return $this->proxy;
return $this->getStorageObject();
}
public static function loadID($id) {
@ -134,148 +123,31 @@ final class PhabricatorAuditInlineComment
return $results;
}
public function setSyntheticAuthor($synthetic_author) {
$this->syntheticAuthor = $synthetic_author;
return $this;
}
public function getSyntheticAuthor() {
return $this->syntheticAuthor;
}
public function openTransaction() {
$this->proxy->openTransaction();
}
public function saveTransaction() {
$this->proxy->saveTransaction();
}
public function save() {
$this->getTransactionCommentForSave()->save();
return $this;
}
public function delete() {
$this->proxy->delete();
return $this;
}
public function getID() {
return $this->proxy->getID();
}
public function getPHID() {
return $this->proxy->getPHID();
}
public static function newFromModernComment(
PhabricatorAuditTransactionComment $comment) {
$obj = new PhabricatorAuditInlineComment();
$obj->proxy = $comment;
$obj->setStorageObject($comment);
return $obj;
}
public function isCompatible(PhabricatorInlineCommentInterface $comment) {
return
($this->getAuthorPHID() === $comment->getAuthorPHID()) &&
($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) &&
($this->getContent() === $comment->getContent());
}
public function setContent($content) {
$this->proxy->setContent($content);
return $this;
}
public function getContent() {
return $this->proxy->getContent();
}
public function isDraft() {
return !$this->proxy->getTransactionPHID();
}
public function setPathID($id) {
$this->proxy->setPathID($id);
$this->getStorageObject()->setPathID($id);
return $this;
}
public function getPathID() {
return $this->proxy->getPathID();
}
public function setIsNewFile($is_new) {
$this->proxy->setIsNewFile($is_new);
return $this;
}
public function getIsNewFile() {
return $this->proxy->getIsNewFile();
}
public function setLineNumber($number) {
$this->proxy->setLineNumber($number);
return $this;
}
public function getLineNumber() {
return $this->proxy->getLineNumber();
}
public function setLineLength($length) {
$this->proxy->setLineLength($length);
return $this;
}
public function getLineLength() {
return $this->proxy->getLineLength();
}
public function setCache($cache) {
return $this;
}
public function getCache() {
return null;
}
public function setAuthorPHID($phid) {
$this->proxy->setAuthorPHID($phid);
return $this;
}
public function getAuthorPHID() {
return $this->proxy->getAuthorPHID();
return $this->getStorageObject()->getPathID();
}
public function setCommitPHID($commit_phid) {
$this->proxy->setCommitPHID($commit_phid);
$this->getStorageObject()->setCommitPHID($commit_phid);
return $this;
}
public function getCommitPHID() {
return $this->proxy->getCommitPHID();
}
// When setting a comment ID, we also generate a phantom transaction PHID for
// the future transaction.
public function setAuditCommentID($id) {
$this->proxy->setLegacyCommentID($id);
$this->proxy->setTransactionPHID(
PhabricatorPHID::generateNewPHID(
PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST,
PhabricatorRepositoryCommitPHIDType::TYPECONST));
return $this;
}
public function getAuditCommentID() {
return $this->proxy->getLegacyCommentID();
return $this->getStorageObject()->getCommitPHID();
}
public function setChangesetID($id) {
@ -286,82 +158,4 @@ final class PhabricatorAuditInlineComment
return $this->getPathID();
}
public function setReplyToCommentPHID($phid) {
$this->proxy->setReplyToCommentPHID($phid);
return $this;
}
public function getReplyToCommentPHID() {
return $this->proxy->getReplyToCommentPHID();
}
public function setHasReplies($has_replies) {
$this->proxy->setHasReplies($has_replies);
return $this;
}
public function getHasReplies() {
return $this->proxy->getHasReplies();
}
public function setIsDeleted($is_deleted) {
$this->proxy->setIsDeleted($is_deleted);
return $this;
}
public function getIsDeleted() {
return $this->proxy->getIsDeleted();
}
public function setFixedState($state) {
$this->proxy->setFixedState($state);
return $this;
}
public function getFixedState() {
return $this->proxy->getFixedState();
}
public function setIsGhost($is_ghost) {
$this->isGhost = $is_ghost;
return $this;
}
public function getIsGhost() {
return $this->isGhost;
}
public function getDateModified() {
return $this->proxy->getDateModified();
}
public function getDateCreated() {
return $this->proxy->getDateCreated();
}
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
public function getMarkupFieldKey($field) {
return 'AI:'.$this->getID();
}
public function newMarkupEngine($field) {
return PhabricatorMarkupEngine::newDifferentialMarkupEngine();
}
public function getMarkupText($field) {
return $this->getContent();
}
public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
return $output;
}
public function shouldUseMarkupCache($field) {
// Only cache submitted comments.
return ($this->getID() && $this->getAuditCommentID());
}
}

9
src/applications/audit/storage/PhabricatorAuditTransactionComment.php
View file

@ -72,4 +72,13 @@ final class PhabricatorAuditTransactionComment
return $this->assertAttached($this->replyToComment);
}
public function getAttribute($key, $default = null) {
return idx($this->attributes, $key, $default);
}
public function setAttribute($key, $value) {
$this->attributes[$key] = $value;
return $this;
}
}

2
src/applications/differential/controller/DifferentialChangesetViewController.php
View file

@ -238,7 +238,7 @@ final class DifferentialChangesetViewController extends DifferentialController {
foreach ($inlines as $inline) {
$engine->addObject(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
}
$engine->process();

14
src/applications/differential/controller/DifferentialInlineCommentEditController.php
View file

@ -80,9 +80,15 @@ final class DifferentialInlineCommentEditController
$viewer = $this->getViewer();
$inline = $this->loadComment($id);
if (!$inline) {
throw new Exception(
pht('Unable to load inline "%s".', $id));
}
if (!$this->canEditInlineComment($viewer, $inline)) {
throw new Exception(pht('That comment is not editable!'));
}
return $inline;
}
@ -161,7 +167,7 @@ final class DifferentialInlineCommentEditController
return true;
}
protected function deleteComment(PhabricatorInlineCommentInterface $inline) {
protected function deleteComment(PhabricatorInlineComment $inline) {
$inline->openTransaction();
$inline->setIsDeleted(1)->save();
$this->syncDraft();
@ -169,14 +175,14 @@ final class DifferentialInlineCommentEditController
}
protected function undeleteComment(
PhabricatorInlineCommentInterface $inline) {
PhabricatorInlineComment $inline) {
$inline->openTransaction();
$inline->setIsDeleted(0)->save();
$this->syncDraft();
$inline->saveTransaction();
}
protected function saveComment(PhabricatorInlineCommentInterface $inline) {
protected function saveComment(PhabricatorInlineComment $inline) {
$inline->openTransaction();
$inline->save();
$this->syncDraft();
@ -184,7 +190,7 @@ final class DifferentialInlineCommentEditController
}
protected function loadObjectOwnerPHID(
PhabricatorInlineCommentInterface $inline) {
PhabricatorInlineComment $inline) {
return $this->loadRevision()->getAuthorPHID();
}

2
src/applications/differential/controller/DifferentialRevisionInlinesController.php
View file

@ -124,7 +124,7 @@ final class DifferentialRevisionInlinesController
$inline->getContent());
$state = $inline->getFixedState();
if ($state == PhabricatorInlineCommentInterface::STATE_DONE) {
if ($state == PhabricatorInlineComment::STATE_DONE) {
$status_icons[] = id(new PHUIIconView())
->setIcon('fa-check green')
->addClass('mmr');

10
src/applications/differential/parser/DifferentialChangesetParser.php
View file

@ -354,7 +354,7 @@ final class DifferentialChangesetParser extends Phobject {
}
public function parseInlineComment(
PhabricatorInlineCommentInterface $comment) {
PhabricatorInlineComment $comment) {
// Parse only comments which are actually visible.
if ($this->isCommentVisibleOnRenderedDiff($comment)) {
@ -1191,11 +1191,11 @@ final class DifferentialChangesetParser extends Phobject {
* taking into consideration which halves of which changesets will actually
* be shown.
*
* @param PhabricatorInlineCommentInterface Comment to test for visibility.
* @param PhabricatorInlineComment Comment to test for visibility.
* @return bool True if the comment is visible on the rendered diff.
*/
private function isCommentVisibleOnRenderedDiff(
PhabricatorInlineCommentInterface $comment) {
PhabricatorInlineComment $comment) {
$changeset_id = $comment->getChangesetID();
$is_new = $comment->getIsNewFile();
@ -1219,12 +1219,12 @@ final class DifferentialChangesetParser extends Phobject {
* Note that the comment must appear somewhere on the rendered changeset, as
* per isCommentVisibleOnRenderedDiff().
*
* @param PhabricatorInlineCommentInterface Comment to test for display
* @param PhabricatorInlineComment Comment to test for display
* location.
* @return bool True for right, false for left.
*/
private function isCommentOnRightSideWhenDisplayed(
PhabricatorInlineCommentInterface $comment) {
PhabricatorInlineComment $comment) {
if (!$this->isCommentVisibleOnRenderedDiff($comment)) {
throw new Exception(pht('Comment is not visible on changeset!'));

12
src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
View file

@ -464,19 +464,19 @@ abstract class DifferentialChangesetHTMLRenderer
}
protected function buildInlineComment(
PhabricatorInlineCommentInterface $comment,
PhabricatorInlineComment $comment,
$on_right = false) {
$user = $this->getUser();
$edit = $user &&
($comment->getAuthorPHID() == $user->getPHID()) &&
$viewer = $this->getUser();
$edit = $viewer &&
($comment->getAuthorPHID() == $viewer->getPHID()) &&
($comment->isDraft())
&& $this->getShowEditAndReplyLinks();
$allow_reply = (bool)$user && $this->getShowEditAndReplyLinks();
$allow_reply = (bool)$viewer && $this->getShowEditAndReplyLinks();
$allow_done = !$comment->isDraft() && $this->getCanMarkDone();
return id(new PHUIDiffInlineCommentDetailView())
->setUser($user)
->setViewer($viewer)
->setInlineComment($comment)
->setIsOnRight($on_right)
->setHandles($this->getHandles())

4
src/applications/differential/render/DifferentialChangesetRenderer.php
View file

@ -263,7 +263,7 @@ abstract class DifferentialChangesetRenderer extends Phobject {
public function setNewComments(array $new_comments) {
foreach ($new_comments as $line_number => $comments) {
assert_instances_of($comments, 'PhabricatorInlineCommentInterface');
assert_instances_of($comments, 'PhabricatorInlineComment');
}
$this->newComments = $new_comments;
return $this;
@ -274,7 +274,7 @@ abstract class DifferentialChangesetRenderer extends Phobject {
public function setOldComments(array $old_comments) {
foreach ($old_comments as $line_number => $comments) {
assert_instances_of($comments, 'PhabricatorInlineCommentInterface');
assert_instances_of($comments, 'PhabricatorInlineComment');
}
$this->oldComments = $old_comments;
return $this;

230
src/applications/differential/storage/DifferentialInlineComment.php
View file

@ -1,53 +1,30 @@
<?php
final class DifferentialInlineComment
extends Phobject
implements PhabricatorInlineCommentInterface {
extends PhabricatorInlineComment {
private $proxy;
private $syntheticAuthor;
private $isGhost;
public function __construct() {
$this->proxy = new DifferentialTransactionComment();
protected function newStorageObject() {
return new DifferentialTransactionComment();
}
public function __clone() {
$this->proxy = clone $this->proxy;
public function getControllerURI() {
return urisprintf(
'/differential/comment/inline/edit/%s/',
$this->getRevisionID());
}
public function getTransactionCommentForSave() {
$content_source = PhabricatorContentSource::newForSource(
PhabricatorOldWorldContentSource::SOURCECONST);
$this->proxy
$this->getStorageObject()
->setViewPolicy('public')
->setEditPolicy($this->getAuthorPHID())
->setContentSource($content_source)
->attachIsHidden(false)
->setCommentVersion(1);
return $this->proxy;
}
public function openTransaction() {
$this->proxy->openTransaction();
}
public function saveTransaction() {
$this->proxy->saveTransaction();
}
public function save() {
$this->getTransactionCommentForSave()->save();
return $this;
}
public function delete() {
$this->proxy->delete();
return $this;
return $this->getStorageObject();
}
public function supportsHiding() {
@ -61,115 +38,34 @@ final class DifferentialInlineComment
if (!$this->supportsHiding()) {
return false;
}
return $this->proxy->getIsHidden();
}
public function getID() {
return $this->proxy->getID();
}
public function getPHID() {
return $this->proxy->getPHID();
return $this->getStorageObject()->getIsHidden();
}
public static function newFromModernComment(
DifferentialTransactionComment $comment) {
$obj = new DifferentialInlineComment();
$obj->proxy = $comment;
$obj->setStorageObject($comment);
return $obj;
}
public function setSyntheticAuthor($synthetic_author) {
$this->syntheticAuthor = $synthetic_author;
return $this;
}
public function getSyntheticAuthor() {
return $this->syntheticAuthor;
}
public function isCompatible(PhabricatorInlineCommentInterface $comment) {
return
($this->getAuthorPHID() === $comment->getAuthorPHID()) &&
($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) &&
($this->getContent() === $comment->getContent());
}
public function setContent($content) {
$this->proxy->setContent($content);
return $this;
}
public function getContent() {
return $this->proxy->getContent();
}
public function isDraft() {
return !$this->proxy->getTransactionPHID();
}
public function setChangesetID($id) {
$this->proxy->setChangesetID($id);
$this->getStorageObject()->setChangesetID($id);
return $this;
}
public function getChangesetID() {
return $this->proxy->getChangesetID();
}
public function setIsNewFile($is_new) {
$this->proxy->setIsNewFile($is_new);
return $this;
}
public function getIsNewFile() {
return $this->proxy->getIsNewFile();
}
public function setLineNumber($number) {
$this->proxy->setLineNumber($number);
return $this;
}
public function getLineNumber() {
return $this->proxy->getLineNumber();
}
public function setLineLength($length) {
$this->proxy->setLineLength($length);
return $this;
}
public function getLineLength() {
return $this->proxy->getLineLength();
}
public function setCache($cache) {
return $this;
}
public function getCache() {
return null;
}
public function setAuthorPHID($phid) {
$this->proxy->setAuthorPHID($phid);
return $this;
}
public function getAuthorPHID() {
return $this->proxy->getAuthorPHID();
return $this->getStorageObject()->getChangesetID();
}
public function setRevision(DifferentialRevision $revision) {
$this->proxy->setRevisionPHID($revision->getPHID());
$this->getStorageObject()->setRevisionPHID($revision->getPHID());
return $this;
}
public function getRevisionPHID() {
return $this->proxy->getRevisionPHID();
return $this->getStorageObject()->getRevisionPHID();
}
// Although these are purely transitional, they're also *extra* dumb.
@ -180,7 +76,7 @@ final class DifferentialInlineComment
}
public function getRevisionID() {
$phid = $this->proxy->getRevisionPHID();
$phid = $this->getStorageObject()->getRevisionPHID();
if (!$phid) {
return null;
}
@ -194,98 +90,4 @@ final class DifferentialInlineComment
return $revision->getID();
}
// When setting a comment ID, we also generate a phantom transaction PHID for
// the future transaction.
public function setCommentID($id) {
$this->proxy->setTransactionPHID(
PhabricatorPHID::generateNewPHID(
PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST,
DifferentialRevisionPHIDType::TYPECONST));
return $this;
}
public function setReplyToCommentPHID($phid) {
$this->proxy->setReplyToCommentPHID($phid);
return $this;
}
public function getReplyToCommentPHID() {
return $this->proxy->getReplyToCommentPHID();
}
public function setHasReplies($has_replies) {
$this->proxy->setHasReplies($has_replies);
return $this;
}
public function getHasReplies() {
return $this->proxy->getHasReplies();
}
public function setIsDeleted($is_deleted) {
$this->proxy->setIsDeleted($is_deleted);
return $this;
}
public function getIsDeleted() {
return $this->proxy->getIsDeleted();
}
public function setFixedState($state) {
$this->proxy->setFixedState($state);
return $this;
}
public function getFixedState() {
return $this->proxy->getFixedState();
}
public function setIsGhost($is_ghost) {
$this->isGhost = $is_ghost;
return $this;
}
public function getIsGhost() {
return $this->isGhost;
}
public function makeEphemeral() {
$this->proxy->makeEphemeral();
return $this;
}
public function getDateModified() {
return $this->proxy->getDateModified();
}
public function getDateCreated() {
return $this->proxy->getDateCreated();
}
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
public function getMarkupFieldKey($field) {
$content = $this->getMarkupText($field);
return PhabricatorMarkupEngine::digestRemarkupContent($this, $content);
}
public function newMarkupEngine($field) {
return PhabricatorMarkupEngine::newDifferentialMarkupEngine();
}
public function getMarkupText($field) {
return $this->getContent();
}
public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
return $output;
}
public function shouldUseMarkupCache($field) {
// Only cache submitted comments.
return ($this->getID() && !$this->isDraft());
}
}

9
src/applications/differential/storage/DifferentialTransactionComment.php
View file

@ -118,4 +118,13 @@ final class DifferentialTransactionComment
return $this;
}
public function getAttribute($key, $default = null) {
return idx($this->attributes, $key, $default);
}
public function setAttribute($key, $value) {
$this->attributes[$key] = $value;
return $this;
}
}

4
src/applications/differential/xaction/DifferentialRevisionInlineTransaction.php
View file

@ -40,8 +40,8 @@ final class DifferentialRevisionInlineTransaction
$is_done = false;
switch ($comment->getFixedState()) {
case PhabricatorInlineCommentInterface::STATE_DONE:
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
case PhabricatorInlineComment::STATE_DONE:
case PhabricatorInlineComment::STATE_UNDRAFT:
$is_done = true;
break;
}

2
src/applications/diffusion/controller/DiffusionDiffController.php
View file

@ -120,7 +120,7 @@ final class DiffusionDiffController extends DiffusionController {
foreach ($inlines as $inline) {
$engine->addObject(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
}
$engine->process();

10
src/applications/diffusion/controller/DiffusionInlineCommentController.php
View file

@ -105,7 +105,7 @@ final class DiffusionInlineCommentController
}
// Saved comments may not be edited.
if ($inline->getAuditCommentID()) {
if ($inline->getTransactionPHID()) {
return false;
}
@ -117,21 +117,21 @@ final class DiffusionInlineCommentController
return true;
}
protected function deleteComment(PhabricatorInlineCommentInterface $inline) {
protected function deleteComment(PhabricatorInlineComment $inline) {
$inline->setIsDeleted(1)->save();
}
protected function undeleteComment(
PhabricatorInlineCommentInterface $inline) {
PhabricatorInlineComment $inline) {
$inline->setIsDeleted(0)->save();
}
protected function saveComment(PhabricatorInlineCommentInterface $inline) {
protected function saveComment(PhabricatorInlineComment $inline) {
return $inline->save();
}
protected function loadObjectOwnerPHID(
PhabricatorInlineCommentInterface $inline) {
PhabricatorInlineComment $inline) {
return $this->loadCommit()->getAuthorPHID();
}

8
src/applications/transactions/constants/PhabricatorTransactions.php
View file

@ -32,10 +32,10 @@ final class PhabricatorTransactions extends Phobject {
public static function getInlineStateMap() {
return array(
PhabricatorInlineCommentInterface::STATE_DRAFT =>
PhabricatorInlineCommentInterface::STATE_DONE,
PhabricatorInlineCommentInterface::STATE_UNDRAFT =>
PhabricatorInlineCommentInterface::STATE_UNDONE,
PhabricatorInlineComment::STATE_DRAFT =>
PhabricatorInlineComment::STATE_DONE,
PhabricatorInlineComment::STATE_UNDRAFT =>
PhabricatorInlineComment::STATE_UNDONE,
);
}

2
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
View file

@ -1690,7 +1690,7 @@ abstract class PhabricatorApplicationTransaction
$done = 0;
$undone = 0;
foreach ($new as $phid => $state) {
$is_done = ($state == PhabricatorInlineCommentInterface::STATE_DONE);
$is_done = ($state == PhabricatorInlineComment::STATE_DONE);
// See PHI995. If you're marking your own inline comments as "Done",
// don't count them when rendering a timeline story. In the case where

180
src/infrastructure/diff/PhabricatorInlineCommentController.php
View file

@ -9,13 +9,13 @@ abstract class PhabricatorInlineCommentController
abstract protected function loadCommentForDone($id);
abstract protected function loadCommentByPHID($phid);
abstract protected function loadObjectOwnerPHID(
PhabricatorInlineCommentInterface $inline);
PhabricatorInlineComment $inline);
abstract protected function deleteComment(
PhabricatorInlineCommentInterface $inline);
PhabricatorInlineComment $inline);
abstract protected function undeleteComment(
PhabricatorInlineCommentInterface $inline);
PhabricatorInlineComment $inline);
abstract protected function saveComment(
PhabricatorInlineCommentInterface $inline);
PhabricatorInlineComment $inline);
protected function hideComments(array $ids) {
throw new PhutilMethodNotImplementedException();
@ -119,20 +119,20 @@ abstract class PhabricatorInlineCommentController
$is_draft_state = false;
$is_checked = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineCommentInterface::STATE_DRAFT:
$next_state = PhabricatorInlineCommentInterface::STATE_UNDONE;
case PhabricatorInlineComment::STATE_DRAFT:
$next_state = PhabricatorInlineComment::STATE_UNDONE;
break;
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
$next_state = PhabricatorInlineCommentInterface::STATE_DONE;
case PhabricatorInlineComment::STATE_UNDRAFT:
$next_state = PhabricatorInlineComment::STATE_DONE;
$is_checked = true;
break;
case PhabricatorInlineCommentInterface::STATE_DONE:
$next_state = PhabricatorInlineCommentInterface::STATE_UNDRAFT;
case PhabricatorInlineComment::STATE_DONE:
$next_state = PhabricatorInlineComment::STATE_UNDRAFT;
$is_draft_state = true;
break;
default:
case PhabricatorInlineCommentInterface::STATE_UNDONE:
$next_state = PhabricatorInlineCommentInterface::STATE_DRAFT;
case PhabricatorInlineComment::STATE_UNDONE:
$next_state = PhabricatorInlineComment::STATE_DRAFT;
$is_draft_state = true;
$is_checked = true;
break;
@ -187,7 +187,10 @@ abstract class PhabricatorInlineCommentController
if ($request->isFormPost()) {
if (strlen($text)) {
$inline->setContent($text);
$inline
->setContent($text)
->setIsEditing(false);
$this->saveComment($inline);
return $this->buildRenderedCommentResponse(
$inline,
@ -196,65 +199,25 @@ abstract class PhabricatorInlineCommentController
$this->deleteComment($inline);
return $this->buildEmptyResponse();
}
} else {
$inline->setIsEditing(true);
if (strlen($text)) {
$inline->setContent($text);
}
$this->saveComment($inline);
}
$edit_dialog = $this->buildEditDialog();
$edit_dialog->setTitle(pht('Edit Inline Comment'));
$edit_dialog->addHiddenInput('id', $this->getCommentID());
$edit_dialog->addHiddenInput('op', 'edit');
$edit_dialog->appendChild(
$this->renderTextArea(
nonempty($text, $inline->getContent())));
$edit_dialog = $this->buildEditDialog($inline)
->setTitle(pht('Edit Inline Comment'));
$view = $this->buildScaffoldForView($edit_dialog);
return id(new AphrontAjaxResponse())
->setContent($view->render());
case 'create':
$text = $this->getCommentText();
if (!$request->isFormPost() || !strlen($text)) {
return $this->buildEmptyResponse();
}
$inline = $this->createComment()
->setChangesetID($this->getChangesetID())
->setAuthorPHID($viewer->getPHID())
->setLineNumber($this->getLineNumber())
->setLineLength($this->getLineLength())
->setIsNewFile($this->getIsNewFile())
->setContent($text);
if ($this->getReplyToCommentPHID()) {
$inline->setReplyToCommentPHID($this->getReplyToCommentPHID());
}
// If you own this object, mark your own inlines as "Done" by default.
$owner_phid = $this->loadObjectOwnerPHID($inline);
if ($owner_phid) {
if ($viewer->getPHID() == $owner_phid) {
$fixed_state = PhabricatorInlineCommentInterface::STATE_DRAFT;
$inline->setFixedState($fixed_state);
}
}
$this->saveComment($inline);
return $this->buildRenderedCommentResponse(
$inline,
$this->getIsOnRight());
return $this->newInlineResponse($inline, $view);
case 'new':
case 'reply':
default:
$edit_dialog = $this->buildEditDialog();
if ($this->getOperation() == 'reply') {
$edit_dialog->setTitle(pht('Reply to Inline Comment'));
} else {
$edit_dialog->setTitle(pht('New Inline Comment'));
}
// NOTE: We read the values from the client (the display values), not
// the values from the database (the original values) when replying.
// In particular, when replying to a ghost comment which was moved
@ -265,18 +228,38 @@ abstract class PhabricatorInlineCommentController
$number = $this->getLineNumber();
$length = $this->getLineLength();
$edit_dialog->addHiddenInput('op', 'create');
$edit_dialog->addHiddenInput('is_new', $is_new);
$edit_dialog->addHiddenInput('number', $number);
$edit_dialog->addHiddenInput('length', $length);
$inline = $this->createComment()
->setChangesetID($this->getChangesetID())
->setAuthorPHID($viewer->getPHID())
->setIsNewFile($is_new)
->setLineNumber($number)
->setLineLength($length)
->setContent($this->getCommentText())
->setReplyToCommentPHID($this->getReplyToCommentPHID())
->setIsEditing(true);
$text_area = $this->renderTextArea($this->getCommentText());
$edit_dialog->appendChild($text_area);
// If you own this object, mark your own inlines as "Done" by default.
$owner_phid = $this->loadObjectOwnerPHID($inline);
if ($owner_phid) {
if ($viewer->getPHID() == $owner_phid) {
$fixed_state = PhabricatorInlineComment::STATE_DRAFT;
$inline->setFixedState($fixed_state);
}
}
$this->saveComment($inline);
$edit_dialog = $this->buildEditDialog($inline);
if ($this->getOperation() == 'reply') {
$edit_dialog->setTitle(pht('Reply to Inline Comment'));
} else {
$edit_dialog->setTitle(pht('New Inline Comment'));
}
$view = $this->buildScaffoldForView($edit_dialog);
return id(new AphrontAjaxResponse())
->setContent($view->render());
return $this->newInlineResponse($inline, $view);
}
}
@ -320,20 +303,15 @@ abstract class PhabricatorInlineCommentController
}
}
private function buildEditDialog() {
private function buildEditDialog(PhabricatorInlineComment $inline) {
$request = $this->getRequest();
$viewer = $this->getViewer();
$edit_dialog = id(new PHUIDiffInlineCommentEditView())
->setUser($viewer)
->setSubmitURI($request->getRequestURI())
->setViewer($viewer)
->setInlineComment($inline)
->setIsOnRight($this->getIsOnRight())
->setIsNewFile($this->getIsNewFile())
->setNumber($this->getLineNumber())
->setLength($this->getLineLength())
->setRenderer($this->getRenderer())
->setReplyToCommentPHID($this->getReplyToCommentPHID())
->setChangesetID($this->getChangesetID());
->setRenderer($this->getRenderer());
return $edit_dialog;
}
@ -342,12 +320,13 @@ abstract class PhabricatorInlineCommentController
return id(new AphrontAjaxResponse())
->setContent(
array(
'markup' => '',
'inline' => array(),
'view' => null,
));
}
private function buildRenderedCommentResponse(
PhabricatorInlineCommentInterface $inline,
PhabricatorInlineComment $inline,
$on_right) {
$request = $this->getRequest();
@ -357,7 +336,7 @@ abstract class PhabricatorInlineCommentController
$engine->setViewer($viewer);
$engine->addObject(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
$engine->process();
$phids = array($viewer->getPHID());
@ -377,21 +356,7 @@ abstract class PhabricatorInlineCommentController
$view = $this->buildScaffoldForView($view);
return id(new AphrontAjaxResponse())
->setContent(
array(
'inlineCommentID' => $inline->getID(),
'markup' => $view->render(),
));
}
private function renderTextArea($text) {
return id(new PhabricatorRemarkupControl())
->setViewer($this->getViewer())
->setSigil('differential-inline-comment-edit-textarea')
->setName('text')
->setValue($text)
->setDisableFullScreen(true);
return $this->newInlineResponse($inline, $view);
}
private function buildScaffoldForView(PHUIDiffInlineCommentView $view) {
@ -404,4 +369,19 @@ abstract class PhabricatorInlineCommentController
->addRowScaffold($view);
}
private function newInlineResponse(
PhabricatorInlineComment $inline,
$view) {
$response = array(
'inline' => array(
'id' => $inline->getID(),
),
'view' => hsprintf('%s', $view),
);
return id(new AphrontAjaxResponse())
->setContent($response);
}
}

4
src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php
View file

@ -11,14 +11,14 @@ abstract class PhabricatorInlineCommentPreviewController
$viewer = $request->getUser();
$inlines = $this->loadInlineComments();
assert_instances_of($inlines, 'PhabricatorInlineCommentInterface');
assert_instances_of($inlines, 'PhabricatorInlineComment');
$engine = new PhabricatorMarkupEngine();
$engine->setViewer($viewer);
foreach ($inlines as $inline) {
$engine->addObject(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
}
$engine->process();

232
src/infrastructure/diff/interface/PhabricatorInlineComment.php Normal file
View file

@ -0,0 +1,232 @@
<?php
abstract class PhabricatorInlineComment
extends Phobject
implements
PhabricatorMarkupInterface {
const MARKUP_FIELD_BODY = 'markup:body';
const STATE_UNDONE = 'undone';
const STATE_DRAFT = 'draft';
const STATE_UNDRAFT = 'undraft';
const STATE_DONE = 'done';
private $storageObject;
private $syntheticAuthor;
private $isGhost;
public function __clone() {
$this->storageObject = clone $this->storageObject;
}
public function setSyntheticAuthor($synthetic_author) {
$this->syntheticAuthor = $synthetic_author;
return $this;
}
public function getSyntheticAuthor() {
return $this->syntheticAuthor;
}
public function setStorageObject($storage_object) {
$this->storageObject = $storage_object;
return $this;
}
public function getStorageObject() {
if (!$this->storageObject) {
$this->storageObject = $this->newStorageObject();
}
return $this->storageObject;
}
abstract protected function newStorageObject();
abstract public function getControllerURI();
abstract public function setChangesetID($id);
abstract public function getChangesetID();
abstract public function supportsHiding();
abstract public function isHidden();
public function isDraft() {
return !$this->getTransactionPHID();
}
public function getTransactionPHID() {
return $this->getStorageObject()->getTransactionPHID();
}
public function isCompatible(PhabricatorInlineComment $comment) {
return
($this->getAuthorPHID() === $comment->getAuthorPHID()) &&
($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) &&
($this->getContent() === $comment->getContent());
}
public function setIsGhost($is_ghost) {
$this->isGhost = $is_ghost;
return $this;
}
public function getIsGhost() {
return $this->isGhost;
}
public function setContent($content) {
$this->getStorageObject()->setContent($content);
return $this;
}
public function getContent() {
return $this->getStorageObject()->getContent();
}
public function getID() {
return $this->getStorageObject()->getID();
}
public function getPHID() {
return $this->getStorageObject()->getPHID();
}
public function setIsNewFile($is_new) {
$this->getStorageObject()->setIsNewFile($is_new);
return $this;
}
public function getIsNewFile() {
return $this->getStorageObject()->getIsNewFile();
}
public function setFixedState($state) {
$this->getStorageObject()->setFixedState($state);
return $this;
}
public function setHasReplies($has_replies) {
$this->getStorageObject()->setHasReplies($has_replies);
return $this;
}
public function getHasReplies() {
return $this->getStorageObject()->getHasReplies();
}
public function getFixedState() {
return $this->getStorageObject()->getFixedState();
}
public function setLineNumber($number) {
$this->getStorageObject()->setLineNumber($number);
return $this;
}
public function getLineNumber() {
return $this->getStorageObject()->getLineNumber();
}
public function setLineLength($length) {
$this->getStorageObject()->setLineLength($length);
return $this;
}
public function getLineLength() {
return $this->getStorageObject()->getLineLength();
}
public function setAuthorPHID($phid) {
$this->getStorageObject()->setAuthorPHID($phid);
return $this;
}
public function getAuthorPHID() {
return $this->getStorageObject()->getAuthorPHID();
}
public function setReplyToCommentPHID($phid) {
$this->getStorageObject()->setReplyToCommentPHID($phid);
return $this;
}
public function getReplyToCommentPHID() {
return $this->getStorageObject()->getReplyToCommentPHID();
}
public function setIsDeleted($is_deleted) {
$this->getStorageObject()->setIsDeleted($is_deleted);
return $this;
}
public function getIsDeleted() {
return $this->getStorageObject()->getIsDeleted();
}
public function setIsEditing($is_editing) {
$this->getStorageObject()->setAttribute('editing', (bool)$is_editing);
return $this;
}
public function getIsEditing() {
return (bool)$this->getStorageObject()->getAttribute('editing', false);
}
public function getDateModified() {
return $this->getStorageObject()->getDateModified();
}
public function getDateCreated() {
return $this->getStorageObject()->getDateCreated();
}
public function openTransaction() {
$this->getStorageObject()->openTransaction();
}
public function saveTransaction() {
$this->getStorageObject()->saveTransaction();
}
public function save() {
$this->getTransactionCommentForSave()->save();
return $this;
}
public function delete() {
$this->getStorageObject()->delete();
return $this;
}
public function makeEphemeral() {
$this->getStorageObject()->makeEphemeral();
return $this;
}
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
public function getMarkupFieldKey($field) {
$content = $this->getMarkupText($field);
return PhabricatorMarkupEngine::digestRemarkupContent($this, $content);
}
public function newMarkupEngine($field) {
return PhabricatorMarkupEngine::newDifferentialMarkupEngine();
}
public function getMarkupText($field) {
return $this->getContent();
}
public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
return $output;
}
public function shouldUseMarkupCache($field) {
return !$this->isDraft();
}
}

66
src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php
View file

@ -1,66 +0,0 @@
<?php
/**
* Shared interface used by Differential and Diffusion inline comments.
*/
interface PhabricatorInlineCommentInterface extends PhabricatorMarkupInterface {
const MARKUP_FIELD_BODY = 'markup:body';
const STATE_UNDONE = 'undone';
const STATE_DRAFT = 'draft';
const STATE_UNDRAFT = 'undraft';
const STATE_DONE = 'done';
public function setChangesetID($id);
public function getChangesetID();
public function setIsNewFile($is_new);
public function getIsNewFile();
public function setLineNumber($number);
public function getLineNumber();
public function setLineLength($length);
public function getLineLength();
public function setReplyToCommentPHID($phid);
public function getReplyToCommentPHID();
public function setHasReplies($has_replies);
public function getHasReplies();
public function setIsDeleted($deleted);
public function getIsDeleted();
public function setFixedState($state);
public function getFixedState();
public function setContent($content);
public function getContent();
public function setCache($cache);
public function getCache();
public function setAuthorPHID($phid);
public function getAuthorPHID();
public function setSyntheticAuthor($synthetic_author);
public function getSyntheticAuthor();
public function isCompatible(PhabricatorInlineCommentInterface $inline);
public function isDraft();
public function save();
public function delete();
public function setIsGhost($is_ghost);
public function getIsGhost();
public function supportsHiding();
public function isHidden();
public function getDateModified();
public function getDateCreated();
}

67
src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
View file

@ -3,23 +3,16 @@
final class PHUIDiffInlineCommentDetailView
extends PHUIDiffInlineCommentView {
private $inlineComment;
private $handles;
private $markupEngine;
private $editable;
private $preview;
private $allowReply;
private $renderer;
private $canMarkDone;
private $objectOwnerPHID;
public function setInlineComment(PhabricatorInlineCommentInterface $comment) {
$this->inlineComment = $comment;
return $this;
}
public function isHidden() {
return $this->inlineComment->isHidden();
return $this->getInlineComment()->isHidden();
}
public function setHandles(array $handles) {
@ -48,15 +41,6 @@ final class PHUIDiffInlineCommentDetailView
return $this;
}
public function setRenderer($renderer) {
$this->renderer = $renderer;
return $this;
}
public function getRenderer() {
return $this->renderer;
}
public function setCanMarkDone($can_mark_done) {
$this->canMarkDone = $can_mark_done;
return $this;
@ -76,7 +60,7 @@ final class PHUIDiffInlineCommentDetailView
}
public function getAnchorName() {
$inline = $this->inlineComment;
$inline = $this->getInlineComment();
if ($inline->getID()) {
return 'inline-'.$inline->getID();
}
@ -93,49 +77,18 @@ final class PHUIDiffInlineCommentDetailView
public function render() {
require_celerity_resource('phui-inline-comment-view-css');
$inline = $this->inlineComment;
$inline = $this->getInlineComment();
$classes = array(
'differential-inline-comment',
);
$is_fixed = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineCommentInterface::STATE_DONE:
case PhabricatorInlineCommentInterface::STATE_DRAFT:
$is_fixed = true;
break;
}
$is_draft_done = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineCommentInterface::STATE_DRAFT:
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
$is_draft_done = true;
break;
}
$is_synthetic = false;
if ($inline->getSyntheticAuthor()) {
$is_synthetic = true;
}
$metadata = array(
'id' => $inline->getID(),
'phid' => $inline->getPHID(),
'changesetID' => $inline->getChangesetID(),
'number' => $inline->getLineNumber(),
'length' => $inline->getLineLength(),
'isNewFile' => (bool)$inline->getIsNewFile(),
'on_right' => $this->getIsOnRight(),
'original' => $inline->getContent(),
'replyToCommentPHID' => $inline->getReplyToCommentPHID(),
'isDraft' => $inline->isDraft(),
'isFixed' => $is_fixed,
'isGhost' => $inline->getIsGhost(),
'isSynthetic' => $is_synthetic,
'isDraftDone' => $is_draft_done,
);
$metadata = $this->getInlineCommentMetadata();
$sigil = 'differential-inline-comment';
if ($this->preview) {
@ -299,19 +252,19 @@ final class PHUIDiffInlineCommentDetailView
if (!$is_synthetic) {
$draft_state = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineCommentInterface::STATE_DRAFT:
case PhabricatorInlineComment::STATE_DRAFT:
$is_done = $mark_done;
$draft_state = true;
break;
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
case PhabricatorInlineComment::STATE_UNDRAFT:
$is_done = !$mark_done;
$draft_state = true;
break;
case PhabricatorInlineCommentInterface::STATE_DONE:
case PhabricatorInlineComment::STATE_DONE:
$is_done = true;
break;
default:
case PhabricatorInlineCommentInterface::STATE_UNDONE:
case PhabricatorInlineComment::STATE_UNDONE:
$is_done = false;
break;
}
@ -372,7 +325,7 @@ final class PHUIDiffInlineCommentDetailView
$content = $this->markupEngine->getOutput(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
if ($this->preview) {
$anchor = null;
@ -490,7 +443,7 @@ final class PHUIDiffInlineCommentDetailView
}
private function canHide() {
$inline = $this->inlineComment;
$inline = $this->getInlineComment();
if ($inline->isDraft()) {
return false;

116
src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php
View file

@ -3,90 +3,23 @@
final class PHUIDiffInlineCommentEditView
extends PHUIDiffInlineCommentView {
private $inputs = array();
private $uri;
private $title;
private $number;
private $length;
private $renderer;
private $isNewFile;
private $replyToCommentPHID;
private $changesetID;
public function setIsNewFile($is_new_file) {
$this->isNewFile = $is_new_file;
return $this;
}
public function getIsNewFile() {
return $this->isNewFile;
}
public function setRenderer($renderer) {
$this->renderer = $renderer;
return $this;
}
public function getRenderer() {
return $this->renderer;
}
public function addHiddenInput($key, $value) {
$this->inputs[] = array($key, $value);
return $this;
}
public function setSubmitURI($uri) {
$this->uri = $uri;
return $this;
}
public function setTitle($title) {
$this->title = $title;
return $this;
}
public function setReplyToCommentPHID($reply_to_phid) {
$this->replyToCommentPHID = $reply_to_phid;
return $this;
}
public function getReplyToCommentPHID() {
return $this->replyToCommentPHID;
}
public function setChangesetID($changeset_id) {
$this->changesetID = $changeset_id;
return $this;
}
public function getChangesetID() {
return $this->changesetID;
}
public function setNumber($number) {
$this->number = $number;
return $this;
}
public function setLength($length) {
$this->length = $length;
return $this;
}
public function render() {
if (!$this->uri) {
throw new PhutilInvalidStateException('setSubmitURI');
}
$viewer = $this->getViewer();
$inline = $this->getInlineComment();
$content = phabricator_form(
$viewer,
array(
'action' => $this->uri,
'method' => 'POST',
'sigil' => 'inline-edit-form',
'action' => $inline->getControllerURI(),
'method' => 'POST',
'sigil' => 'inline-edit-form',
),
array(
$this->renderInputs(),
@ -97,13 +30,16 @@ final class PHUIDiffInlineCommentEditView
}
private function renderInputs() {
$inputs = $this->inputs;
$out = array();
$inputs = array();
$inline = $this->getInlineComment();
$inputs[] = array('on_right', (bool)$this->getIsOnRight());
$inputs[] = array('replyToCommentPHID', $this->getReplyToCommentPHID());
$inputs[] = array('op', 'edit');
$inputs[] = array('id', $inline->getID());
$inputs[] = array('on_right', $this->getIsOnRight());
$inputs[] = array('renderer', $this->getRenderer());
$inputs[] = array('changesetID', $this->getChangesetID());
$out = array();
foreach ($inputs as $input) {
list($name, $value) = $input;
@ -115,6 +51,7 @@ final class PHUIDiffInlineCommentEditView
'value' => $value,
));
}
return $out;
}
@ -141,7 +78,7 @@ final class PHUIDiffInlineCommentEditView
array(
'class' => 'differential-inline-comment-edit-body',
),
$this->renderChildren());
$this->newTextarea());
$edit = phutil_tag(
'div',
@ -152,19 +89,14 @@ final class PHUIDiffInlineCommentEditView
$buttons,
));
$inline = $this->getInlineComment();
return javelin_tag(
'div',
array(
'class' => 'differential-inline-comment-edit',
'sigil' => 'differential-inline-comment',
'meta' => array(
'changesetID' => $this->getChangesetID(),
'on_right' => $this->getIsOnRight(),
'isNewFile' => (bool)$this->getIsNewFile(),
'number' => $this->number,
'length' => $this->length,
'replyToCommentPHID' => $this->getReplyToCommentPHID(),
),
'meta' => $this->getInlineCommentMetadata(),
),
array(
$title,
@ -173,4 +105,18 @@ final class PHUIDiffInlineCommentEditView
));
}
private function newTextarea() {
$viewer = $this->getViewer();
$inline = $this->getInlineComment();
$text = $inline->getContent();
return id(new PhabricatorRemarkupControl())
->setViewer($viewer)
->setSigil('differential-inline-comment-edit-textarea')
->setName('text')
->setValue($text)
->setDisableFullScreen(true);
}
}

2
src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php
View file

@ -54,7 +54,7 @@ final class PHUIDiffInlineCommentPreviewListView
foreach ($inlines as $inline) {
$engine->addObject(
$inline,
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
PhabricatorInlineComment::MARKUP_FIELD_BODY);
}
$engine->process();

62
src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
View file

@ -3,6 +3,17 @@
abstract class PHUIDiffInlineCommentView extends AphrontView {
private $isOnRight;
private $renderer;
private $inlineComment;
public function setInlineComment(PhabricatorInlineComment $comment) {
$this->inlineComment = $comment;
return $this;
}
public function getInlineComment() {
return $this->inlineComment;
}
public function getIsOnRight() {
return $this->isOnRight;
@ -13,6 +24,15 @@ abstract class PHUIDiffInlineCommentView extends AphrontView {
return $this;
}
public function setRenderer($renderer) {
$this->renderer = $renderer;
return $this;
}
public function getRenderer() {
return $this->renderer;
}
public function getScaffoldCellID() {
return null;
}
@ -33,4 +53,46 @@ abstract class PHUIDiffInlineCommentView extends AphrontView {
}
}
protected function getInlineCommentMetadata() {
$inline = $this->getInlineComment();
$is_synthetic = (bool)$inline->getSyntheticAuthor();
$is_fixed = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineComment::STATE_DONE:
case PhabricatorInlineComment::STATE_DRAFT:
$is_fixed = true;
break;
}
$is_draft_done = false;
switch ($inline->getFixedState()) {
case PhabricatorInlineComment::STATE_DRAFT:
case PhabricatorInlineComment::STATE_UNDRAFT:
$is_draft_done = true;
break;
}
return array(
'id' => $inline->getID(),
'phid' => $inline->getPHID(),
'changesetID' => $inline->getChangesetID(),
'number' => $inline->getLineNumber(),
'length' => $inline->getLineLength(),
'isNewFile' => (bool)$inline->getIsNewFile(),
'original' => $inline->getContent(),
'replyToCommentPHID' => $inline->getReplyToCommentPHID(),
'isDraft' => $inline->isDraft(),
'isFixed' => $is_fixed,
'isGhost' => $inline->getIsGhost(),
'isSynthetic' => $is_synthetic,
'isDraftDone' => $is_draft_done,
'isEditing' => $inline->getIsEditing(),
'on_right' => $this->getIsOnRight(),
);
}
}

183
webroot/rsrc/js/application/diff/DiffChangesetList.js
View file

@ -26,30 +26,6 @@ JX.install('DiffChangesetList', {
var onexpand = JX.bind(this, this._ifawake, this._oncollapse, false);
JX.Stratcom.listen('click', 'reveal-inline', onexpand);
var onedit = JX.bind(this, this._ifawake, this._onaction, 'edit');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-edit'],
onedit);
var ondone = JX.bind(this, this._ifawake, this._onaction, 'done');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-done'],
ondone);
var ondelete = JX.bind(this, this._ifawake, this._onaction, 'delete');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-delete'],
ondelete);
var onreply = JX.bind(this, this._ifawake, this._onaction, 'reply');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-reply'],
onreply);
var onresize = JX.bind(this, this._ifawake, this._onresize);
JX.Stratcom.listen('resize', null, onresize);
@ -85,6 +61,8 @@ JX.install('DiffChangesetList', {
'mouseup',
null,
onrangeup);
this._setupInlineCommentListeners();
},
properties: {
@ -1163,56 +1141,6 @@ JX.install('DiffChangesetList', {
}
},
_onaction: function(action, e) {
e.kill();
var inline = this._getInlineForEvent(e);
var is_ref = false;
// If we don't have a natural inline object, the user may have clicked
// an action (like "Delete") inside a preview element at the bottom of
// the page.
// If they did, try to find an associated normal inline to act on, and
// pretend they clicked that instead. This makes the overall state of
// the page more consistent.
// However, there may be no normal inline (for example, because it is
// on a version of the diff which is not visible). In this case, we
// act by reference.
if (inline === null) {
var data = e.getNodeData('differential-inline-comment');
inline = this.getInlineByID(data.id);
if (inline) {
is_ref = true;
} else {
switch (action) {
case 'delete':
this._deleteInlineByID(data.id);
return;
}
}
}
// TODO: For normal operations, highlight the inline range here.
switch (action) {
case 'edit':
inline.edit();
break;
case 'done':
inline.toggleDone();
break;
case 'delete':
inline.delete(is_ref);
break;
case 'reply':
inline.reply();
break;
}
},
redrawPreview: function() {
// TODO: This isn't the cleanest way to find the preview form, but
// rendering no longer has direct access to it.
@ -2138,6 +2066,113 @@ JX.install('DiffChangesetList', {
var tree = this._getTreeView();
JX.DOM.setContent(flank_body, tree.getNode());
},
_setupInlineCommentListeners: function() {
var onsave = JX.bind(this, this._onInlineEvent, 'save');
JX.Stratcom.listen(
['submit', 'didSyntheticSubmit'],
'inline-edit-form',
onsave);
var oncancel = JX.bind(this, this._onInlineEvent, 'cancel');
JX.Stratcom.listen(
'click',
'inline-edit-cancel',
oncancel);
var onundo = JX.bind(this, this._onInlineEvent, 'undo');
JX.Stratcom.listen(
'click',
'differential-inline-comment-undo',
onundo);
var onedit = JX.bind(this, this._onInlineEvent, 'edit');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-edit'],
onedit);
var ondone = JX.bind(this, this._onInlineEvent, 'done');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-done'],
ondone);
var ondelete = JX.bind(this, this._onInlineEvent, 'delete');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-delete'],
ondelete);
var onreply = JX.bind(this, this._onInlineEvent, 'reply');
JX.Stratcom.listen(
'click',
['differential-inline-comment', 'differential-inline-reply'],
onreply);
},
_onInlineEvent: function(action, e) {
if (this.isAsleep()) {
return;
}
e.kill();
var inline = this._getInlineForEvent(e);
var is_ref = false;
// If we don't have a natural inline object, the user may have clicked
// an action (like "Delete") inside a preview element at the bottom of
// the page.
// If they did, try to find an associated normal inline to act on, and
// pretend they clicked that instead. This makes the overall state of
// the page more consistent.
// However, there may be no normal inline (for example, because it is
// on a version of the diff which is not visible). In this case, we
// act by reference.
if (inline === null) {
var data = e.getNodeData('differential-inline-comment');
inline = this.getInlineByID(data.id);
if (inline) {
is_ref = true;
} else {
switch (action) {
case 'delete':
this._deleteInlineByID(data.id);
return;
}
}
}
// TODO: For normal operations, highlight the inline range here.
switch (action) {
case 'save':
inline.save(e.getTarget());
break;
case 'cancel':
inline.cancel();
break;
case 'undo':
inline.undo();
break;
case 'edit':
inline.edit();
break;
case 'done':
inline.toggleDone();
break;
case 'delete':
inline.delete(is_ref);
break;
case 'reply':
inline.reply();
break;
}
}
}

142
webroot/rsrc/js/application/diff/DiffInline.js
View file

@ -18,7 +18,6 @@ JX.install('DiffInline', {
_length: null,
_displaySide: null,
_isNewFile: null,
_undoRow: null,
_replyToCommentPHID: null,
_originalText: null,
_snippet: null,
@ -38,6 +37,11 @@ JX.install('DiffInline', {
_isSynthetic: false,
_isHidden: false,
_editRow: null,
_undoRow: null,
_undoType: null,
_undoText: null,
bindToRow: function(row) {
this._row = row;
@ -50,13 +54,10 @@ JX.install('DiffInline', {
var comment = JX.DOM.find(row, 'div', 'differential-inline-comment');
var data = JX.Stratcom.getData(comment);
this._id = data.id;
this._readInlineState(data);
this._phid = data.phid;
// TODO: This is very, very, very, very, very, very, very hacky.
var td = comment.parentNode;
var th = td.previousSibling;
if (th.parentNode.firstChild != th) {
if (data.on_right) {
this._displaySide = 'right';
} else {
this._displaySide = 'left';
@ -65,9 +66,7 @@ JX.install('DiffInline', {
this._number = parseInt(data.number, 10);
this._length = parseInt(data.length, 10);
this._originalText = data.original;
this._isNewFile =
(this.getDisplaySide() == 'right') ||
(data.left != data.right);
this._isNewFile = data.isNewFile;
this._replyToCommentPHID = data.replyToCommentPHID;
@ -81,7 +80,13 @@ JX.install('DiffInline', {
this._isNew = false;
this._snippet = data.snippet;
this.setInvisible(false);
this._isEditing = data.isEditing;
if (this._isEditing) {
this.edit();
} else {
this.setInvisible(false);
}
return this;
},
@ -397,6 +402,12 @@ JX.install('DiffInline', {
op = 'delete';
}
// If there's an existing "unedit" undo element, remove it.
if (this._undoRow) {
JX.DOM.remove(this._undoRow);
this._undoRow = null;
}
var data = this._newRequestData(op);
this.setLoading(true);
@ -472,8 +483,9 @@ JX.install('DiffInline', {
},
_oneditresponse: function(response) {
var rows = JX.$H(response).getNode();
var rows = JX.$H(response.view).getNode();
this._readInlineState(response.inline);
this._drawEditRows(rows);
this.setLoading(false);
@ -481,11 +493,16 @@ JX.install('DiffInline', {
},
_oncreateresponse: function(response) {
var rows = JX.$H(response).getNode();
var rows = JX.$H(response.view).getNode();
this._readInlineState(response.inline);
this._drawEditRows(rows);
},
_readInlineState: function(state) {
this._id = state.id;
},
_ondeleteresponse: function() {
this._drawUndeleteRows();
@ -496,10 +513,16 @@ JX.install('DiffInline', {
},
_drawUndeleteRows: function() {
this._undoType = 'undelete';
this._undoText = null;
return this._drawUndoRows('undelete', this._row);
},
_drawUneditRows: function(text) {
this._undoType = 'unedit';
this._undoText = text;
return this._drawUndoRows('unedit', null, text);
},
@ -523,16 +546,17 @@ JX.install('DiffInline', {
_drawEditRows: function(rows) {
this.setEditing(true);
return this._drawRows(rows, null, 'edit');
this._editRow = this._drawRows(rows, null, 'edit');
},
_drawRows: function(rows, cursor, type, text) {
var first_row = JX.DOM.scry(rows, 'tr')[0];
var first_meta;
var row = first_row;
var anchor = cursor || this._row;
cursor = cursor || this._row.nextSibling;
var result_row;
var next_row;
while (row) {
// Grab this first, since it's going to change once we insert the row
@ -546,40 +570,8 @@ JX.install('DiffInline', {
anchor.parentNode.insertBefore(row, cursor);
cursor = row;
var row_meta = {
node: row,
type: type,
text: text || null,
listeners: []
};
if (!first_meta) {
first_meta = row_meta;
}
if (type == 'edit') {
row_meta.listeners.push(
JX.DOM.listen(
row,
['submit', 'didSyntheticSubmit'],
'inline-edit-form',
JX.bind(this, this._onsubmit, row_meta)));
row_meta.listeners.push(
JX.DOM.listen(
row,
'click',
'inline-edit-cancel',
JX.bind(this, this._oncancel, row_meta)));
} else if (type == 'content') {
// No special listeners for these rows.
} else {
row_meta.listeners.push(
JX.DOM.listen(
row,
'click',
'differential-inline-comment-undo',
JX.bind(this, this._onundo, row_meta)));
if (!result_row) {
result_row = row;
}
// If the row has a textarea, focus it. This allows the user to start
@ -602,27 +594,25 @@ JX.install('DiffInline', {
JX.Stratcom.invoke('resize');
return first_meta;
return result_row;
},
_onsubmit: function(row, e) {
e.kill();
var handler = JX.bind(this, this._onsubmitresponse, row);
save: function(form) {
var handler = JX.bind(this, this._onsubmitresponse);
this.setLoading(true);
JX.Workflow.newFromForm(e.getTarget())
JX.Workflow.newFromForm(form)
.setHandler(handler)
.start();
},
_onundo: function(row, e) {
e.kill();
undo: function() {
this._removeRow(row);
JX.DOM.remove(this._undoRow);
this._undoRow = null;
if (row.type == 'undelete') {
if (this._undoType === 'undelete') {
var uri = this._getInlineURI();
var data = this._newRequestData('undelete');
var handler = JX.bind(this, this._onundelete);
@ -635,13 +625,10 @@ JX.install('DiffInline', {
.send();
}
if (row.type == 'unedit') {
if (this.getID()) {
this.edit(row.text);
} else {
this.create(row.text);
}
if (this._undoType === 'unedit') {
this.edit(this._undoText);
}
},
_onundelete: function() {
@ -649,15 +636,16 @@ JX.install('DiffInline', {
this._didUpdate();
},
_oncancel: function(row, e) {
e.kill();
cancel: function() {
var text = this._readText(this._editRow);
JX.DOM.remove(this._editRow);
this._editRow = null;
var text = this._readText(row.node);
if (text && text.length && (text != this._originalText)) {
this._drawUneditRows(text);
}
this._removeRow(row);
this.setEditing(false);
this.setInvisible(false);
@ -679,8 +667,11 @@ JX.install('DiffInline', {
return textarea.value;
},
_onsubmitresponse: function(row, response) {
this._removeRow(row);
_onsubmitresponse: function(response) {
if (this._editRow) {
JX.DOM.remove(this._editRow);
this._editRow = null;
}
this.setLoading(false);
this.setInvisible(false);
@ -691,8 +682,8 @@ JX.install('DiffInline', {
_onupdate: function(response) {
var new_row;
if (response.markup) {
new_row = this._drawContentRows(JX.$H(response.markup).getNode()).node;
if (response.view) {
new_row = this._drawContentRows(JX.$H(response.view).getNode());
}
// TODO: Save the old row so the action it's undo-able if it was a
@ -741,13 +732,6 @@ JX.install('DiffInline', {
JX.DOM.alterClass(row, 'inline-hidden', is_collapsed);
},
_removeRow: function(row) {
JX.DOM.remove(row.node);
for (var ii = 0; ii < row.listeners.length; ii++) {
row.listeners[ii].remove();
}
},
_getInlineURI: function() {
var changeset = this.getChangeset();
var list = changeset.getChangesetList();