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 @@ -519,6 +519,7 @@ 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', 'DifferentialRevisionDependedOnByRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php', 'DifferentialRevisionDependsOnRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php', + 'DifferentialRevisionDraftEngine' => 'applications/differential/engine/DifferentialRevisionDraftEngine.php', 'DifferentialRevisionEditConduitAPIMethod' => 'applications/differential/conduit/DifferentialRevisionEditConduitAPIMethod.php', 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', 'DifferentialRevisionEditEngine' => 'applications/differential/editor/DifferentialRevisionEditEngine.php', @@ -2055,6 +2056,7 @@ 'PhabricatorBotUser' => 'infrastructure/daemon/bot/target/PhabricatorBotUser.php', 'PhabricatorBotWhatsNewHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php', 'PhabricatorBritishEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorBritishEnglishTranslation.php', + 'PhabricatorBuiltinDraftEngine' => 'applications/transactions/draft/PhabricatorBuiltinDraftEngine.php', 'PhabricatorBuiltinPatchList' => 'infrastructure/storage/patch/PhabricatorBuiltinPatchList.php', 'PhabricatorBulkContentSource' => 'infrastructure/daemon/contentsource/PhabricatorBulkContentSource.php', 'PhabricatorBusyUIExample' => 'applications/uiexample/examples/PhabricatorBusyUIExample.php', @@ -2550,6 +2552,8 @@ 'PhabricatorDoorkeeperApplication' => 'applications/doorkeeper/application/PhabricatorDoorkeeperApplication.php', 'PhabricatorDraft' => 'applications/draft/storage/PhabricatorDraft.php', 'PhabricatorDraftDAO' => 'applications/draft/storage/PhabricatorDraftDAO.php', + 'PhabricatorDraftEngine' => 'applications/transactions/draft/PhabricatorDraftEngine.php', + 'PhabricatorDraftInterface' => 'applications/transactions/draft/PhabricatorDraftInterface.php', 'PhabricatorDrydockApplication' => 'applications/drydock/application/PhabricatorDrydockApplication.php', 'PhabricatorEdgeConfig' => 'infrastructure/edges/constants/PhabricatorEdgeConfig.php', 'PhabricatorEdgeConstants' => 'infrastructure/edges/constants/PhabricatorEdgeConstants.php', @@ -3095,6 +3099,7 @@ 'PhabricatorObjectHasAsanaSubtaskEdgeType' => 'applications/doorkeeper/edge/PhabricatorObjectHasAsanaSubtaskEdgeType.php', 'PhabricatorObjectHasAsanaTaskEdgeType' => 'applications/doorkeeper/edge/PhabricatorObjectHasAsanaTaskEdgeType.php', 'PhabricatorObjectHasContributorEdgeType' => 'applications/transactions/edges/PhabricatorObjectHasContributorEdgeType.php', + 'PhabricatorObjectHasDraftEdgeType' => 'applications/transactions/edges/PhabricatorObjectHasDraftEdgeType.php', 'PhabricatorObjectHasFileEdgeType' => 'applications/transactions/edges/PhabricatorObjectHasFileEdgeType.php', 'PhabricatorObjectHasJiraIssueEdgeType' => 'applications/doorkeeper/edge/PhabricatorObjectHasJiraIssueEdgeType.php', 'PhabricatorObjectHasSubscriberEdgeType' => 'applications/transactions/edges/PhabricatorObjectHasSubscriberEdgeType.php', @@ -5210,6 +5215,7 @@ 'PhabricatorProjectInterface', 'PhabricatorFulltextInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorDraftInterface', ), 'DifferentialRevisionAbandonTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionAcceptTransaction' => 'DifferentialRevisionReviewTransaction', @@ -5226,6 +5232,7 @@ 'DifferentialRevisionControlSystem' => 'Phobject', 'DifferentialRevisionDependedOnByRevisionEdgeType' => 'PhabricatorEdgeType', 'DifferentialRevisionDependsOnRevisionEdgeType' => 'PhabricatorEdgeType', + 'DifferentialRevisionDraftEngine' => 'PhabricatorDraftEngine', 'DifferentialRevisionEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'DifferentialRevisionEditController' => 'DifferentialController', 'DifferentialRevisionEditEngine' => 'PhabricatorEditEngine', @@ -6981,6 +6988,7 @@ 'PhabricatorBotUser' => 'PhabricatorBotTarget', 'PhabricatorBotWhatsNewHandler' => 'PhabricatorBotHandler', 'PhabricatorBritishEnglishTranslation' => 'PhutilTranslation', + 'PhabricatorBuiltinDraftEngine' => 'PhabricatorDraftEngine', 'PhabricatorBuiltinPatchList' => 'PhabricatorSQLPatchList', 'PhabricatorBulkContentSource' => 'PhabricatorContentSource', 'PhabricatorBusyUIExample' => 'PhabricatorUIExample', @@ -7559,6 +7567,7 @@ 'PhabricatorDoorkeeperApplication' => 'PhabricatorApplication', 'PhabricatorDraft' => 'PhabricatorDraftDAO', 'PhabricatorDraftDAO' => 'PhabricatorLiskDAO', + 'PhabricatorDraftEngine' => 'Phobject', 'PhabricatorDrydockApplication' => 'PhabricatorApplication', 'PhabricatorEdgeConfig' => 'PhabricatorEdgeConstants', 'PhabricatorEdgeConstants' => 'Phobject', @@ -8172,6 +8181,7 @@ 'PhabricatorObjectHasAsanaSubtaskEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectHasAsanaTaskEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectHasContributorEdgeType' => 'PhabricatorEdgeType', + 'PhabricatorObjectHasDraftEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectHasFileEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectHasJiraIssueEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectHasSubscriberEdgeType' => 'PhabricatorEdgeType', 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 @@ -152,36 +152,23 @@ protected function deleteComment(PhabricatorInlineCommentInterface $inline) { $inline->openTransaction(); - $inline->setIsDeleted(1)->save(); - DifferentialDraft::deleteHasDraft( - $inline->getAuthorPHID(), - $inline->getRevisionPHID(), - $inline->getPHID()); - + $this->syncDraft(); $inline->saveTransaction(); } protected function undeleteComment( PhabricatorInlineCommentInterface $inline) { $inline->openTransaction(); - $inline->setIsDeleted(0)->save(); - DifferentialDraft::markHasDraft( - $inline->getAuthorPHID(), - $inline->getRevisionPHID(), - $inline->getPHID()); - + $this->syncDraft(); $inline->saveTransaction(); } protected function saveComment(PhabricatorInlineCommentInterface $inline) { $inline->openTransaction(); $inline->save(); - DifferentialDraft::markHasDraft( - $inline->getAuthorPHID(), - $inline->getRevisionPHID(), - $inline->getPHID()); + $this->syncDraft(); $inline->saveTransaction(); } @@ -224,4 +211,14 @@ $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 new file mode 100644 --- /dev/null +++ b/src/applications/differential/engine/DifferentialRevisionDraftEngine.php @@ -0,0 +1,17 @@ +getViewer(); + $revision = $this->getObject(); + + $inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments( + $viewer, + $revision); + + return (bool)$inlines; + } + +} diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -473,15 +473,33 @@ } if ($this->needDrafts) { - $drafts = id(new DifferentialDraft())->loadAllWhere( - 'authorPHID = %s AND objectPHID IN (%Ls)', - $viewer->getPHID(), - mpull($revisions, 'getPHID')); - $drafts = mgroup($drafts, 'getObjectPHID'); - foreach ($revisions as $revision) { - $revision->attachDrafts( - $viewer, - idx($drafts, $revision->getPHID(), array())); + $viewer_phid = $viewer->getPHID(); + $draft_type = PhabricatorObjectHasDraftEdgeType::EDGECONST; + + if (!$viewer_phid) { + // Viewers without a valid PHID can never have drafts. + foreach ($revisions as $revision) { + $revision->attachHasDraft($viewer, false); + } + } else { + $edge_query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(mpull($revisions, 'getPHID')) + ->withEdgeTypes( + array( + $draft_type, + )) + ->withDestinationPHIDs(array($viewer_phid)); + + $edge_query->execute(); + + foreach ($revisions as $revision) { + $has_draft = (bool)$edge_query->getDestinationPHIDs( + array( + $revision->getPHID(), + )); + + $revision->attachHasDraft($viewer, $has_draft); + } } } @@ -621,12 +639,13 @@ } 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(), + 'JOIN %T has_draft ON has_draft.srcPHID = r.phid + AND has_draft.type = %s + AND has_draft.dstPHID IN (%Ls)', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + PhabricatorObjectHasDraftEdgeType::EDGECONST, $this->draftAuthors); } diff --git a/src/applications/differential/storage/DifferentialDraft.php b/src/applications/differential/storage/DifferentialDraft.php --- a/src/applications/differential/storage/DifferentialDraft.php +++ b/src/applications/differential/storage/DifferentialDraft.php @@ -20,46 +20,4 @@ ) + parent::getConfiguration(); } - public static function markHasDraft( - $author_phid, - $object_phid, - $draft_key) { - try { - id(new DifferentialDraft()) - ->setObjectPHID($object_phid) - ->setAuthorPHID($author_phid) - ->setDraftKey($draft_key) - ->save(); - } catch (AphrontDuplicateKeyQueryException $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/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -15,7 +15,8 @@ PhabricatorDestructibleInterface, PhabricatorProjectInterface, PhabricatorFulltextInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorDraftInterface { protected $title = ''; protected $originalTitle; @@ -488,12 +489,12 @@ return $this; } - public function getDrafts(PhabricatorUser $viewer) { - return $this->assertAttachedKey($this->drafts, $viewer->getPHID()); + public function getHasDraft(PhabricatorUser $viewer) { + return $this->assertAttachedKey($this->drafts, $viewer->getCacheFragment()); } - public function attachDrafts(PhabricatorUser $viewer, array $drafts) { - $this->drafts[$viewer->getPHID()] = $drafts; + public function attachHasDraft(PhabricatorUser $viewer, $has_draft) { + $this->drafts[$viewer->getCacheFragment()] = $has_draft; return $this; } @@ -735,4 +736,12 @@ return array(); } + +/* -( PhabricatorDraftInterface )------------------------------------------ */ + + + public function newDraftEngine() { + return new DifferentialRevisionDraftEngine(); + } + } diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -92,7 +92,7 @@ ''); } - if ($revision->getDrafts($viewer)) { + if ($revision->getHasDraft($viewer)) { $icons['draft'] = true; } diff --git a/src/applications/transactions/draft/PhabricatorBuiltinDraftEngine.php b/src/applications/transactions/draft/PhabricatorBuiltinDraftEngine.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/draft/PhabricatorBuiltinDraftEngine.php @@ -0,0 +1,4 @@ +viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function setObject($object) { + $this->object = $object; + return $this; + } + + final public function getObject() { + return $this->object; + } + + final public function setVersionedDraft( + PhabricatorVersionedDraft $draft = null) { + $this->hasVersionedDraft = true; + $this->versionedDraft = $draft; + return $this; + } + + final public function getVersionedDraft() { + if (!$this->hasVersionedDraft) { + $draft = PhabricatorVersionedDraft::loadDraft( + $this->getObject()->getPHID(), + $this->getViewer()->getPHID()); + $this->setVersionedDraft($draft); + } + + return $this->versionedDraft; + } + + protected function hasVersionedDraftContent() { + $draft = $this->getVersionedDraft(); + if (!$draft) { + return false; + } + + if ($draft->getProperty('comment')) { + return true; + } + + if ($draft->getProperty('actions')) { + return true; + } + + return false; + } + + protected function hasCustomDraftContent() { + return false; + } + + final protected function hasAnyDraftContent() { + if ($this->hasVersionedDraftContent()) { + return true; + } + + if ($this->hasCustomDraftContent()) { + return true; + } + + return false; + } + + final public function synchronize() { + $object_phid = $this->getObject()->getPHID(); + $viewer_phid = $this->getViewer()->getPHID(); + + $has_draft = $this->hasAnyDraftContent(); + + $draft_type = PhabricatorObjectHasDraftEdgeType::EDGECONST; + $editor = id(new PhabricatorEdgeEditor()); + + if ($has_draft) { + $editor->addEdge($object_phid, $draft_type, $viewer_phid); + } else { + $editor->removeEdge($object_phid, $draft_type, $viewer_phid); + } + + $editor->save(); + } + +} diff --git a/src/applications/transactions/draft/PhabricatorDraftInterface.php b/src/applications/transactions/draft/PhabricatorDraftInterface.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/draft/PhabricatorDraftInterface.php @@ -0,0 +1,7 @@ +getPHID(), $current_version); + $is_empty = (!strlen($comment_text) && !$actions); + $draft ->setProperty('comment', $comment_text) ->setProperty('actions', $actions) ->save(); + + $draft_engine = $this->newDraftEngine($object); + if ($draft_engine) { + $draft_engine + ->setVersionedDraft($draft) + ->synchronize(); + } } } @@ -1831,6 +1840,13 @@ $object->getPHID(), $viewer->getPHID(), $this->loadDraftVersion($object)); + + $draft_engine = $this->newDraftEngine($object); + if ($draft_engine) { + $draft_engine + ->setVersionedDraft(null) + ->synchronize(); + } } if ($request->isAjax() && $is_preview) { @@ -1847,6 +1863,20 @@ } } + protected function newDraftEngine($object) { + $viewer = $this->getViewer(); + + if ($object instanceof PhabricatorDraftInterface) { + $engine = $object->newDraftEngine(); + } else { + $engine = new PhabricatorBuiltinDraftEngine(); + } + + return $engine + ->setObject($object) + ->setViewer($viewer); + } + /* -( Conduit )------------------------------------------------------------ */