From 903e37a21b50ee8d92fba1b854b508c55ca237e2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 14 Jan 2017 09:22:51 -0800 Subject: [PATCH] Show yellow "draft" bubble in Audit Summary: Fixes T6660. Uses the new stuff in Audit to build an EditEngine-aware icon. Test Plan: {F2364304} Reviewers: chad Reviewed By: chad Maniphest Tasks: T6660 Differential Revision: https://secure.phabricator.com/D17208 --- src/__phutil_library_map__.php | 3 ++ .../query/PhabricatorCommitSearchEngine.php | 6 ++- .../audit/view/PhabricatorAuditListView.php | 32 ++++++++++++++-- .../query/DifferentialRevisionQuery.php | 31 ++-------------- .../engine/DiffusionCommitDraftEngine.php | 18 +++++++++ .../diffusion/query/DiffusionCommitQuery.php | 14 +++++++ .../storage/PhabricatorRepositoryCommit.php | 21 ++++++++++- .../draft/PhabricatorDraftEngine.php | 37 +++++++++++++++++++ .../draft/PhabricatorDraftInterface.php | 21 +++++++++++ 9 files changed, 149 insertions(+), 34 deletions(-) create mode 100644 src/applications/diffusion/engine/DiffusionCommitDraftEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 76144781ad..4083a5aeb8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -629,6 +629,7 @@ phutil_register_library_map(array( 'DiffusionCommitDiffContentHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffContentHeraldField.php', 'DiffusionCommitDiffContentRemovedHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffContentRemovedHeraldField.php', 'DiffusionCommitDiffEnormousHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffEnormousHeraldField.php', + 'DiffusionCommitDraftEngine' => 'applications/diffusion/engine/DiffusionCommitDraftEngine.php', 'DiffusionCommitEditConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCommitEditConduitAPIMethod.php', 'DiffusionCommitEditController' => 'applications/diffusion/controller/DiffusionCommitEditController.php', 'DiffusionCommitEditEngine' => 'applications/diffusion/editor/DiffusionCommitEditEngine.php', @@ -5341,6 +5342,7 @@ phutil_register_library_map(array( 'DiffusionCommitDiffContentHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitDiffContentRemovedHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitDiffEnormousHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitDraftEngine' => 'PhabricatorDraftEngine', 'DiffusionCommitEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'DiffusionCommitEditController' => 'DiffusionController', 'DiffusionCommitEditEngine' => 'PhabricatorEditEngine', @@ -8794,6 +8796,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorFulltextInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorDraftInterface', ), 'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO', diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php index fa4fa3c963..f24b12d2b8 100644 --- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php +++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php @@ -14,7 +14,8 @@ final class PhabricatorCommitSearchEngine public function newQuery() { return id(new DiffusionCommitQuery()) ->needAuditRequests(true) - ->needCommitData(true); + ->needCommitData(true) + ->needDrafts(true); } protected function newResultBuckets() { @@ -140,7 +141,8 @@ final class PhabricatorCommitSearchEngine $bucket = $this->getResultBucket($query); $template = id(new PhabricatorAuditListView()) - ->setViewer($viewer); + ->setViewer($viewer) + ->setShowDrafts(true); $views = array(); if ($bucket) { diff --git a/src/applications/audit/view/PhabricatorAuditListView.php b/src/applications/audit/view/PhabricatorAuditListView.php index f348acc759..8d4aef61f2 100644 --- a/src/applications/audit/view/PhabricatorAuditListView.php +++ b/src/applications/audit/view/PhabricatorAuditListView.php @@ -4,6 +4,7 @@ final class PhabricatorAuditListView extends AphrontView { private $commits; private $header; + private $showDrafts; private $noDataString; private $highlightedAudits; @@ -25,6 +26,15 @@ final class PhabricatorAuditListView extends AphrontView { return $this->header; } + public function setShowDrafts($show_drafts) { + $this->showDrafts = $show_drafts; + return $this; + } + + public function getShowDrafts() { + return $this->showDrafts; + } + /** * These commits should have both commit data and audit requests attached. */ @@ -75,6 +85,16 @@ final class PhabricatorAuditListView extends AphrontView { $handles = $viewer->loadHandles(mpull($this->commits, 'getPHID')); + $show_drafts = $this->getShowDrafts(); + + $draft_icon = id(new PHUIIconView()) + ->setIcon('fa-comment yellow') + ->addSigil('has-tooltip') + ->setMetadata( + array( + 'tip' => pht('Unsubmitted Comments'), + )); + $list = new PHUIObjectItemListView(); foreach ($this->commits as $commit) { $commit_phid = $commit->getPHID(); @@ -88,7 +108,6 @@ final class PhabricatorAuditListView extends AphrontView { $audits = mpull($commit->getAudits(), null, 'getAuditorPHID'); $auditors = array(); - $reasons = array(); foreach ($audits as $audit) { $auditor_phid = $audit->getAuditorPHID(); $auditors[$auditor_phid] = $viewer->renderHandle($auditor_phid); @@ -114,9 +133,16 @@ final class PhabricatorAuditListView extends AphrontView { $item = id(new PHUIObjectItemView()) ->setObjectName($commit_name) ->setHeader($commit_desc) - ->setHref($commit_link) + ->setHref($commit_link); + + if ($show_drafts) { + if ($commit->getHasDraft($viewer)) { + $item->addAttribute($draft_icon); + } + } + + $item ->addAttribute(pht('Author: %s', $author_name)) - ->addAttribute($reasons) ->addIcon('none', $committed); if (!empty($auditors)) { diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index a4e446ad0d..5dff433e9c 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -473,34 +473,9 @@ final class DifferentialRevisionQuery } if ($this->needDrafts) { - $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); - } - } + PhabricatorDraftEngine::attachDrafts( + $viewer, + $revisions); } return $revisions; diff --git a/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php b/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php new file mode 100644 index 0000000000..ce39e525a6 --- /dev/null +++ b/src/applications/diffusion/engine/DiffusionCommitDraftEngine.php @@ -0,0 +1,18 @@ +getViewer(); + $commit = $this->getObject(); + + $inlines = PhabricatorAuditInlineComment::loadDraftComments( + $viewer, + $commit->getPHID(), + $raw = true); + + return (bool)$inlines; + } + +} diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index a24f8038e0..fddaf547bd 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -22,6 +22,7 @@ final class DiffusionCommitQuery private $importing; private $needCommitData; + private $needDrafts; public function withIDs(array $ids) { $this->ids = $ids; @@ -98,6 +99,11 @@ final class DiffusionCommitQuery return $this; } + public function needDrafts($need) { + $this->needDrafts = $need; + return $this; + } + public function needAuditRequests($need) { $this->needAuditRequests = $need; return $this; @@ -239,6 +245,8 @@ final class DiffusionCommitQuery } protected function didFilterPage(array $commits) { + $viewer = $this->getViewer(); + if ($this->needCommitData) { $data = id(new PhabricatorRepositoryCommitData())->loadAllWhere( 'commitID in (%Ld)', @@ -268,6 +276,12 @@ final class DiffusionCommitQuery } } + if ($this->needDrafts) { + PhabricatorDraftEngine::attachDrafts( + $viewer, + $commits); + } + return $commits; } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 67f187d9c2..7f617069ec 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -14,7 +14,8 @@ final class PhabricatorRepositoryCommit PhabricatorCustomFieldInterface, PhabricatorApplicationTransactionInterface, PhabricatorFulltextInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorDraftInterface { protected $repositoryID; protected $phid; @@ -39,6 +40,7 @@ final class PhabricatorRepositoryCommit private $audits = self::ATTACHABLE; private $repository = self::ATTACHABLE; private $customFields = self::ATTACHABLE; + private $drafts = array(); public function attachRepository(PhabricatorRepository $repository) { $this->repository = $repository; @@ -342,6 +344,7 @@ final class PhabricatorRepositoryCommit return nonempty($parsed->getDisplayName(), $parsed->getAddress()); } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() { @@ -603,4 +606,20 @@ final class PhabricatorRepositoryCommit return array(); } + +/* -( PhabricatorDraftInterface )------------------------------------------ */ + + public function newDraftEngine() { + return new DiffusionCommitDraftEngine(); + } + + public function getHasDraft(PhabricatorUser $viewer) { + return $this->assertAttachedKey($this->drafts, $viewer->getCacheFragment()); + } + + public function attachHasDraft(PhabricatorUser $viewer, $has_draft) { + $this->drafts[$viewer->getCacheFragment()] = $has_draft; + return $this; + } + } diff --git a/src/applications/transactions/draft/PhabricatorDraftEngine.php b/src/applications/transactions/draft/PhabricatorDraftEngine.php index 63a9945151..9e55104912 100644 --- a/src/applications/transactions/draft/PhabricatorDraftEngine.php +++ b/src/applications/transactions/draft/PhabricatorDraftEngine.php @@ -95,4 +95,41 @@ abstract class PhabricatorDraftEngine $editor->save(); } + final public static function attachDrafts( + PhabricatorUser $viewer, + array $objects) { + assert_instances_of($objects, 'PhabricatorDraftInterface'); + + $viewer_phid = $viewer->getPHID(); + + if (!$viewer_phid) { + // Viewers without a valid PHID can never have drafts. + foreach ($objects as $object) { + $object->attachHasDraft($viewer, false); + } + return; + } else { + $draft_type = PhabricatorObjectHasDraftEdgeType::EDGECONST; + + $edge_query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(mpull($objects, 'getPHID')) + ->withEdgeTypes( + array( + $draft_type, + )) + ->withDestinationPHIDs(array($viewer_phid)); + + $edge_query->execute(); + + foreach ($objects as $object) { + $has_draft = (bool)$edge_query->getDestinationPHIDs( + array( + $object->getPHID(), + )); + + $object->attachHasDraft($viewer, $has_draft); + } + } + } + } diff --git a/src/applications/transactions/draft/PhabricatorDraftInterface.php b/src/applications/transactions/draft/PhabricatorDraftInterface.php index 69b2edf8a0..2e03c17d50 100644 --- a/src/applications/transactions/draft/PhabricatorDraftInterface.php +++ b/src/applications/transactions/draft/PhabricatorDraftInterface.php @@ -4,4 +4,25 @@ interface PhabricatorDraftInterface { public function newDraftEngine(); + public function getHasDraft(PhabricatorUser $viewer); + public function attachHasDraft(PhabricatorUser $viewer, $has_draft); + } + +/* -( PhabricatorDraftInterface )------------------------------------------ */ +/* + + public function newDraftEngine() { + return new <...>DraftEngine(); + } + + public function getHasDraft(PhabricatorUser $viewer) { + return $this->assertAttachedKey($this->drafts, $viewer->getCacheFragment()); + } + + public function attachHasDraft(PhabricatorUser $viewer, $has_draft) { + $this->drafts[$viewer->getCacheFragment()] = $has_draft; + return $this; + } + +*/