Page MenuHomePhabricator

D8236.id19597.diff
No OneTemporary

D8236.id19597.diff

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 @@
-<?php
-
-final class DifferentialRevisionCommentView extends AphrontView {
-
- private $comment;
- private $handles;
- private $markupEngine;
- private $preview;
- private $inlines;
- private $changesets;
- private $target;
- private $anchorName;
- private $versusDiffID;
- private $revision;
-
- public function setRevision(DifferentialRevision $revision) {
- $this->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;
- }
-
-}

File Metadata

Mime Type
text/plain
Expires
Mar 18 2025, 1:14 AM (4 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7709164
Default Alt Text
D8236.id19597.diff (15 KB)

Event Timeline