From 2972894a4df99bbde7de4c2b2af80bd2b4c6eb95 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 9 Mar 2015 11:51:26 -0700 Subject: [PATCH] Write "hasReplies" to database for inline comments Summary: Ref T1460. Ref T2618. When publishing a draft inline, mark the inline it replies to (if any) as replied to. Also, don't load deleted comments as drafts (sets the stage for T2618). I'll make an effort to clean up the loading mess here in the next revision, and find some more appropriate home for the shared code. Test Plan: Made and replied to comments in Differential and Diffusion. Saw comments get marked as "Has Replies" and "Is Reply". Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2618, T1460 Differential Revision: https://secure.phabricator.com/D12025 --- .../audit/editor/PhabricatorAuditEditor.php | 7 +++- .../storage/PhabricatorAuditInlineComment.php | 9 ++++- .../PhabricatorAuditTransactionComment.php | 12 ++++++ .../editor/DifferentialTransactionEditor.php | 5 +++ .../query/DifferentialTransactionQuery.php | 11 ++++- .../DifferentialTransactionComment.php | 12 ++++++ .../PhabricatorInlineCommentController.php | 40 +++++++++++++++++++ 7 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index a77d931689..84764c1426 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -135,9 +135,14 @@ final class PhabricatorAuditEditor case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_EDGE: case PhabricatorAuditActionConstants::ACTION: - case PhabricatorAuditActionConstants::INLINE: case PhabricatorAuditTransaction::TYPE_COMMIT: return; + case PhabricatorAuditActionConstants::INLINE: + $reply = $xaction->getComment()->getReplyToComment(); + if ($reply && !$reply->getHasReplies()) { + $reply->setHasReplies(1)->save(); + } + return; case PhabricatorAuditActionConstants::ADD_AUDITORS: $new = $xaction->getNewValue(); if (!is_array($new)) { diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php index c3f2263de6..13cb76174d 100644 --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -63,10 +63,15 @@ final class PhabricatorAuditInlineComment $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'authorPHID = %s AND commitPHID = %s AND transactionPHID IS NULL - AND pathID IS NOT NULL', + AND pathID IS NOT NULL + AND isDeleted = 0', $viewer->getPHID(), $commit_phid); + $inlines = PhabricatorInlineCommentController::loadAndAttachReplies( + $viewer, + $inlines); + return self::buildProxies($inlines); } @@ -96,7 +101,7 @@ final class PhabricatorAuditInlineComment } else { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'commitPHID = %s AND pathID = %d AND - (authorPHID = %s OR transactionPHID IS NOT NULL)', + ((authorPHID = %s AND isDeleted = 0) OR transactionPHID IS NOT NULL)', $commit_phid, $path_id, $viewer->getPHID()); diff --git a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php index 9a9925258e..64ddca8e25 100644 --- a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php +++ b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php @@ -13,6 +13,8 @@ final class PhabricatorAuditTransactionComment protected $replyToCommentPHID; protected $legacyCommentID; + private $replyToComment = self::ATTACHABLE; + public function getApplicationTransactionObject() { return new PhabricatorAuditTransaction(); } @@ -55,4 +57,14 @@ final class PhabricatorAuditTransactionComment return $config; } + public function attachReplyToComment( + PhabricatorAuditTransactionComment $comment = null) { + $this->replyToComment = $comment; + return $this; + } + + public function getReplyToComment() { + return $this->assertAttached($this->replyToComment); + } + } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index e17746fce0..c3dd3c3ace 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -534,7 +534,12 @@ final class DifferentialTransactionEditor case PhabricatorTransactions::TYPE_EDGE: case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_ACTION: + return; case DifferentialTransaction::TYPE_INLINE: + $reply = $xaction->getComment()->getReplyToComment(); + if ($reply && !$reply->getHasReplies()) { + $reply->setHasReplies(1)->save(); + } return; case DifferentialTransaction::TYPE_UPDATE: // Now that we're inside the transaction, do a final check. diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php index 60fea527c1..64ef271655 100644 --- a/src/applications/differential/query/DifferentialTransactionQuery.php +++ b/src/applications/differential/query/DifferentialTransactionQuery.php @@ -25,7 +25,8 @@ final class DifferentialTransactionQuery 'SELECT phid FROM %T WHERE revisionPHID = %s AND authorPHID = %s - AND transactionPHID IS NULL', + AND transactionPHID IS NULL + AND isDeleted = 0', $table->getTableName(), $revision->getPHID(), $viewer->getPHID()); @@ -35,11 +36,17 @@ final class DifferentialTransactionQuery return array(); } - return id(new PhabricatorApplicationTransactionCommentQuery()) + $comments = id(new PhabricatorApplicationTransactionCommentQuery()) ->setTemplate(new DifferentialTransactionComment()) ->setViewer($viewer) ->withPHIDs($phids) ->execute(); + + $comments = PhabricatorInlineCommentController::loadAndAttachReplies( + $viewer, + $comments); + + return $comments; } } diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php index fd406f3969..0d1e6ea33a 100644 --- a/src/applications/differential/storage/DifferentialTransactionComment.php +++ b/src/applications/differential/storage/DifferentialTransactionComment.php @@ -12,10 +12,22 @@ final class DifferentialTransactionComment protected $hasReplies = 0; protected $replyToCommentPHID; + private $replyToComment = self::ATTACHABLE; + public function getApplicationTransactionObject() { return new DifferentialTransaction(); } + public function attachReplyToComment( + DifferentialTransactionComment $comment = null) { + $this->replyToComment = $comment; + return $this; + } + + public function getReplyToComment() { + return $this->assertAttached($this->replyToComment); + } + protected function getConfiguration() { $config = parent::getConfiguration(); $config[self::CONFIG_COLUMN_SCHEMA] = array( diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index a23efbc9a4..c400b965d2 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -309,4 +309,44 @@ abstract class PhabricatorInlineCommentController ->addRowScaffold($view); } + public static function loadAndAttachReplies( + PhabricatorUser $viewer, + array $comments) { + // TODO: This code doesn't really belong here, but we don't have a much + // better place to put it at the moment. + + if (!$comments) { + return $comments; + } + + $template = head($comments); + + $reply_phids = array(); + foreach ($comments as $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + if ($reply_phid) { + $reply_phids[] = $reply_phid; + } + } + + if ($reply_phids) { + $reply_comments = id(new PhabricatorApplicationTransactionCommentQuery()) + ->setTemplate($template) + ->setViewer($viewer) + ->withPHIDs($reply_phids) + ->execute(); + $reply_comments = mpull($reply_comments, null, 'getPHID'); + } else { + $reply_comments = array(); + } + + foreach ($comments as $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + $reply = idx($reply_comments, $reply_phid); + $comment->attachReplyToComment($reply); + } + + return $comments; + } + }