diff --git a/resources/sql/autopatches/20140218.differentialdraft.sql b/resources/sql/autopatches/20140218.differentialdraft.sql new file mode 100644 index 0000000000..7421e08c98 --- /dev/null +++ b/resources/sql/autopatches/20140218.differentialdraft.sql @@ -0,0 +1,9 @@ +CREATE TABLE {$NAMESPACE}_differential.differential_draft( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + draftKey VARCHAR(64) NOT NULL COLLATE utf8_bin, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_unique` (objectPHID, authorPHID, draftKey) +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index eb76217a3d..4269d031a4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -378,6 +378,7 @@ phutil_register_library_map(array( 'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php', 'DifferentialDiffViewPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialDiffViewPolicyFieldSpecification.php', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php', + 'DifferentialDraft' => 'applications/differential/storage/DifferentialDraft.php', 'DifferentialEditPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialEditPolicyFieldSpecification.php', 'DifferentialException' => 'applications/differential/exception/DifferentialException.php', 'DifferentialExceptionMail' => 'applications/differential/mail/DifferentialExceptionMail.php', @@ -2914,6 +2915,7 @@ phutil_register_library_map(array( 'DifferentialDiffViewController' => 'DifferentialController', 'DifferentialDiffViewPolicyFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher', + 'DifferentialDraft' => 'DifferentialDAO', 'DifferentialEditPolicyFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialException' => 'Exception', 'DifferentialExceptionMail' => 'DifferentialMail', @@ -3957,7 +3959,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventOverlapException' => 'Exception', 'PhabricatorCalendarEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorCalendarEventSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'PhabricatorCalendarEventViewController' => 'PhabricatorDashboardController', + 'PhabricatorCalendarEventViewController' => 'PhabricatorCalendarController', 'PhabricatorCalendarHoliday' => 'PhabricatorCalendarDAO', 'PhabricatorCalendarHolidayTestCase' => 'PhabricatorTestCase', 'PhabricatorCalendarPHIDTypeEvent' => 'PhabricatorPHIDType', diff --git a/src/applications/differential/controller/DifferentialCommentPreviewController.php b/src/applications/differential/controller/DifferentialCommentPreviewController.php index b413b9306f..627d02d2bc 100644 --- a/src/applications/differential/controller/DifferentialCommentPreviewController.php +++ b/src/applications/differential/controller/DifferentialCommentPreviewController.php @@ -119,12 +119,24 @@ final class DifferentialCommentPreviewController $metadata['action'] = $action; } - id(new PhabricatorDraft()) + $draft_key = 'differential-comment-'.$this->id; + $draft = id(new PhabricatorDraft()) ->setAuthorPHID($viewer->getPHID()) - ->setDraftKey('differential-comment-'.$this->id) + ->setDraftKey($draft_key) ->setDraft($request->getStr('content')) ->setMetadata($metadata) ->replaceOrDelete(); + if ($draft->isDeleted()) { + DifferentialDraft::deleteHasDraft( + $viewer->getPHID(), + $revision->getPHID(), + $draft_key); + } else { + DifferentialDraft::markHasDraft( + $viewer->getPHID(), + $revision->getPHID(), + $draft_key); + } return id(new AphrontAjaxResponse()) ->setContent((string)phutil_implode_html('', $view->buildEvents())); diff --git a/src/applications/differential/controller/DifferentialCommentSaveController.php b/src/applications/differential/controller/DifferentialCommentSaveController.php index 6e888947a9..89eed0e093 100644 --- a/src/applications/differential/controller/DifferentialCommentSaveController.php +++ b/src/applications/differential/controller/DifferentialCommentSaveController.php @@ -74,13 +74,15 @@ final class DifferentialCommentSaveController extends DifferentialController { // TODO: Diff change detection? + $user = $request->getUser(); $draft = id(new PhabricatorDraft())->loadOneWhere( 'authorPHID = %s AND draftKey = %s', - $request->getUser()->getPHID(), + $user->getPHID(), 'differential-comment-'.$revision->getID()); if ($draft) { $draft->delete(); } + DifferentialDraft::deleteAllDrafts($user->getPHID(), $revision->getPHID()); return id(new AphrontRedirectResponse()) ->setURI('/D'.$revision->getID()); diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php index 619990b731..1bda9e05d2 100644 --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -74,4 +74,23 @@ final class DifferentialInlineCommentEditController return true; } + protected function deleteComment(PhabricatorInlineCommentInterface $inline) { + $inline->openTransaction(); + DifferentialDraft::deleteHasDraft( + $inline->getAuthorPHID(), + $inline->getRevisionPHID(), + $inline->getPHID()); + $inline->delete(); + $inline->saveTransaction(); + } + + protected function saveComment(PhabricatorInlineCommentInterface $inline) { + $inline->openTransaction(); + $inline->save(); + DifferentialDraft::markHasDraft( + $inline->getAuthorPHID(), + $inline->getRevisionPHID(), + $inline->getPHID()); + $inline->saveTransaction(); + } } diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index c0258dfd52..c942fad813 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -36,7 +36,6 @@ final class DifferentialRevisionQuery private $responsibles = array(); private $branches = array(); private $arcanistProjectPHIDs = array(); - private $draftRevisions = array(); private $repositoryPHIDs; private $order = 'order-modified'; @@ -468,39 +467,6 @@ final class DifferentialRevisionQuery $table = new DifferentialRevision(); $conn_r = $table->establishConnection('r'); - if ($this->draftAuthors) { - $this->draftRevisions = array(); - - $draft_key = 'differential-comment-'; - $drafts = id(new PhabricatorDraft())->loadAllWhere( - 'authorPHID IN (%Ls) AND draftKey LIKE %> AND draft != %s', - $this->draftAuthors, - $draft_key, - ''); - $len = strlen($draft_key); - foreach ($drafts as $draft) { - $this->draftRevisions[] = substr($draft->getDraftKey(), $len); - } - - // TODO: Restore this after drafts are sorted out. It's now very - // expensive to get revision IDs. - - /* - - $inlines = id(new DifferentialInlineCommentQuery()) - ->withDraftsByAuthors($this->draftAuthors) - ->execute(); - foreach ($inlines as $inline) { - $this->draftRevisions[] = $inline->getRevisionID(); - } - - */ - - if (!$this->draftRevisions) { - return array(); - } - } - $selects = array(); // NOTE: If the query includes "responsiblePHIDs", we execute it as a @@ -632,6 +598,16 @@ final class DifferentialRevisionQuery $this->reviewers); } + if ($this->draftAuthors) { + $differential_draft = new DifferentialDraft(); + $joins[] = qsprintf( + $conn_r, + 'JOIN %T has_draft ON has_draft.objectPHID = r.phid '. + 'AND has_draft.authorPHID IN (%Ls)', + $differential_draft->getTableName(), + $this->draftAuthors); + } + $joins = implode(' ', $joins); return $joins; @@ -665,13 +641,6 @@ final class DifferentialRevisionQuery $this->authors); } - if ($this->draftRevisions) { - $where[] = qsprintf( - $conn_r, - 'r.id IN (%Ld)', - $this->draftRevisions); - } - if ($this->revIDs) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/differential/storage/DifferentialDraft.php b/src/applications/differential/storage/DifferentialDraft.php new file mode 100644 index 0000000000..4831d8a5f9 --- /dev/null +++ b/src/applications/differential/storage/DifferentialDraft.php @@ -0,0 +1,51 @@ +setObjectPHID($object_phid) + ->setAuthorPHID($author_phid) + ->setDraftKey($draft_key) + ->save(); + } catch (AphrontQueryDuplicateKeyException $ex) { + // no worries + } + } + + public static function deleteHasDraft( + $author_phid, + $object_phid, + $draft_key) { + $draft = id(new DifferentialDraft())->loadOneWhere( + 'objectPHID = %s AND authorPHID = %s AND draftKey = %s', + $object_phid, + $author_phid, + $draft_key); + if ($draft) { + $draft->delete(); + } + } + + public static function deleteAllDrafts( + $author_phid, + $object_phid) { + + $drafts = id(new DifferentialDraft())->loadAllWhere( + 'objectPHID = %s AND authorPHID = %s', + $object_phid, + $author_phid); + foreach ($drafts as $draft) { + $draft->delete(); + } + } + +} diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php index 928104d9d6..c2dec0d510 100644 --- a/src/applications/differential/storage/DifferentialInlineComment.php +++ b/src/applications/differential/storage/DifferentialInlineComment.php @@ -28,6 +28,13 @@ final class DifferentialInlineComment return $this->proxy; } + public function openTransaction() { + $this->proxy->openTransaction(); + } + + public function saveTransaction() { + $this->proxy->saveTransaction(); + } public function save() { $this->getTransactionCommentForSave()->save(); @@ -45,6 +52,10 @@ final class DifferentialInlineComment return $this->proxy->getID(); } + public function getPHID() { + return $this->proxy->getPHID(); + } + public static function newFromModernComment( DifferentialTransactionComment $comment) { @@ -141,6 +152,10 @@ final class DifferentialInlineComment return $this; } + public function getRevisionPHID() { + return $this->proxy->getRevisionPHID(); + } + // Although these are purely transitional, they're also *extra* dumb. public function setRevisionID($revision_id) { diff --git a/src/applications/diffusion/controller/DiffusionInlineCommentController.php b/src/applications/diffusion/controller/DiffusionInlineCommentController.php index 5cc0c384d5..78df83292d 100644 --- a/src/applications/diffusion/controller/DiffusionInlineCommentController.php +++ b/src/applications/diffusion/controller/DiffusionInlineCommentController.php @@ -76,4 +76,12 @@ final class DiffusionInlineCommentController return true; } + protected function deleteComment(PhabricatorInlineCommentInterface $inline) { + return $inline->delete(); + } + + protected function saveComment(PhabricatorInlineCommentInterface $inline) { + return $inline->save(); + } + } diff --git a/src/applications/draft/storage/PhabricatorDraft.php b/src/applications/draft/storage/PhabricatorDraft.php index 21daa14b12..5b6a6e1aee 100644 --- a/src/applications/draft/storage/PhabricatorDraft.php +++ b/src/applications/draft/storage/PhabricatorDraft.php @@ -7,6 +7,8 @@ final class PhabricatorDraft extends PhabricatorDraftDAO { protected $draft; protected $metadata = array(); + private $deleted = false; + public function getConfiguration() { return array( self::CONFIG_SERIALIZATION => array( @@ -23,11 +25,20 @@ final class PhabricatorDraft extends PhabricatorDraftDAO { $this->getTableName(), $this->authorPHID, $this->draftKey); + $this->deleted = true; return $this; } return parent::replace(); } + protected function didDelete() { + $this->deleted = true; + } + + public function isDeleted() { + return $this->deleted; + } + public static function newFromUserAndKey(PhabricatorUser $user, $key) { if ($user->getPHID() && strlen($key)) { $draft = id(new PhabricatorDraft())->loadOneWhere( diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index 22550936cf..780a152799 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -6,6 +6,10 @@ abstract class PhabricatorInlineCommentController abstract protected function createComment(); abstract protected function loadComment($id); abstract protected function loadCommentForEdit($id); + abstract protected function deleteComment( + PhabricatorInlineCommentInterface $inline); + abstract protected function saveComment( + PhabricatorInlineCommentInterface $inline); private $changesetID; private $isNewFile; @@ -60,7 +64,7 @@ abstract class PhabricatorInlineCommentController $inline = $this->loadCommentForEdit($this->getCommentID()); if ($request->isFormPost()) { - $inline->delete(); + $this->deleteComment($inline); return $this->buildEmptyResponse(); } @@ -86,12 +90,12 @@ abstract class PhabricatorInlineCommentController if ($request->isFormPost()) { if (strlen($text)) { $inline->setContent($text); - $inline->save(); + $this->saveComment($inline); return $this->buildRenderedCommentResponse( $inline, $this->getIsOnRight()); } else { - $inline->delete(); + $this->deleteComment($inline); return $this->buildEmptyResponse(); } } @@ -122,8 +126,8 @@ abstract class PhabricatorInlineCommentController ->setLineNumber($this->getLineNumber()) ->setLineLength($this->getLineLength()) ->setIsNewFile($this->getIsNewFile()) - ->setContent($text) - ->save(); + ->setContent($text); + $this->saveComment($inline); return $this->buildRenderedCommentResponse( $inline,