1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-02 09:58:24 +01:00

When a revision changes more than 1,000 files, don't show the changes on the main page

Summary: Depends on D19295. Ref T13110. Degrade the review UX when users try to interact with changes which are too large to receive human review.

Test Plan: Reduced the "very large" limit, browsed some changes, saw various elements degrade.

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19296
This commit is contained in:
epriestley 2018-04-04 06:42:20 -07:00
parent 8c8e7f07b5
commit 1b363a831e

View file

@ -3,11 +3,20 @@
final class DifferentialRevisionViewController extends DifferentialController { final class DifferentialRevisionViewController extends DifferentialController {
private $revisionID; private $revisionID;
private $veryLargeDiff;
public function shouldAllowPublic() { public function shouldAllowPublic() {
return true; return true;
} }
public function isVeryLargeDiff() {
return $this->veryLargeDiff;
}
public function getVeryLargeDiffLimit() {
return 1000;
}
public function handleRequest(AphrontRequest $request) { public function handleRequest(AphrontRequest $request) {
$viewer = $this->getViewer(); $viewer = $this->getViewer();
$this->revisionID = $request->getURIData('id'); $this->revisionID = $request->getURIData('id');
@ -73,6 +82,10 @@ final class DifferentialRevisionViewController extends DifferentialController {
idx($diffs, $diff_vs), idx($diffs, $diff_vs),
$repository); $repository);
if (count($rendering_references) > $this->getVeryLargeDiffLimit()) {
$this->veryLargeDiff = true;
}
if ($request->getExists('download')) { if ($request->getExists('download')) {
return $this->buildRawDiffResponse( return $this->buildRawDiffResponse(
$revision, $revision,
@ -150,7 +163,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
$warning->appendChild(hsprintf( $warning->appendChild(hsprintf(
'%s <strong>%s</strong>', '%s <strong>%s</strong>',
pht( pht(
'This diff is very large and affects %s files. '. 'This diff is large and affects %s files. '.
'You may load each file individually or ', 'You may load each file individually or ',
new PhutilNumber($count)), new PhutilNumber($count)),
phutil_tag( phutil_tag(
@ -265,35 +278,53 @@ final class DifferentialRevisionViewController extends DifferentialController {
$timeline->setQuoteRef($revision->getMonogram()); $timeline->setQuoteRef($revision->getMonogram());
$changeset_view = id(new DifferentialChangesetListView()) if ($this->isVeryLargeDiff()) {
->setChangesets($changesets) $messages = array();
->setVisibleChangesets($visible_changesets)
->setStandaloneURI('/differential/changeset/') $messages[] = pht(
->setRawFileURIs( 'This very large diff affects more than %s files. Use the %s to '.
'/differential/changeset/?view=old', 'browse changes.',
'/differential/changeset/?view=new') new PhutilNumber($this->getVeryLargeDiffLimit()),
->setUser($viewer) phutil_tag(
->setDiff($target) 'a',
->setRenderingReferences($rendering_references) array(
->setVsMap($vs_map) 'href' => '/differential/diff/'.$target->getID().'/changesets/',
->setWhitespace($whitespace) ),
->setSymbolIndexes($symbol_indexes) phutil_tag('strong', array(), pht('Changeset List'))));
->setTitle(pht('Diff %s', $target->getID()))
->setBackground(PHUIObjectBoxView::BLUE_PROPERTY); $changeset_view = id(new PHUIInfoView())
->setErrors($messages);
} else {
$changeset_view = id(new DifferentialChangesetListView())
->setChangesets($changesets)
->setVisibleChangesets($visible_changesets)
->setStandaloneURI('/differential/changeset/')
->setRawFileURIs(
'/differential/changeset/?view=old',
'/differential/changeset/?view=new')
->setUser($viewer)
->setDiff($target)
->setRenderingReferences($rendering_references)
->setVsMap($vs_map)
->setWhitespace($whitespace)
->setSymbolIndexes($symbol_indexes)
->setTitle(pht('Diff %s', $target->getID()))
->setBackground(PHUIObjectBoxView::BLUE_PROPERTY);
$revision_id = $revision->getID(); $revision_id = $revision->getID();
$inline_list_uri = "/revision/inlines/{$revision_id}/"; $inline_list_uri = "/revision/inlines/{$revision_id}/";
$inline_list_uri = $this->getApplicationURI($inline_list_uri); $inline_list_uri = $this->getApplicationURI($inline_list_uri);
$changeset_view->setInlineListURI($inline_list_uri); $changeset_view->setInlineListURI($inline_list_uri);
if ($repository) { if ($repository) {
$changeset_view->setRepository($repository); $changeset_view->setRepository($repository);
} }
if (!$viewer_is_anonymous) { if (!$viewer_is_anonymous) {
$changeset_view->setInlineCommentControllerURI( $changeset_view->setInlineCommentControllerURI(
'/differential/comment/inline/edit/'.$revision->getID().'/'); '/differential/comment/inline/edit/'.$revision->getID().'/');
}
} }
$broken_diffs = $this->loadHistoryDiffStatus($diffs); $broken_diffs = $this->loadHistoryDiffStatus($diffs);
@ -312,7 +343,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
->setLocalCommits(idx($props, 'local:commits')) ->setLocalCommits(idx($props, 'local:commits'))
->setCommitsForLinks($commits_for_links); ->setCommitsForLinks($commits_for_links);
if ($repository) { if ($repository && !$this->isVeryLargeDiff()) {
$other_revisions = $this->loadOtherRevisions( $other_revisions = $this->loadOtherRevisions(
$changesets, $changesets,
$target, $target,
@ -328,10 +359,14 @@ final class DifferentialRevisionViewController extends DifferentialController {
$this->buildPackageMaps($changesets); $this->buildPackageMaps($changesets);
$toc_view = $this->buildTableOfContents( if ($this->isVeryLargeDiff()) {
$changesets, $toc_view = null;
$visible_changesets, } else {
$target->loadCoverageMap($viewer)); $toc_view = $this->buildTableOfContents(
$changesets,
$visible_changesets,
$target->loadCoverageMap($viewer));
}
// Attach changesets to each reviewer so we can show which Owners package // Attach changesets to each reviewer so we can show which Owners package
// reviewers own no files. // reviewers own no files.
@ -341,12 +376,17 @@ final class DifferentialRevisionViewController extends DifferentialController {
$reviewer->attachChangesets($reviewer_changesets); $reviewer->attachChangesets($reviewer_changesets);
} }
$tab_group = id(new PHUITabGroupView()) $tab_group = id(new PHUITabGroupView());
->addTab(
if ($toc_view) {
$tab_group->addTab(
id(new PHUITabView()) id(new PHUITabView())
->setName(pht('Files')) ->setName(pht('Files'))
->setKey('files') ->setKey('files')
->appendChild($toc_view)) ->appendChild($toc_view));
}
$tab_group
->addTab( ->addTab(
id(new PHUITabView()) id(new PHUITabView())
->setName(pht('History')) ->setName(pht('History'))
@ -479,7 +519,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE); PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE);
$nav = null; $nav = null;
if ($filetree_on) { if ($filetree_on && !$this->isVeryLargeDiff()) {
$collapsed_key = PhabricatorFiletreeVisibleSetting::SETTINGKEY; $collapsed_key = PhabricatorFiletreeVisibleSetting::SETTINGKEY;
$collapsed_value = $viewer->getUserSetting($collapsed_key); $collapsed_value = $viewer->getUserSetting($collapsed_key);