Page MenuHomePhabricator

D17205.id.diff
No OneTemporary

D17205.id.diff

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 @@
+<?php
+
+final class DifferentialRevisionDraftEngine
+ extends PhabricatorDraftEngine {
+
+ protected function hasCustomDraftContent() {
+ $viewer = $this->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 @@
+<?php
+
+final class PhabricatorBuiltinDraftEngine
+ extends PhabricatorDraftEngine {}
diff --git a/src/applications/transactions/draft/PhabricatorDraftEngine.php b/src/applications/transactions/draft/PhabricatorDraftEngine.php
new file mode 100644
--- /dev/null
+++ b/src/applications/transactions/draft/PhabricatorDraftEngine.php
@@ -0,0 +1,98 @@
+<?php
+
+abstract class PhabricatorDraftEngine
+ extends Phobject {
+
+ private $viewer;
+ private $object;
+ private $hasVersionedDraft;
+ private $versionedDraft;
+
+ final public function setViewer(PhabricatorUser $viewer) {
+ $this->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 @@
+<?php
+
+interface PhabricatorDraftInterface {
+
+ public function newDraftEngine();
+
+}
diff --git a/src/applications/transactions/edges/PhabricatorObjectHasDraftEdgeType.php b/src/applications/transactions/edges/PhabricatorObjectHasDraftEdgeType.php
new file mode 100644
--- /dev/null
+++ b/src/applications/transactions/edges/PhabricatorObjectHasDraftEdgeType.php
@@ -0,0 +1,8 @@
+<?php
+
+final class PhabricatorObjectHasDraftEdgeType
+ extends PhabricatorEdgeType {
+
+ const EDGECONST = 64;
+
+}
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php
--- a/src/applications/transactions/editengine/PhabricatorEditEngine.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php
@@ -1746,10 +1746,19 @@
$viewer->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 )------------------------------------------------------------ */

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 29, 11:20 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7724257
Default Alt Text
D17205.id.diff (18 KB)

Event Timeline