From 8605a1808d019915860ccfa060f5ba32fdb14269 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Jul 2014 17:59:28 -0700 Subject: [PATCH] Hide direct accesses to Audit inline comment table behind API Summary: Ref T4896. Move all direct accesses to the inline comment table behind a small amount of API to make it easier to migrate the table. Test Plan: - Grepped for `PhabricatorAuditInlineComment`. - Grepped for `audit_inlinecomment`. - Created a draft comment. - Previewed a draft comment. - Reloaded page, still saw draft. - Viewed standalone, still saw draft. - Made comment, inline published. - Added a draft, saw both. - Edited inline comment. - Reindexed commit. - Searched for unique word in published comment, found commit. - Searched for unique word in draft comment, no results. Reviewers: btrahan, joshuaspence Reviewed By: joshuaspence Subscribers: epriestley Maniphest Tasks: T4896 Differential Revision: https://secure.phabricator.com/D10016 --- .../editor/PhabricatorAuditCommentEditor.php | 6 +-- .../storage/PhabricatorAuditInlineComment.php | 39 +++++++++++++++++++ .../controller/DiffusionCommitController.php | 11 +++--- .../controller/DiffusionDiffController.php | 8 ++-- ...iffusionInlineCommentPreviewController.php | 5 +-- ...abricatorRepositoryCommitSearchIndexer.php | 4 +- 6 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 88a293a576..bf81fe4fa5 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -46,10 +46,8 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor { $inline_comments = array(); if ($this->attachInlineComments) { - $inline_comments = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'authorPHID = %s AND commitPHID = %s - AND auditCommentID IS NULL', - $actor->getPHID(), + $inline_comments = PhabricatorAuditInlineComment::loadDraftComments( + $actor, $commit->getPHID()); } diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php index 010b903088..615144eae7 100644 --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -17,6 +17,45 @@ final class PhabricatorAuditInlineComment private $syntheticAuthor; + public static function loadDraftComments( + PhabricatorUser $viewer, + $commit_phid) { + + return id(new PhabricatorAuditInlineComment())->loadAllWhere( + 'authorPHID = %s AND commitPHID = %s AND auditCommentID IS NULL', + $viewer->getPHID(), + $commit_phid); + } + + public static function loadPublishedComments( + PhabricatorUser $viewer, + $commit_phid) { + + return id(new PhabricatorAuditInlineComment())->loadAllWhere( + 'commitPHID = %s AND auditCommentID IS NOT NULL', + $commit_phid); + } + + public static function loadDraftAndPublishedComments( + PhabricatorUser $viewer, + $commit_phid, + $path_id = null) { + + if ($path_id === null) { + return id(new PhabricatorAuditInlineComment())->loadAllWhere( + 'commitPHID = %s AND (auditCommentID IS NOT NULL OR authorPHID = %s)', + $commit_phid, + $viewer->getPHID()); + } else { + return id(new PhabricatorAuditInlineComment())->loadAllWhere( + 'commitPHID = %s AND pathID = %d AND + (authorPHID = %s OR auditCommentID IS NOT NULL)', + $commit_phid, + $path_id, + $viewer->getPHID()); + } + } + public function setSyntheticAuthor($synthetic_author) { $this->syntheticAuthor = $synthetic_author; return $this; diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 3f0b4179dc..ff0eef5c01 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -323,10 +323,9 @@ final class DiffusionCommitController extends DiffusionController { $visible_changesets = $changesets; } else { $visible_changesets = array(); - $inlines = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'commitPHID = %s AND (auditCommentID IS NOT NULL OR authorPHID = %s)', - $commit->getPHID(), - $user->getPHID()); + $inlines = PhabricatorAuditInlineComment::loadDraftAndPublishedComments( + $user, + $commit->getPHID()); $path_ids = mpull($inlines, null, 'getPathID'); foreach ($changesets as $key => $changeset) { if (array_key_exists($changeset->getID(), $path_ids)) { @@ -648,8 +647,8 @@ final class DiffusionCommitController extends DiffusionController { 'targetPHID = %s ORDER BY dateCreated ASC', $commit->getPHID()); - $inlines = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'commitPHID = %s AND auditCommentID IS NOT NULL', + $inlines = PhabricatorAuditInlineComment::loadPublishedComments( + $user, $commit->getPHID()); $path_ids = mpull($inlines, 'getPathID'); diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index 16e21ba09b..6740eb0bc5 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -86,12 +86,10 @@ final class DiffusionDiffController extends DiffusionController { $parser->setWhitespaceMode( DifferentialChangesetParser::WHITESPACE_SHOW_ALL); - $inlines = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'commitPHID = %s AND pathID = %d AND - (authorPHID = %s OR auditCommentID IS NOT NULL)', + $inlines = PhabricatorAuditInlineComment::loadDraftAndPublishedComments( + $user, $drequest->loadCommit()->getPHID(), - $path_id, - $user->getPHID()); + $path_id); if ($inlines) { foreach ($inlines as $inline) { diff --git a/src/applications/diffusion/controller/DiffusionInlineCommentPreviewController.php b/src/applications/diffusion/controller/DiffusionInlineCommentPreviewController.php index 23908d5743..7d66c58a78 100644 --- a/src/applications/diffusion/controller/DiffusionInlineCommentPreviewController.php +++ b/src/applications/diffusion/controller/DiffusionInlineCommentPreviewController.php @@ -12,9 +12,8 @@ final class DiffusionInlineCommentPreviewController protected function loadInlineComments() { $user = $this->getRequest()->getUser(); - $inlines = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'authorPHID = %s AND commitPHID = %s AND auditCommentID IS NULL', - $user->getPHID(), + $inlines = PhabricatorAuditInlineComment::loadDraftComments( + $user, $this->commitPHID); return $inlines; diff --git a/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php index 868f6c4001..d8bc0125b8 100644 --- a/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php +++ b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php @@ -77,8 +77,8 @@ final class PhabricatorRepositoryCommitSearchIndexer } } - $inlines = id(new PhabricatorAuditInlineComment())->loadAllWhere( - 'commitPHID = %s AND (auditCommentID IS NOT NULL)', + $inlines = PhabricatorAuditInlineComment::loadPublishedComments( + $this->getViewer(), $commit->getPHID()); foreach ($inlines as $inline) { if (strlen($inline->getContent())) {