diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php index c09caa9374..619990b731 100644 --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -1,76 +1,77 @@ revisionID = $data['id']; } protected function createComment() { // Verify revision and changeset correspond to actual objects. $revision_id = $this->revisionID; $changeset_id = $this->getChangesetID(); $viewer = $this->getRequest()->getUser(); $revision = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs(array($revision_id)) ->executeOne(); if (!$revision) { throw new Exception("Invalid revision ID!"); } if (!id(new DifferentialChangeset())->load($changeset_id)) { throw new Exception("Invalid changeset ID!"); } return id(new DifferentialInlineComment()) ->setRevision($revision) ->setChangesetID($changeset_id); } protected function loadComment($id) { return id(new DifferentialInlineCommentQuery()) ->withIDs(array($id)) ->executeOne(); } protected function loadCommentForEdit($id) { $request = $this->getRequest(); $user = $request->getUser(); $inline = $this->loadComment($id); if (!$this->canEditInlineComment($user, $inline)) { throw new Exception("That comment is not editable!"); } return $inline; } private function canEditInlineComment( PhabricatorUser $user, DifferentialInlineComment $inline) { // Only the author may edit a comment. if ($inline->getAuthorPHID() != $user->getPHID()) { return false; } - // Saved comments may not be edited. - if ($inline->getCommentID()) { + // Saved comments may not be edited, for now, although the schema now + // supports it. + if (!$inline->isDraft()) { return false; } // Inline must be attached to the active revision. if ($inline->getRevisionID() != $this->revisionID) { return false; } return true; } } diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php index 8eb59760c1..cd338b74c1 100644 --- a/src/applications/differential/query/DifferentialInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php @@ -1,168 +1,156 @@ revisionIDs = $ids; return $this; } public function withNotDraft($not_draft) { $this->notDraft = $not_draft; return $this; } public function withIDs(array $ids) { $this->ids = $ids; return $this; } public function withViewerAndChangesetIDs($author_phid, array $ids) { $this->viewerAndChangesetIDs = array($author_phid, $ids); return $this; } public function withDraftComments($author_phid, $revision_id) { $this->draftComments = array($author_phid, $revision_id); return $this; } public function withDraftsByAuthors(array $author_phids) { $this->draftsByAuthors = $author_phids; return $this; } - public function withCommentIDs(array $comment_ids) { - $this->commentIDs = $comment_ids; - return $this; - } - public function execute() { $table = new DifferentialTransactionComment(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( $conn_r, 'SELECT * FROM %T %Q %Q', $table->getTableName(), $this->buildWhereClause($conn_r), $this->buildLimitClause($conn_r)); $comments = $table->loadAllFromArray($data); foreach ($comments as $key => $value) { $comments[$key] = DifferentialInlineComment::newFromModernComment( $value); } return $comments; } public function executeOne() { return head($this->execute()); } private function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); // Only find inline comments. $where[] = qsprintf( $conn_r, 'changesetID IS NOT NULL'); if ($this->revisionIDs) { // Look up revision PHIDs. $revision_phids = queryfx_all( $conn_r, 'SELECT phid FROM %T WHERE id IN (%Ld)', id(new DifferentialRevision())->getTableName(), $this->revisionIDs); if (!$revision_phids) { throw new PhabricatorEmptyQueryException(); } $revision_phids = ipull($revision_phids, 'phid'); $where[] = qsprintf( $conn_r, 'revisionPHID IN (%Ls)', $revision_phids); } if ($this->notDraft) { $where[] = qsprintf( $conn_r, 'transactionPHID IS NOT NULL'); } if ($this->ids) { $where[] = qsprintf( $conn_r, 'id IN (%Ld)', $this->ids); } if ($this->viewerAndChangesetIDs) { list($phid, $ids) = $this->viewerAndChangesetIDs; $where[] = qsprintf( $conn_r, 'changesetID IN (%Ld) AND (authorPHID = %s OR transactionPHID IS NOT NULL)', $ids, $phid); } if ($this->draftComments) { list($phid, $rev_id) = $this->draftComments; $rev_phid = queryfx_one( $conn_r, 'SELECT phid FROM %T WHERE id = %d', id(new DifferentialRevision())->getTableName(), $rev_id); if (!$rev_phid) { throw new PhabricatorEmptyQueryException(); } $rev_phid = $rev_phid['phid']; $where[] = qsprintf( $conn_r, 'authorPHID = %s AND revisionPHID = %s AND transactionPHID IS NULL', $phid, $rev_phid); } if ($this->draftsByAuthors) { $where[] = qsprintf( $conn_r, 'authorPHID IN (%Ls) AND transactionPHID IS NULL', $this->draftsByAuthors); } - if ($this->commentIDs) { - $where[] = qsprintf( - $conn_r, - 'legacyCommentID IN (%Ld)', - $this->commentIDs); - } - return $this->formatWhereClause($where); } } diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php index 4e7dcf47a7..928104d9d6 100644 --- a/src/applications/differential/storage/DifferentialInlineComment.php +++ b/src/applications/differential/storage/DifferentialInlineComment.php @@ -1,208 +1,202 @@ proxy = new DifferentialTransactionComment(); } public function __clone() { $this->proxy = clone $this->proxy; } public function getTransactionCommentForSave() { $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_LEGACY, array()); $this->proxy ->setViewPolicy('public') ->setEditPolicy($this->getAuthorPHID()) ->setContentSource($content_source) ->setCommentVersion(1); return $this->proxy; } public function save() { $this->getTransactionCommentForSave()->save(); return $this; } public function delete() { $this->proxy->delete(); return $this; } public function getID() { return $this->proxy->getID(); } public static function newFromModernComment( DifferentialTransactionComment $comment) { $obj = new DifferentialInlineComment(); $obj->proxy = $comment; return $obj; } public function setSyntheticAuthor($synthetic_author) { $this->syntheticAuthor = $synthetic_author; return $this; } public function getSyntheticAuthor() { return $this->syntheticAuthor; } public function isCompatible(PhabricatorInlineCommentInterface $comment) { return ($this->getAuthorPHID() === $comment->getAuthorPHID()) && ($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) && ($this->getContent() === $comment->getContent()); } public function setContent($content) { $this->proxy->setContent($content); return $this; } public function getContent() { return $this->proxy->getContent(); } public function isDraft() { return !$this->proxy->getTransactionPHID(); } public function setChangesetID($id) { $this->proxy->setChangesetID($id); return $this; } public function getChangesetID() { return $this->proxy->getChangesetID(); } public function setIsNewFile($is_new) { $this->proxy->setIsNewFile($is_new); return $this; } public function getIsNewFile() { return $this->proxy->getIsNewFile(); } public function setLineNumber($number) { $this->proxy->setLineNumber($number); return $this; } public function getLineNumber() { return $this->proxy->getLineNumber(); } public function setLineLength($length) { $this->proxy->setLineLength($length); return $this; } public function getLineLength() { return $this->proxy->getLineLength(); } public function setCache($cache) { return $this; } public function getCache() { return null; } public function setAuthorPHID($phid) { $this->proxy->setAuthorPHID($phid); return $this; } public function getAuthorPHID() { return $this->proxy->getAuthorPHID(); } public function setRevision(DifferentialRevision $revision) { $this->proxy->setRevisionPHID($revision->getPHID()); return $this; } // Although these are purely transitional, they're also *extra* dumb. public function setRevisionID($revision_id) { $revision = id(new DifferentialRevision())->load($revision_id); return $this->setRevision($revision); } public function getRevisionID() { $phid = $this->proxy->getRevisionPHID(); if (!$phid) { return null; } $revision = id(new DifferentialRevision())->loadOneWhere( 'phid = %s', $phid); if (!$revision) { return null; } return $revision->getID(); } // When setting a comment ID, we also generate a phantom transaction PHID for // the future transaction. public function setCommentID($id) { - $this->proxy->setLegacyCommentID($id); $this->proxy->setTransactionPHID( PhabricatorPHID::generateNewPHID( PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST, DifferentialPHIDTypeRevision::TYPECONST)); return $this; } - public function getCommentID() { - return $this->proxy->getLegacyCommentID(); - } - - /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ public function getMarkupFieldKey($field) { // We can't use ID because synthetic comments don't have it. return 'DI:'.PhabricatorHash::digest($this->getContent()); } public function newMarkupEngine($field) { return PhabricatorMarkupEngine::newDifferentialMarkupEngine(); } public function getMarkupText($field) { return $this->getContent(); } public function didMarkupText($field, $output, PhutilMarkupEngine $engine) { return $output; } public function shouldUseMarkupCache($field) { // Only cache submitted comments. - return ($this->getID() && $this->getCommentID()); + return ($this->getID() && !$this->isDraft()); } }