mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-19 12:00:55 +01:00
Make Differential timeline aware of ghostly inlines
Summary: Ref T7447. Ref T7870. When a ghostly inline appears on the page, the timeline isn't currently aware that it's present, so it links elsewhere. Instead, apply adjustments before rendering the timeline. Ref T5030. This makes the behaviors in T5030 irrelevant most of the time. Test Plan: Before: {F378106} After: - Inlines visible on page are linked directly. - Inlines which we can't port (e.g., on files not present on current page) are still linked off-page. - Used "Show Older" to page through and verify consistent rendering. Reviewers: btrahan Reviewed By: btrahan Subscribers: yelirekim, epriestley Maniphest Tasks: T5030, T7870, T7447 Differential Revision: https://secure.phabricator.com/D12497
This commit is contained in:
parent
d8bd3efa2c
commit
1858d2aa66
3 changed files with 65 additions and 22 deletions
|
@ -90,6 +90,23 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
||||||
$repository);
|
$repository);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$map = $vs_map;
|
||||||
|
if (!$map) {
|
||||||
|
$map = array_fill_keys(array_keys($changesets), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
$old_ids = array();
|
||||||
|
$new_ids = array();
|
||||||
|
foreach ($map as $id => $vs) {
|
||||||
|
if ($vs <= 0) {
|
||||||
|
$old_ids[] = $id;
|
||||||
|
$new_ids[] = $id;
|
||||||
|
} else {
|
||||||
|
$new_ids[] = $id;
|
||||||
|
$new_ids[] = $vs;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$props = id(new DifferentialDiffProperty())->loadAllWhere(
|
$props = id(new DifferentialDiffProperty())->loadAllWhere(
|
||||||
'diffID = %d',
|
'diffID = %d',
|
||||||
$target_manual->getID());
|
$target_manual->getID());
|
||||||
|
@ -154,24 +171,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
||||||
pht('Show All Files Inline'))));
|
pht('Show All Files Inline'))));
|
||||||
$warning = $warning->render();
|
$warning = $warning->render();
|
||||||
|
|
||||||
$map = $vs_map;
|
$old = array_select_keys($changesets, $old_ids);
|
||||||
if (!$map) {
|
$new = array_select_keys($changesets, $new_ids);
|
||||||
$map = array_fill_keys(array_keys($changesets), 0);
|
|
||||||
}
|
|
||||||
|
|
||||||
$old = array();
|
|
||||||
$new = array();
|
|
||||||
foreach ($map as $id => $vs) {
|
|
||||||
if ($vs <= 0) {
|
|
||||||
$old[] = $id;
|
|
||||||
$new[] = $id;
|
|
||||||
} else {
|
|
||||||
$new[] = $id;
|
|
||||||
$new[] = $vs;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
$old = array_select_keys($changesets, $old);
|
|
||||||
$new = array_select_keys($changesets, $new);
|
|
||||||
|
|
||||||
$query = id(new DifferentialInlineCommentQuery())
|
$query = id(new DifferentialInlineCommentQuery())
|
||||||
->setViewer($user)
|
->setViewer($user)
|
||||||
|
@ -285,7 +286,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
||||||
$comment_view = $this->buildTransactions(
|
$comment_view = $this->buildTransactions(
|
||||||
$revision,
|
$revision,
|
||||||
$diff_vs ? $diffs[$diff_vs] : $target,
|
$diff_vs ? $diffs[$diff_vs] : $target,
|
||||||
$target);
|
$target,
|
||||||
|
$old_ids,
|
||||||
|
$new_ids);
|
||||||
|
|
||||||
if (!$viewer_is_anonymous) {
|
if (!$viewer_is_anonymous) {
|
||||||
$comment_view->setQuoteRef('D'.$revision->getID());
|
$comment_view->setQuoteRef('D'.$revision->getID());
|
||||||
|
@ -931,7 +934,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
||||||
private function buildTransactions(
|
private function buildTransactions(
|
||||||
DifferentialRevision $revision,
|
DifferentialRevision $revision,
|
||||||
DifferentialDiff $left_diff,
|
DifferentialDiff $left_diff,
|
||||||
DifferentialDiff $right_diff) {
|
DifferentialDiff $right_diff,
|
||||||
|
array $old_ids,
|
||||||
|
array $new_ids) {
|
||||||
|
|
||||||
$timeline = $this->buildTransactionTimeline(
|
$timeline = $this->buildTransactionTimeline(
|
||||||
$revision,
|
$revision,
|
||||||
|
@ -940,6 +945,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
||||||
array(
|
array(
|
||||||
'left' => $left_diff->getID(),
|
'left' => $left_diff->getID(),
|
||||||
'right' => $right_diff->getID(),
|
'right' => $right_diff->getID(),
|
||||||
|
'old' => implode(',', $old_ids),
|
||||||
|
'new' => implode(',', $new_ids),
|
||||||
));
|
));
|
||||||
|
|
||||||
return $timeline;
|
return $timeline;
|
||||||
|
|
|
@ -151,6 +151,18 @@ final class DifferentialInlineCommentQuery
|
||||||
|
|
||||||
$changeset_ids = mpull($inlines, 'getChangesetID');
|
$changeset_ids = mpull($inlines, 'getChangesetID');
|
||||||
$changeset_ids = array_unique($changeset_ids);
|
$changeset_ids = array_unique($changeset_ids);
|
||||||
|
|
||||||
|
$all_map = mpull($all, null, 'getID');
|
||||||
|
|
||||||
|
// We already have at least some changesets, and we might not need to do
|
||||||
|
// any more data fetching. Remove everything we already have so we can
|
||||||
|
// tell if we need new stuff.
|
||||||
|
foreach ($changeset_ids as $key => $id) {
|
||||||
|
if (isset($all_map[$id])) {
|
||||||
|
unset($changeset_ids[$key]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if ($changeset_ids) {
|
if ($changeset_ids) {
|
||||||
$changesets = id(new DifferentialChangesetQuery())
|
$changesets = id(new DifferentialChangesetQuery())
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
|
@ -160,7 +172,7 @@ final class DifferentialInlineCommentQuery
|
||||||
} else {
|
} else {
|
||||||
$changesets = array();
|
$changesets = array();
|
||||||
}
|
}
|
||||||
$changesets += mpull($all, null, 'getID');
|
$changesets += $all_map;
|
||||||
|
|
||||||
$id_map = array();
|
$id_map = array();
|
||||||
foreach ($all as $changeset) {
|
foreach ($all as $changeset) {
|
||||||
|
|
|
@ -478,6 +478,7 @@ final class DifferentialRevision extends DifferentialDAO
|
||||||
public function willRenderTimeline(
|
public function willRenderTimeline(
|
||||||
PhabricatorApplicationTransactionView $timeline,
|
PhabricatorApplicationTransactionView $timeline,
|
||||||
AphrontRequest $request) {
|
AphrontRequest $request) {
|
||||||
|
$viewer = $request->getViewer();
|
||||||
|
|
||||||
$render_data = $timeline->getRenderData();
|
$render_data = $timeline->getRenderData();
|
||||||
$left = $request->getInt('left', idx($render_data, 'left'));
|
$left = $request->getInt('left', idx($render_data, 'left'));
|
||||||
|
@ -491,10 +492,17 @@ final class DifferentialRevision extends DifferentialDAO
|
||||||
$left_diff = $diffs[$left];
|
$left_diff = $diffs[$left];
|
||||||
$right_diff = $diffs[$right];
|
$right_diff = $diffs[$right];
|
||||||
|
|
||||||
|
$old_ids = $request->getStr('old', idx($render_data, 'old'));
|
||||||
|
$new_ids = $request->getStr('new', idx($render_data, 'new'));
|
||||||
|
$old_ids = explode(',', $old_ids);
|
||||||
|
$new_ids = explode(',', $new_ids);
|
||||||
|
|
||||||
$type_inline = DifferentialTransaction::TYPE_INLINE;
|
$type_inline = DifferentialTransaction::TYPE_INLINE;
|
||||||
$changeset_ids = array();
|
$changeset_ids = array_merge($old_ids, $new_ids);
|
||||||
|
$inlines = array();
|
||||||
foreach ($timeline->getTransactions() as $xaction) {
|
foreach ($timeline->getTransactions() as $xaction) {
|
||||||
if ($xaction->getTransactionType() == $type_inline) {
|
if ($xaction->getTransactionType() == $type_inline) {
|
||||||
|
$inlines[] = $xaction->getComment();
|
||||||
$changeset_ids[] = $xaction->getComment()->getChangesetID();
|
$changeset_ids[] = $xaction->getComment()->getChangesetID();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -509,6 +517,22 @@ final class DifferentialRevision extends DifferentialDAO
|
||||||
$changesets = array();
|
$changesets = array();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
foreach ($inlines as $key => $inline) {
|
||||||
|
$inlines[$key] = DifferentialInlineComment::newFromModernComment(
|
||||||
|
$inline);
|
||||||
|
}
|
||||||
|
|
||||||
|
$query = id(new DifferentialInlineCommentQuery())
|
||||||
|
->setViewer($viewer);
|
||||||
|
|
||||||
|
// NOTE: This is a bit sketchy: this method adjusts the inlines as a
|
||||||
|
// side effect, which means it will ultimately adjust the transaction
|
||||||
|
// comments and affect timeline rendering.
|
||||||
|
$query->adjustInlinesForChangesets(
|
||||||
|
$inlines,
|
||||||
|
array_select_keys($changesets, $old_ids),
|
||||||
|
array_select_keys($changesets, $new_ids));
|
||||||
|
|
||||||
return $timeline
|
return $timeline
|
||||||
->setChangesets($changesets)
|
->setChangesets($changesets)
|
||||||
->setRevision($this)
|
->setRevision($this)
|
||||||
|
|
Loading…
Reference in a new issue