Page MenuHomePhabricator
Files F14081193

D21235.id50571.diff
No OneTemporary
Actions

D21235.id50571.diff
View Options

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
@@ -7,6 +7,10 @@
return new DifferentialDiffInlineCommentQuery();
}
+ protected function newContainerObject() {
+ return $this->loadRevision();
+ }
+
private function getRevisionID() {
return $this->getRequest()->getURIData('id');
}
@@ -137,28 +141,6 @@
return true;
}
- protected function deleteComment(PhabricatorInlineComment $inline) {
- $inline->openTransaction();
- $inline->setIsDeleted(1)->save();
- $this->syncDraft();
- $inline->saveTransaction();
- }
-
- protected function undeleteComment(
- PhabricatorInlineComment $inline) {
- $inline->openTransaction();
- $inline->setIsDeleted(0)->save();
- $this->syncDraft();
- $inline->saveTransaction();
- }
-
- protected function saveComment(PhabricatorInlineComment $inline) {
- $inline->openTransaction();
- $inline->save();
- $this->syncDraft();
- $inline->saveTransaction();
- }
-
protected function loadObjectOwnerPHID(
PhabricatorInlineComment $inline) {
return $this->loadRevision()->getAuthorPHID();
@@ -198,14 +180,4 @@
$ids);
}
- private function syncDraft() {
- $viewer = $this->getViewer();
- $revision = $this->loadRevision();
-
- $revision->newDraftEngine()
- ->setObject($revision)
- ->setViewer($viewer)
- ->synchronize();
- }
-
}
diff --git a/src/applications/differential/engine/DifferentialRevisionDraftEngine.php b/src/applications/differential/engine/DifferentialRevisionDraftEngine.php
--- a/src/applications/differential/engine/DifferentialRevisionDraftEngine.php
+++ b/src/applications/differential/engine/DifferentialRevisionDraftEngine.php
@@ -11,6 +11,7 @@
->setViewer($viewer)
->withRevisionPHIDs(array($revision->getPHID()))
->withPublishableComments(true)
+ ->setLimit(1)
->execute();
return (bool)$inlines;
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
@@ -7,6 +7,10 @@
return new DiffusionDiffInlineCommentQuery();
}
+ protected function newContainerObject() {
+ return $this->loadCommit();
+ }
+
private function getCommitPHID() {
return $this->getRequest()->getURIData('phid');
}
@@ -103,19 +107,6 @@
return true;
}
- protected function deleteComment(PhabricatorInlineComment $inline) {
- $inline->setIsDeleted(1)->save();
- }
-
- protected function undeleteComment(
- PhabricatorInlineComment $inline) {
- $inline->setIsDeleted(0)->save();
- }
-
- protected function saveComment(PhabricatorInlineComment $inline) {
- return $inline->save();
- }
-
protected function loadObjectOwnerPHID(
PhabricatorInlineComment $inline) {
return $this->loadCommit()->getAuthorPHID();
diff --git a/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php b/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php
@@ -11,6 +11,7 @@
->setViewer($viewer)
->withCommitPHIDs(array($commit->getPHID()))
->withPublishableComments(true)
+ ->setLimit(1)
->execute();
return (bool)$inlines;
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
@@ -3,17 +3,28 @@
abstract class PhabricatorInlineCommentController
extends PhabricatorController {
+ private $containerObject;
+
abstract protected function createComment();
abstract protected function newInlineCommentQuery();
abstract protected function loadCommentForDone($id);
abstract protected function loadObjectOwnerPHID(
PhabricatorInlineComment $inline);
- abstract protected function deleteComment(
- PhabricatorInlineComment $inline);
- abstract protected function undeleteComment(
- PhabricatorInlineComment $inline);
- abstract protected function saveComment(
- PhabricatorInlineComment $inline);
+ abstract protected function newContainerObject();
+
+ final protected function getContainerObject() {
+ if ($this->containerObject === null) {
+ $object = $this->newContainerObject();
+ if (!$object) {
+ throw new Exception(
+ pht(
+ 'Failed to load container object for inline comment.'));
+ }
+ $this->containerObject = $object;
+ }
+
+ return $this->containerObject;
+ }
protected function hideComments(array $ids) {
throw new PhutilMethodNotImplementedException();
@@ -173,11 +184,13 @@
$inline = $this->loadCommentByIDForEdit($this->getCommentID());
if ($is_delete) {
- $this->deleteComment($inline);
+ $inline->setIsDeleted(1);
} else {
- $this->undeleteComment($inline);
+ $inline->setIsDeleted(0);
}
+ $this->saveComment($inline);
+
return $this->buildEmptyResponse();
case 'edit':
$inline = $this->loadCommentByIDForEdit($this->getCommentID());
@@ -190,33 +203,49 @@
->setIsEditing(false);
$this->saveComment($inline);
- $this->purgeVersionedDrafts($inline);
return $this->buildRenderedCommentResponse(
$inline,
$this->getIsOnRight());
} else {
- $this->deleteComment($inline);
- $this->purgeVersionedDrafts($inline);
+ $inline->setIsDeleted(1);
+
+ $this->saveComment($inline);
return $this->buildEmptyResponse();
}
} else {
- $inline->setIsEditing(true);
+ // NOTE: At time of writing, the "editing" state of inlines is
+ // preserved by simluating a click on "Edit" when the inline loads.
- if (strlen($text)) {
- $inline->setContent($text);
- }
+ // In this case, we don't want to "saveComment()", because it
+ // recalculates object drafts and purges versioned drafts.
+
+ // The recalculation is merely unnecessary (state doesn't change)
+ // but purging drafts means that loading a page and then closing it
+ // discards your drafts.
- $this->saveComment($inline);
+ // To avoid the purge, only invoke "saveComment()" if we actually
+ // have changes to apply.
+
+ $is_dirty = false;
+ if (!$inline->getIsEditing()) {
+ $inline->setIsEditing(true);
+ $is_dirty = true;
+ }
if (strlen($text)) {
- $this->purgeVersionedDrafts($inline);
+ $inline->setContent($text);
+ $is_dirty = true;
+ } else {
+ PhabricatorInlineComment::loadAndAttachVersionedDrafts(
+ $viewer,
+ array($inline));
}
- PhabricatorInlineComment::loadAndAttachVersionedDrafts(
- $viewer,
- array($inline));
+ if ($is_dirty) {
+ $this->saveComment($inline);
+ }
}
$edit_dialog = $this->buildEditDialog($inline)
@@ -240,12 +269,10 @@
$content = $inline->getContent();
if (!strlen($content)) {
- $this->deleteComment($inline);
- } else {
- $this->saveComment($inline);
+ $inline->setIsDeleted(1);
}
- $this->purgeVersionedDrafts($inline);
+ $this->saveComment($inline);
return $this->buildEmptyResponse();
case 'draft':
@@ -262,6 +289,17 @@
->setProperty('inline.text', $text)
->save();
+ // We have to synchronize the draft engine after saving a versioned
+ // draft, because taking an inline comment from "no text, no draft"
+ // to "no text, text in a draft" marks the container object as having
+ // a draft.
+ $draft_engine = $this->newDraftEngine();
+ if ($draft_engine) {
+ $draft_engine->synchronize();
+ } else {
+ phlog('no draft engine');
+ }
+
return $this->buildEmptyResponse();
case 'new':
case 'reply':
@@ -432,14 +470,6 @@
->setContent($response);
}
- private function purgeVersionedDrafts(
- PhabricatorInlineComment $inline) {
- $viewer = $this->getViewer();
- PhabricatorVersionedDraft::purgeDrafts(
- $inline->getPHID(),
- $viewer->getPHID());
- }
-
final protected function loadCommentByID($id) {
$query = $this->newInlineCommentQuery()
->withIDs(array($id));
@@ -494,4 +524,35 @@
return $inline;
}
+ private function saveComment(PhabricatorInlineComment $inline) {
+ $viewer = $this->getViewer();
+ $draft_engine = $this->newDraftEngine();
+
+ $inline->openTransaction();
+ $inline->save();
+
+ PhabricatorVersionedDraft::purgeDrafts(
+ $inline->getPHID(),
+ $viewer->getPHID());
+
+ if ($draft_engine) {
+ $draft_engine->synchronize();
+ }
+
+ $inline->saveTransaction();
+ }
+
+ private function newDraftEngine() {
+ $viewer = $this->getViewer();
+ $object = $this->getContainerObject();
+
+ if (!($object instanceof PhabricatorDraftInterface)) {
+ return null;
+ }
+
+ return $object->newDraftEngine()
+ ->setObject($object)
+ ->setViewer($viewer);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 23, 6:44 PM (17 h, 51 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6779261
Default Alt Text
D21235.id50571.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility