diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ '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 @@ '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 @@ '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 @@ 'javelin-workflow', 'phuix-icon-view', ), + '10726e6a' => array( + 'javelin-install', + 'phuix-button-view', + 'phabricator-diff-tree-view', + ), '111bfd2d' => array( 'javelin-install', ), @@ -1036,9 +1041,6 @@ 'javelin-stratcom', 'javelin-util', ), - '16e97ebc' => array( - 'javelin-dom', - ), '1a844c06' => array( 'javelin-install', 'javelin-util', @@ -1614,6 +1616,9 @@ 'phabricator-drag-and-drop-file-upload', 'phabricator-textareautils', ), + '7b0bdd6d' => array( + 'javelin-dom', + ), '7b139193' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1930,11 +1935,6 @@ 'javelin-typeahead-ondemand-source', 'javelin-util', ), - 'adf069cd' => array( - 'javelin-install', - 'phuix-button-view', - 'phabricator-diff-tree-view', - ), 'aec8e38c' => array( 'javelin-dom', 'javelin-util', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3592,8 +3592,8 @@ '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 @@ 'DifferentialHunkParserTestCase' => 'PhabricatorTestCase', 'DifferentialHunkQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialHunkTestCase' => 'PhutilTestCase', - 'DifferentialInlineComment' => array( - 'Phobject', - 'PhabricatorInlineCommentInterface', - ), + 'DifferentialInlineComment' => 'PhabricatorInlineComment', 'DifferentialInlineCommentEditController' => 'PhabricatorInlineCommentController', 'DifferentialInlineCommentMailView' => 'DifferentialMailView', 'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery', @@ -8595,10 +8592,7 @@ 'PhabricatorAuditCommentEditor' => 'PhabricatorEditor', 'PhabricatorAuditController' => 'PhabricatorController', 'PhabricatorAuditEditor' => 'PhabricatorApplicationTransactionEditor', - 'PhabricatorAuditInlineComment' => array( - 'Phobject', - 'PhabricatorInlineCommentInterface', - ), + 'PhabricatorAuditInlineComment' => 'PhabricatorInlineComment', 'PhabricatorAuditListView' => 'AphrontView', 'PhabricatorAuditMailReceiver' => 'PhabricatorObjectMailReceiver', 'PhabricatorAuditManagementDeleteWorkflow' => 'PhabricatorAuditManagementWorkflow', @@ -10115,8 +10109,11 @@ 'PhabricatorIndexEngineExtension' => 'Phobject', 'PhabricatorIndexEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorInfrastructureTestCase' => 'PhabricatorTestCase', + 'PhabricatorInlineComment' => array( + 'Phobject', + 'PhabricatorMarkupInterface', + ), 'PhabricatorInlineCommentController' => 'PhabricatorController', - 'PhabricatorInlineCommentInterface' => 'PhabricatorMarkupInterface', 'PhabricatorInlineCommentPreviewController' => 'PhabricatorController', 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInstructionsEditField' => 'PhabricatorEditField', diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -1,27 +1,16 @@ proxy = new PhabricatorAuditTransactionComment(); - } - - public function __clone() { - $this->proxy = clone $this->proxy; + protected function newStorageObject() { + return new PhabricatorAuditTransactionComment(); } - 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 @@ $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 @@ 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 @@ 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()); - } - } diff --git a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php --- a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php +++ b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php @@ -72,4 +72,13 @@ 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; + } + } diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -238,7 +238,7 @@ foreach ($inlines as $inline) { $engine->addObject( $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + PhabricatorInlineComment::MARKUP_FIELD_BODY); } $engine->process(); diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -80,9 +80,15 @@ $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 @@ return true; } - protected function deleteComment(PhabricatorInlineCommentInterface $inline) { + protected function deleteComment(PhabricatorInlineComment $inline) { $inline->openTransaction(); $inline->setIsDeleted(1)->save(); $this->syncDraft(); @@ -169,14 +175,14 @@ } 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 @@ } protected function loadObjectOwnerPHID( - PhabricatorInlineCommentInterface $inline) { + PhabricatorInlineComment $inline) { return $this->loadRevision()->getAuthorPHID(); } diff --git a/src/applications/differential/controller/DifferentialRevisionInlinesController.php b/src/applications/differential/controller/DifferentialRevisionInlinesController.php --- a/src/applications/differential/controller/DifferentialRevisionInlinesController.php +++ b/src/applications/differential/controller/DifferentialRevisionInlinesController.php @@ -124,7 +124,7 @@ $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'); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -354,7 +354,7 @@ } public function parseInlineComment( - PhabricatorInlineCommentInterface $comment) { + PhabricatorInlineComment $comment) { // Parse only comments which are actually visible. if ($this->isCommentVisibleOnRenderedDiff($comment)) { @@ -1191,11 +1191,11 @@ * 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 @@ * 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!')); diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -464,19 +464,19 @@ } 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()) diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -263,7 +263,7 @@ 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 @@ 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; diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php --- a/src/applications/differential/storage/DifferentialInlineComment.php +++ b/src/applications/differential/storage/DifferentialInlineComment.php @@ -1,53 +1,30 @@ 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 @@ 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 @@ } public function getRevisionID() { - $phid = $this->proxy->getRevisionPHID(); + $phid = $this->getStorageObject()->getRevisionPHID(); if (!$phid) { return null; } @@ -194,98 +90,4 @@ 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()); - } - } diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php --- a/src/applications/differential/storage/DifferentialTransactionComment.php +++ b/src/applications/differential/storage/DifferentialTransactionComment.php @@ -118,4 +118,13 @@ 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; + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionInlineTransaction.php b/src/applications/differential/xaction/DifferentialRevisionInlineTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionInlineTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionInlineTransaction.php @@ -40,8 +40,8 @@ $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; } diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -120,7 +120,7 @@ foreach ($inlines as $inline) { $engine->addObject( $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + PhabricatorInlineComment::MARKUP_FIELD_BODY); } $engine->process(); diff --git a/src/applications/diffusion/controller/DiffusionInlineCommentController.php b/src/applications/diffusion/controller/DiffusionInlineCommentController.php --- a/src/applications/diffusion/controller/DiffusionInlineCommentController.php +++ b/src/applications/diffusion/controller/DiffusionInlineCommentController.php @@ -105,7 +105,7 @@ } // Saved comments may not be edited. - if ($inline->getAuditCommentID()) { + if ($inline->getTransactionPHID()) { return false; } @@ -117,21 +117,21 @@ 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(); } diff --git a/src/applications/transactions/constants/PhabricatorTransactions.php b/src/applications/transactions/constants/PhabricatorTransactions.php --- a/src/applications/transactions/constants/PhabricatorTransactions.php +++ b/src/applications/transactions/constants/PhabricatorTransactions.php @@ -32,10 +32,10 @@ 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, ); } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -1690,7 +1690,7 @@ $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 diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -9,13 +9,13 @@ 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 @@ $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 @@ if ($request->isFormPost()) { if (strlen($text)) { - $inline->setContent($text); + $inline + ->setContent($text) + ->setIsEditing(false); + $this->saveComment($inline); return $this->buildRenderedCommentResponse( $inline, @@ -196,58 +199,57 @@ $this->deleteComment($inline); return $this->buildEmptyResponse(); } - } + } else { + $inline->setIsEditing(true); - $edit_dialog = $this->buildEditDialog(); - $edit_dialog->setTitle(pht('Edit Inline Comment')); + if (strlen($text)) { + $inline->setContent($text); + } - $edit_dialog->addHiddenInput('id', $this->getCommentID()); - $edit_dialog->addHiddenInput('op', 'edit'); + $this->saveComment($inline); + } - $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(); - } + return $this->newInlineResponse($inline, $view); + case 'new': + case 'reply': + default: + // 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 + // across diffs and then moved backward to the most recent visible + // line, we want to reply on the display line (which exists), not on + // the comment's original line (which may not exist in this changeset). + $is_new = $this->getIsNewFile(); + $number = $this->getLineNumber(); + $length = $this->getLineLength(); $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()); - } + ->setIsNewFile($is_new) + ->setLineNumber($number) + ->setLineLength($length) + ->setContent($this->getCommentText()) + ->setReplyToCommentPHID($this->getReplyToCommentPHID()) + ->setIsEditing(true); // 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; + $fixed_state = PhabricatorInlineComment::STATE_DRAFT; $inline->setFixedState($fixed_state); } } $this->saveComment($inline); - return $this->buildRenderedCommentResponse( - $inline, - $this->getIsOnRight()); - case 'reply': - default: - $edit_dialog = $this->buildEditDialog(); + $edit_dialog = $this->buildEditDialog($inline); if ($this->getOperation() == 'reply') { $edit_dialog->setTitle(pht('Reply to Inline Comment')); @@ -255,28 +257,9 @@ $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 - // across diffs and then moved backward to the most recent visible - // line, we want to reply on the display line (which exists), not on - // the comment's original line (which may not exist in this changeset). - $is_new = $this->getIsNewFile(); - $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); - - $text_area = $this->renderTextArea($this->getCommentText()); - $edit_dialog->appendChild($text_area); - $view = $this->buildScaffoldForView($edit_dialog); - return id(new AphrontAjaxResponse()) - ->setContent($view->render()); + return $this->newInlineResponse($inline, $view); } } @@ -320,20 +303,15 @@ } } - 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 @@ 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 @@ $engine->setViewer($viewer); $engine->addObject( $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + PhabricatorInlineComment::MARKUP_FIELD_BODY); $engine->process(); $phids = array($viewer->getPHID()); @@ -377,21 +356,7 @@ $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 @@ ->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); + } + } diff --git a/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php b/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php --- a/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php @@ -11,14 +11,14 @@ $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(); diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php new file mode 100644 --- /dev/null +++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php @@ -0,0 +1,232 @@ +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(); + } + +} diff --git a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php b/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php deleted file mode 100644 --- a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php +++ /dev/null @@ -1,66 +0,0 @@ -inlineComment = $comment; - return $this; - } - public function isHidden() { - return $this->inlineComment->isHidden(); + return $this->getInlineComment()->isHidden(); } public function setHandles(array $handles) { @@ -48,15 +41,6 @@ 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 @@ } public function getAnchorName() { - $inline = $this->inlineComment; + $inline = $this->getInlineComment(); if ($inline->getID()) { return 'inline-'.$inline->getID(); } @@ -93,49 +77,18 @@ 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 @@ 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 @@ $content = $this->markupEngine->getOutput( $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + PhabricatorInlineComment::MARKUP_FIELD_BODY); if ($this->preview) { $anchor = null; @@ -490,7 +443,7 @@ } private function canHide() { - $inline = $this->inlineComment; + $inline = $this->getInlineComment(); if ($inline->isDraft()) { return false; diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php @@ -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 @@ } private function renderInputs() { - $inputs = $this->inputs; - $out = array(); + $inputs = array(); + $inline = $this->getInlineComment(); + + $inputs[] = array('op', 'edit'); + $inputs[] = array('id', $inline->getID()); - $inputs[] = array('on_right', (bool)$this->getIsOnRight()); - $inputs[] = array('replyToCommentPHID', $this->getReplyToCommentPHID()); + $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 @@ 'value' => $value, )); } + return $out; } @@ -141,7 +78,7 @@ array( 'class' => 'differential-inline-comment-edit-body', ), - $this->renderChildren()); + $this->newTextarea()); $edit = phutil_tag( 'div', @@ -152,19 +89,14 @@ $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 @@ )); } + 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); + } + } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php @@ -54,7 +54,7 @@ foreach ($inlines as $inline) { $engine->addObject( $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + PhabricatorInlineComment::MARKUP_FIELD_BODY); } $engine->process(); diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php @@ -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 @@ 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 @@ } } + 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(), + ); + } + + } diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -26,30 +26,6 @@ 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 @@ 'mouseup', null, onrangeup); + + this._setupInlineCommentListeners(); }, properties: { @@ -1163,56 +1141,6 @@ } }, - _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 @@ 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; + } } } diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -18,7 +18,6 @@ _length: null, _displaySide: null, _isNewFile: null, - _undoRow: null, _replyToCommentPHID: null, _originalText: null, _snippet: null, @@ -38,6 +37,11 @@ _isSynthetic: false, _isHidden: false, + _editRow: null, + _undoRow: null, + _undoType: null, + _undoText: null, + bindToRow: function(row) { this._row = row; @@ -50,13 +54,10 @@ 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 @@ 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 @@ 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 @@ 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 @@ }, _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 @@ }, _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 @@ }, _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 @@ _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 @@ 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.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 @@ .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 @@ 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 @@ 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 @@ _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.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();