Page MenuHomePhabricator
Files F14003488

D8215.id19549.diff
No OneTemporary
Actions

D8215.id19549.diff
View Options

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',
- 'DifferentialRevisionCommentListView' => 'applications/differential/view/DifferentialRevisionCommentListView.php',
'DifferentialRevisionCommentView' => 'applications/differential/view/DifferentialRevisionCommentView.php',
'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php',
@@ -465,6 +464,7 @@
'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php',
'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php',
+ 'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php',
'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php',
'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
@@ -2951,7 +2951,6 @@
5 => 'HarbormasterBuildableInterface',
6 => 'PhabricatorSubscribableInterface',
),
- 'DifferentialRevisionCommentListView' => 'AphrontView',
'DifferentialRevisionCommentView' => 'AphrontView',
'DifferentialRevisionDetailView' => 'AphrontView',
'DifferentialRevisionEditController' => 'DifferentialController',
@@ -2979,6 +2978,7 @@
'DifferentialTransaction' => 'PhabricatorApplicationTransaction',
'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+ 'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification',
'DiffusionBranchTableController' => 'DiffusionController',
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -258,15 +258,9 @@
$revision_detail->setActions($actions);
$revision_detail->setUser($user);
- $comment_view = new DifferentialRevisionCommentListView();
- $comment_view->setComments($comments);
- $comment_view->setHandles($handles);
- $comment_view->setInlineComments($inlines);
- $comment_view->setChangesets($all_changesets);
- $comment_view->setUser($user);
- $comment_view->setTargetDiff($target);
- $comment_view->setVersusDiffID($diff_vs);
- $comment_view->setRevision($revision);
+ $comment_view = $this->buildTransactions(
+ $revision,
+ $changesets);
if ($arc_project) {
Javelin::initBehavior(
@@ -926,4 +920,28 @@
return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
}
+
+ private function buildTransactions(
+ DifferentialRevision $revision,
+ array $changesets) {
+ $viewer = $this->getRequest()->getUser();
+
+ $xactions = id(new DifferentialTransactionQuery())
+ ->setViewer($viewer)
+ ->withObjectPHIDs(array($revision->getPHID()))
+ ->needComments(true)
+ ->execute();
+
+ $engine = id(new PhabricatorMarkupEngine())
+ ->setViewer($viewer);
+
+ $timeline = id(new DifferentialTransactionView())
+ ->setUser($viewer)
+ ->setObjectPHID($revision->getPHID())
+ ->setChangesets($changesets)
+ ->setTransactions($xactions);
+
+ return $timeline;
+ }
+
}
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -18,4 +18,20 @@
return new DifferentialTransactionComment();
}
+ public function getTitle() {
+ $author_phid = $this->getAuthorPHID();
+
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
+ switch ($this->getTransactionType()) {
+ case self::TYPE_INLINE:
+ return pht(
+ '%s added inline comments.',
+ $this->renderHandleLink($author_phid));
+ }
+
+ return parent::getTitle();
+ }
+
}
diff --git a/src/applications/differential/view/DifferentialRevisionCommentListView.php b/src/applications/differential/view/DifferentialRevisionCommentListView.php
deleted file mode 100644
--- a/src/applications/differential/view/DifferentialRevisionCommentListView.php
+++ /dev/null
@@ -1,209 +0,0 @@
-<?php
-
-final class DifferentialRevisionCommentListView extends AphrontView {
-
- private $comments;
- private $handles;
- private $inlines;
- private $changesets;
- private $target;
- private $versusDiffID;
- private $id;
- private $revision;
-
- public function setRevision(DifferentialRevision $revision) {
- $this->revision = $revision;
- return $this;
- }
-
- public function getRevision() {
- return $this->revision;
- }
-
- public function setComments(array $comments) {
- assert_instances_of($comments, 'DifferentialComment');
- $this->comments = $comments;
- return $this;
- }
-
- public function setInlineComments(array $inline_comments) {
- assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
- $this->inlines = $inline_comments;
- return $this;
- }
-
- public function setHandles(array $handles) {
- assert_instances_of($handles, 'PhabricatorObjectHandle');
- $this->handles = $handles;
- return $this;
- }
-
- public function setChangesets(array $changesets) {
- assert_instances_of($changesets, 'DifferentialChangeset');
- $this->changesets = $changesets;
- return $this;
- }
-
- public function setTargetDiff(DifferentialDiff $target) {
- $this->target = $target;
- return $this;
- }
-
- public function setVersusDiffID($diff_vs) {
- $this->versusDiffID = $diff_vs;
- return $this;
- }
-
- public function getID() {
- if (!$this->id) {
- $this->id = celerity_generate_unique_node_id();
- }
- return $this->id;
- }
-
- public function render() {
-
- $this->requireResource('differential-revision-comment-list-css');
-
- $engine = new PhabricatorMarkupEngine();
- $engine->setViewer($this->user);
- foreach ($this->comments as $comment) {
- $comment->giveFacebookSomeArbitraryDiff($this->target);
-
- $engine->addObject(
- $comment,
- DifferentialComment::MARKUP_FIELD_BODY);
- }
-
- foreach ($this->inlines as $inline) {
- $engine->addObject(
- $inline,
- PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
- }
-
- $engine->process();
-
- $inlines = mgroup($this->inlines, 'getCommentID');
-
- $num = 1;
- $html = array();
- foreach ($this->comments as $comment) {
- $view = new DifferentialRevisionCommentView();
- $view->setComment($comment);
- $view->setUser($this->user);
- $view->setHandles($this->handles);
- $view->setMarkupEngine($engine);
-// $view->setInlineComments(idx($inlines, $comment->getID(), array()));
- $view->setChangesets($this->changesets);
- $view->setTargetDiff($this->target);
- $view->setRevision($this->getRevision());
- $view->setVersusDiffID($this->versusDiffID);
- if ($comment->getAction() == DifferentialAction::ACTION_SUMMARIZE) {
- $view->setAnchorName('summary');
- } else if ($comment->getAction() == DifferentialAction::ACTION_TESTPLAN) {
- $view->setAnchorName('test-plan');
- } else {
- $view->setAnchorName('comment-'.$num);
- $num++;
- }
-
- $html[] = $view->render();
- }
-
- $objs = array_reverse(array_values($this->comments));
- $html = array_reverse(array_values($html));
- $user = $this->user;
-
- $last_comment = null;
- // Find the most recent comment by the viewer.
- foreach ($objs as $position => $comment) {
- if ($user && ($comment->getAuthorPHID() == $user->getPHID())) {
- if ($last_comment === null) {
- $last_comment = $position;
- } else if ($last_comment == $position - 1) {
- // If the viewer made several comments in a row, show them all. This
- // is a spaz rule for epriestley.
- $last_comment = $position;
- }
- }
- }
-
- $header = array();
- $hidden = array();
- if ($last_comment !== null) {
- foreach ($objs as $position => $comment) {
- if (!$comment->getPHID()) {
- // These are synthetic comments with summary/test plan information.
- $header[] = $html[$position];
- unset($html[$position]);
- continue;
- }
- if ($position <= $last_comment) {
- // Always show comments after the viewer's last comment.
- continue;
- }
- if ($position < 3) {
- // Always show the 3 most recent comments.
- continue;
- }
- $hidden[] = $position;
- }
- }
-
- if (count($hidden) <= 3) {
- // Don't hide if there's not much to hide.
- $hidden = array();
- }
-
- $header = array_reverse($header);
-
-
- $hidden = array_select_keys($html, $hidden);
- $visible = array_diff_key($html, $hidden);
-
- $hidden = array_reverse($hidden);
- $visible = array_reverse($visible);
-
- if ($hidden) {
- $this->initBehavior(
- 'differential-show-all-comments',
- array(
- 'markup' => implode("\n", $hidden),
- ));
- $hidden = javelin_tag(
- 'div',
- array(
- 'sigil' => "differential-all-comments-container",
- ),
- phutil_tag(
- 'div',
- array(
- 'class' => 'differential-older-comments-are-hidden',
- ),
- array(
- pht(
- '%s older comments are hidden.',
- new PhutilNumber(count($hidden))),
- ' ',
- javelin_tag(
- 'a',
- array(
- 'href' => '#',
- 'mustcapture' => true,
- 'sigil' => 'differential-show-all-comments',
- ),
- pht('Show all comments.')),
- )));
- } else {
- $hidden = null;
- }
-
- return javelin_tag(
- 'div',
- array(
- 'class' => 'differential-comment-list',
- 'id' => $this->getID(),
- ),
- array_merge($header, array($hidden), $visible));
- }
-}
diff --git a/src/applications/differential/view/DifferentialTransactionView.php b/src/applications/differential/view/DifferentialTransactionView.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/view/DifferentialTransactionView.php
@@ -0,0 +1,127 @@
+<?php
+
+final class DifferentialTransactionView
+ extends PhabricatorApplicationTransactionView {
+
+ private $changesets;
+
+ public function setChangesets(array $changesets) {
+ assert_instances_of($changesets, 'DifferentialChangeset');
+ $this->changesets = $changesets;
+ return $this;
+ }
+
+ public function getChangesets() {
+ return $this->changesets;
+ }
+
+ // TODO: There's a whole lot of code duplication between this and
+ // PholioTransactionView to handle inlines. Merge this into the core? Some of
+ // it can probably be shared, while other parts are trickier.
+
+ protected function shouldGroupTransactions(
+ PhabricatorApplicationTransaction $u,
+ PhabricatorApplicationTransaction $v) {
+
+ if ($u->getAuthorPHID() != $v->getAuthorPHID()) {
+ // Don't group transactions by different authors.
+ return false;
+ }
+
+ if (($v->getDateCreated() - $u->getDateCreated()) > 60) {
+ // Don't group if transactions that happened more than 60s apart.
+ return false;
+ }
+
+ switch ($u->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_COMMENT:
+ case DifferentialTransaction::TYPE_INLINE:
+ break;
+ default:
+ return false;
+ }
+
+ switch ($v->getTransactionType()) {
+ case DifferentialTransaction::TYPE_INLINE:
+ return true;
+ }
+
+ return parent::shouldGroupTransactions($u, $v);
+ }
+
+ protected function renderTransactionContent(
+ PhabricatorApplicationTransaction $xaction) {
+
+ $out = array();
+
+ $type_inline = DifferentialTransaction::TYPE_INLINE;
+
+ $group = $xaction->getTransactionGroup();
+ if ($xaction->getTransactionType() == $type_inline) {
+ array_unshift($group, $xaction);
+ } else {
+ $out[] = parent::renderTransactionContent($xaction);
+ }
+
+ if (!$group) {
+ return $out;
+ }
+
+ $inlines = array();
+ foreach ($group as $xaction) {
+ switch ($xaction->getTransactionType()) {
+ case DifferentialTransaction::TYPE_INLINE:
+ $inlines[] = $xaction;
+ break;
+ default:
+ throw new Exception("Unknown grouped transaction type!");
+ }
+ }
+
+ if ($inlines) {
+ $inline_view = new PhabricatorInlineSummaryView();
+
+ $changesets = $this->getChangesets();
+ $changesets = mpull($changesets, null, 'getID');
+
+ // Group the changesets by file and reorder them by display order.
+ $inline_groups = array();
+ foreach ($inlines as $inline) {
+ $inline_groups[$inline->getComment()->getChangesetID()][] = $inline;
+ }
+
+ $changsets = msort($changesets, 'getFilename');
+ $inline_groups = array_select_keys(
+ $inline_groups,
+ array_keys($changesets));
+
+ foreach ($inline_groups as $changeset_id => $group) {
+ $group = msort($group, 'getLineNumber');
+
+ $changeset = $changesets[$changeset_id];
+ $items = array();
+ foreach ($group as $inline) {
+ $comment = $inline->getComment();
+ $item = array(
+ 'id' => $comment->getID(),
+ 'line' => $comment->getLineNumber(),
+ 'length' => $comment->getLineLength(),
+ 'content' => parent::renderTransactionContent($inline),
+ );
+
+ // TODO: Fix the where/href stuff for nonlocal inlines.
+
+ $items[] = $item;
+ }
+ $inline_view->addCommentGroup(
+ $changeset->getFilename(),
+ $items);
+ }
+
+ $out[] = $inline_view;
+ }
+
+ return $out;
+ }
+
+}
diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php
--- a/src/applications/pholio/storage/PholioTransaction.php
+++ b/src/applications/pholio/storage/PholioTransaction.php
@@ -1,8 +1,5 @@
<?php
-/**
- * @group pholio
- */
final class PholioTransaction extends PhabricatorApplicationTransaction {
public function getApplicationName() {
diff --git a/src/applications/pholio/view/PholioTransactionView.php b/src/applications/pholio/view/PholioTransactionView.php
--- a/src/applications/pholio/view/PholioTransactionView.php
+++ b/src/applications/pholio/view/PholioTransactionView.php
@@ -16,7 +16,7 @@
}
if (($v->getDateCreated() - $u->getDateCreated()) > 60) {
- // Don't group if transactions happend more than 60s apart.
+ // Don't group if transactions happened more than 60s apart.
return false;
}

File Metadata

Mime Type
text/plain
Expires
Sun, Oct 27, 6:45 AM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6751998
Default Alt Text
D8215.id19549.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility