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 @@ -437,7 +437,6 @@ 'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewersFieldSpecification.php', 'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php', 'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php', - 'DifferentialRevisionCommentView' => 'applications/differential/view/DifferentialRevisionCommentView.php', 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', 'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php', 'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php', @@ -2957,7 +2956,6 @@ 5 => 'HarbormasterBuildableInterface', 6 => 'PhabricatorSubscribableInterface', ), - 'DifferentialRevisionCommentView' => 'AphrontView', 'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionEditController' => 'DifferentialController', 'DifferentialRevisionEditor' => 'PhabricatorEditor', diff --git a/src/applications/differential/controller/DifferentialCommentPreviewController.php b/src/applications/differential/controller/DifferentialCommentPreviewController.php --- a/src/applications/differential/controller/DifferentialCommentPreviewController.php +++ b/src/applications/differential/controller/DifferentialCommentPreviewController.php @@ -21,45 +21,95 @@ return new Aphront404Response(); } - $author_phid = $viewer->getPHID(); - $action = $request->getStr('action'); + $type_comment = PhabricatorTransactions::TYPE_COMMENT; + $type_action = DifferentialTransaction::TYPE_ACTION; + $type_edge = PhabricatorTransactions::TYPE_EDGE; + $type_subscribers = PhabricatorTransactions::TYPE_SUBSCRIBERS; + + $xactions = array(); - $comment = new DifferentialComment(); - $comment->setContent($request->getStr('content')); - $comment->setAction($action); - $comment->setAuthorPHID($author_phid); + $action = $request->getStr('action'); + switch ($action) { + case DifferentialAction::ACTION_COMMENT: + case DifferentialAction::ACTION_ADDREVIEWERS: + case DifferentialAction::ACTION_ADDCCS: + break; + default: + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($type_action) + ->setNewValue($action); + break; + } - $handles = array($author_phid); + $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER; $reviewers = $request->getStrList('reviewers'); if (DifferentialAction::allowReviewers($action) && $reviewers) { - $comment->setMetadata(array( - DifferentialComment::METADATA_ADDED_REVIEWERS => $reviewers)); - $handles = array_merge($handles, $reviewers); + $faux_edges = array(); + foreach ($reviewers as $phid) { + $faux_edges[$phid] = array( + 'src' => $revision->getPHID(), + 'type' => $edge_reviewer, + 'dst' => $phid, + ); + } + + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($type_edge) + ->setMetadataValue('edge:type', $edge_reviewer) + ->setOldValue(array()) + ->setNewValue($faux_edges); } $ccs = $request->getStrList('ccs'); - if ($action == DifferentialAction::ACTION_ADDCCS && $ccs) { - $comment->setMetadata(array( - DifferentialComment::METADATA_ADDED_CCS => $ccs)); - $handles = array_merge($handles, $ccs); + if ($ccs) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($type_subscribers) + ->setOldValue(array()) + ->setNewValue(array_fuse($ccs)); } - $handles = $this->loadViewerHandles($handles); + // Add a comment transaction if there's nothing, so we'll generate a + // nonempty result. + if (strlen($request->getStr('content')) || !$xactions) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($type_comment) + ->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($request->getStr('content'))); + } + + foreach ($xactions as $xaction) { + $xaction->setAuthorPHID($viewer->getPHID()); + } $engine = new PhabricatorMarkupEngine(); $engine->setViewer($request->getUser()); - $engine->addObject($comment, DifferentialComment::MARKUP_FIELD_BODY); + foreach ($xactions as $xaction) { + if ($xaction->hasComment()) { + $engine->addObject( + $xaction->getComment(), + PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); + } + } $engine->process(); - $view = new DifferentialRevisionCommentView(); - $view->setUser($request->getUser()); - $view->setComment($comment); - $view->setHandles($handles); - $view->setMarkupEngine($engine); - $view->setRevision($revision); - $view->setPreview(true); - $view->setTargetDiff(null); + $phids = mpull($xactions, 'getRequiredHandlePHIDs'); + $phids = array_mergev($phids); + + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs($phids) + ->execute(); + + foreach ($xactions as $xaction) { + $xaction->setHandles($handles); + } + + $view = id(new DifferentialTransactionView()) + ->setUser($viewer) + ->setTransactions($xactions) + ->setIsPreview(true); $metadata = array( 'reviewers' => $reviewers, @@ -70,14 +120,14 @@ } id(new PhabricatorDraft()) - ->setAuthorPHID($author_phid) + ->setAuthorPHID($viewer->getPHID()) ->setDraftKey('differential-comment-'.$this->id) - ->setDraft($comment->getContent()) + ->setDraft($request->getStr('content')) ->setMetadata($metadata) ->replaceOrDelete(); return id(new AphrontAjaxResponse()) - ->setContent($view->render()); + ->setContent((string)phutil_implode_html('', $view->buildEvents())); } } diff --git a/src/applications/differential/view/DifferentialRevisionCommentView.php b/src/applications/differential/view/DifferentialRevisionCommentView.php deleted file mode 100644 --- a/src/applications/differential/view/DifferentialRevisionCommentView.php +++ /dev/null @@ -1,305 +0,0 @@ -revision = $revision; - return $this; - } - - public function getRevision() { - return $this->revision; - } - - public function setComment($comment) { - $this->comment = $comment; - return $this; - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) { - $this->markupEngine = $markup_engine; - return $this; - } - - public function setPreview($preview) { - $this->preview = $preview; - return $this; - } - - public function setInlineComments(array $inline_comments) { - assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); - $this->inlines = $inline_comments; - return $this; - } - - public function setChangesets(array $changesets) { - assert_instances_of($changesets, 'DifferentialChangeset'); - // Ship these in sorted by getSortKey() and keyed by ID... or else! - $this->changesets = $changesets; - return $this; - } - - public function setTargetDiff($target) { - $this->target = $target; - return $this; - } - - public function setVersusDiffID($diff_vs) { - $this->versusDiffID = $diff_vs; - return $this; - } - - public function setAnchorName($anchor_name) { - $this->anchorName = $anchor_name; - return $this; - } - - public function render() { - - if (!$this->user) { - throw new Exception("Call setUser() before rendering!"); - } - - $this->requireResource('phabricator-remarkup-css'); - $this->requireResource('differential-revision-comment-css'); - - $comment = $this->comment; - - $action = $comment->getAction(); - - $action_class = 'differential-comment-action-'.$action; - - $info = array(); - - $content = $comment->getContent(); - $hide_comments = true; - if (strlen(rtrim($content))) { - $hide_comments = false; - - $content = $this->markupEngine->getOutput( - $comment, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); - - $content = phutil_tag_div('phabricator-remarkup', $content); - } - - $inline_render = $this->renderInlineComments(); - if ($inline_render) { - $hide_comments = false; - } - - $author = $this->handles[$comment->getAuthorPHID()]; - $author_link = $author->renderLink(); - - $metadata = $comment->getMetadata(); - $added_reviewers = idx( - $metadata, - DifferentialComment::METADATA_ADDED_REVIEWERS, - array()); - $removed_reviewers = idx( - $metadata, - DifferentialComment::METADATA_REMOVED_REVIEWERS, - array()); - $added_ccs = idx( - $metadata, - DifferentialComment::METADATA_ADDED_CCS, - array()); - - $actions = array(); - switch ($comment->getAction()) { - case DifferentialAction::ACTION_ADDCCS: - $actions[] = pht( - "%s added CCs: %s.", - $author_link, - $this->renderHandleList($added_ccs)); - $added_ccs = null; - break; - case DifferentialAction::ACTION_ADDREVIEWERS: - $actions[] = pht( - "%s added reviewers: %s.", - $author_link, - $this->renderHandleList($added_reviewers)); - $added_reviewers = null; - break; - case DifferentialAction::ACTION_UPDATE: - $diff_id = idx($metadata, DifferentialComment::METADATA_DIFF_ID); - if ($diff_id) { - $diff_link = phutil_tag( - 'a', - array( - 'href' => '/D'.$this->getRevision()->getID().'?id='.$diff_id, - ), - 'Diff #'.$diff_id); - $actions[] = pht( - "%s updated this revision to %s.", - $author_link, - $diff_link); - } else { - $actions[] = DifferentialAction::getBasicStoryText( - $comment->getAction(), $author_link); - } - break; - default: - $actions[] = DifferentialAction::getBasicStoryText( - $comment->getAction(), $author_link); - break; - } - - if ($added_reviewers) { - $actions[] = pht( - "%s added reviewers: %s.", - $author_link, - $this->renderHandleList($added_reviewers)); - } - - if ($removed_reviewers) { - $actions[] = pht( - "%s removed reviewers: %s.", - $author_link, - $this->renderHandleList($removed_reviewers)); - } - - if ($added_ccs) { - $actions[] = pht( - "%s added CCs: %s.", - $author_link, - $this->renderHandleList($added_ccs)); - } - - foreach ($actions as $key => $action) { - $actions[$key] = phutil_tag('div', array(), $action); - } - - $xaction_view = id(new PhabricatorTransactionView()) - ->setUser($this->user) - ->setImageURI($author->getImageURI()) - ->setContentSource($comment->getContentSource()) - ->addClass($action_class) - ->setActions($actions); - - if ($this->preview) { - $xaction_view->setIsPreview($this->preview); - } else { - $xaction_view->setEpoch($comment->getDateCreated()); - if ($this->anchorName) { - $anchor_text = - 'D'.$this->getRevision()->getID(). - '#'.preg_replace('/^comment-/', '', $this->anchorName); - - $xaction_view->setAnchor($this->anchorName, $anchor_text); - } - } - - if (!$hide_comments) { - $xaction_view->appendChild(phutil_tag_div( - 'differential-comment-core', - array($content, $inline_render))); - } - - return $xaction_view->render(); - } - - private function renderHandleList(array $phids) { - $result = array(); - foreach ($phids as $phid) { - $result[] = $this->handles[$phid]->renderLink(); - } - return phutil_implode_html(', ', $result); - } - - private function renderInlineComments() { - if (!$this->inlines) { - return null; - } - - $inlines = $this->inlines; - $changesets = $this->changesets; - $inlines_by_changeset = mgroup($inlines, 'getChangesetID'); - $inlines_by_changeset = array_select_keys( - $inlines_by_changeset, - array_keys($this->changesets)); - - $view = new PhabricatorInlineSummaryView(); - foreach ($inlines_by_changeset as $changeset_id => $inlines) { - $changeset = $changesets[$changeset_id]; - $items = array(); - foreach ($inlines as $inline) { - - $on_target = ($this->target) && - ($this->target->getID() == $changeset->getDiffID()); - - $is_visible = false; - if ($inline->getIsNewFile()) { - // This comment is on the right side of the versus diff, and visible - // on the left side of the page. - if ($this->versusDiffID) { - if ($changeset->getDiffID() == $this->versusDiffID) { - $is_visible = true; - } - } - - // This comment is on the right side of the target diff, and visible - // on the right side of the page. - if ($on_target) { - $is_visible = true; - } - } else { - // Ths comment is on the left side of the target diff, and visible - // on the left side of the page. - if (!$this->versusDiffID) { - if ($on_target) { - $is_visible = true; - } - } - - // TODO: We still get one edge case wrong here, when we have a - // versus diff and the file didn't exist in the old version. The - // comment is visible because we show the left side of the target - // diff when there's no corresponding file in the versus diff, but - // we incorrectly link it off-page. - } - - $item = array( - 'id' => $inline->getID(), - 'line' => $inline->getLineNumber(), - 'length' => $inline->getLineLength(), - 'content' => $this->markupEngine->getOutput( - $inline, - DifferentialInlineComment::MARKUP_FIELD_BODY), - ); - - if (!$is_visible) { - $diff_id = $changeset->getDiffID(); - $item['where'] = '(On Diff #'.$diff_id.')'; - $item['href'] = - 'D'.$this->getRevision()->getID(). - '?id='.$diff_id. - '#inline-'.$inline->getID(); - } - - $items[] = $item; - } - $view->addCommentGroup($changeset->getFilename(), $items); - } - - return $view; - } - -}