Commit ef8dc859 authored by Mike Greiling's avatar Mike Greiling

Merge branch 'tor/feature/highlight-too-large-diffs' into 'master'

When an MR diff is "Too Large", highlight it like other collapsed diffs

See merge request gitlab-org/gitlab!52146
parents 2e911c2d a2262a2c
<script> <script>
import { mapActions, mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml, GlSprintf } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { sprintf } from '~/locale'; import { sprintf } from '~/locale';
import { deprecatedCreateFlash as createFlash } from '~/flash'; import { deprecatedCreateFlash as createFlash } from '~/flash';
...@@ -11,7 +11,7 @@ import DiffFileHeader from './diff_file_header.vue'; ...@@ -11,7 +11,7 @@ import DiffFileHeader from './diff_file_header.vue';
import DiffContent from './diff_content.vue'; import DiffContent from './diff_content.vue';
import { diffViewerErrors } from '~/ide/constants'; import { diffViewerErrors } from '~/ide/constants';
import { collapsedType, isCollapsed } from '../utils/diff_file'; import { collapsedType, isCollapsed, getShortShaFromFile } from '../utils/diff_file';
import { import {
DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_AUTOMATIC_COLLAPSE,
...@@ -29,6 +29,7 @@ export default { ...@@ -29,6 +29,7 @@ export default {
DiffContent, DiffContent,
GlButton, GlButton,
GlLoadingIcon, GlLoadingIcon,
GlSprintf,
}, },
directives: { directives: {
SafeHtml, SafeHtml,
...@@ -83,15 +84,11 @@ export default { ...@@ -83,15 +84,11 @@ export default {
...mapState('diffs', ['currentDiffFileId']), ...mapState('diffs', ['currentDiffFileId']),
...mapGetters(['isNotesFetched']), ...mapGetters(['isNotesFetched']),
...mapGetters('diffs', ['getDiffFileDiscussions']), ...mapGetters('diffs', ['getDiffFileDiscussions']),
viewBlobLink() { viewBlobHref() {
return sprintf( return escape(this.file.view_path);
this.$options.i18n.blobView, },
{ shortSha() {
linkStart: `<a href="${escape(this.file.view_path)}">`, return getShortShaFromFile(this.file);
linkEnd: '</a>',
},
false,
);
}, },
showLoadingIcon() { showLoadingIcon() {
return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
...@@ -100,7 +97,7 @@ export default { ...@@ -100,7 +97,7 @@ export default {
return hasDiff(this.file); return hasDiff(this.file);
}, },
isFileTooLarge() { isFileTooLarge() {
return this.file.viewer.error === diffViewerErrors.too_large; return !this.manuallyCollapsed && this.file.viewer.error === diffViewerErrors.too_large;
}, },
errorMessage() { errorMessage() {
return !this.manuallyCollapsed ? this.file.viewer.error_message : ''; return !this.manuallyCollapsed ? this.file.viewer.error_message : '';
...@@ -323,7 +320,20 @@ export default { ...@@ -323,7 +320,20 @@ export default {
data-testid="loader-icon" data-testid="loader-icon"
/> />
<div v-else-if="errorMessage" class="diff-viewer"> <div v-else-if="errorMessage" class="diff-viewer">
<div v-safe-html="errorMessage" class="nothing-here-block"></div> <div
v-if="isFileTooLarge"
class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base"
>
<p class="gl-mb-5">
{{ $options.i18n.tooLarge }}
</p>
<gl-button data-testid="blob-button" category="secondary" :href="viewBlobHref">
<gl-sprintf :message="$options.i18n.blobView">
<template #commitSha>{{ shortSha }}</template>
</gl-sprintf>
</gl-button>
</div>
<div v-else v-safe-html="errorMessage" class="nothing-here-block"></div>
</div> </div>
<template v-else> <template v-else>
<div <div
......
...@@ -9,7 +9,8 @@ export const DIFF_FILE_HEADER = { ...@@ -9,7 +9,8 @@ export const DIFF_FILE_HEADER = {
}; };
export const DIFF_FILE = { export const DIFF_FILE = {
blobView: __('You can %{linkStart}view the blob%{linkEnd} instead.'), tooLarge: __('MRDiffFile|Changes are too large to be shown.'),
blobView: __('MRDiffFile|View file @ %{commitSha}'),
editInFork: __( editInFork: __(
"You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request.", "You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request.",
), ),
......
import { truncateSha } from '~/lib/utils/text_utility';
import { import {
DIFF_FILE_SYMLINK_MODE, DIFF_FILE_SYMLINK_MODE,
DIFF_FILE_DELETED_MODE, DIFF_FILE_DELETED_MODE,
...@@ -78,3 +80,7 @@ export function isCollapsed(file) { ...@@ -78,3 +80,7 @@ export function isCollapsed(file) {
return collapsedStates[type]; return collapsedStates[type];
} }
export function getShortShaFromFile(file) {
return file.content_sha ? truncateSha(String(file.content_sha)) : null;
}
---
title: When an MR diff is Too Large, highlight it like other collapsed diffs
merge_request: 52146
author:
type: other
...@@ -125,7 +125,7 @@ Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits. ...@@ -125,7 +125,7 @@ Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits.
#### Not expandable patches (too large) #### Not expandable patches (too large)
The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`. The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`.
Users see a `This source diff could not be displayed because it is too large` message. Users see a `Changes are too large to be shown.` message and a button to view only that file in that commit.
```ruby ```ruby
Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
......
...@@ -17226,6 +17226,12 @@ msgstr "" ...@@ -17226,6 +17226,12 @@ msgstr ""
msgid "MRApprovals|Commented by" msgid "MRApprovals|Commented by"
msgstr "" msgstr ""
msgid "MRDiffFile|Changes are too large to be shown."
msgstr ""
msgid "MRDiffFile|View file @ %{commitSha}"
msgstr ""
msgid "MRDiff|Show changes only" msgid "MRDiff|Show changes only"
msgstr "" msgstr ""
...@@ -32501,9 +32507,6 @@ msgstr "" ...@@ -32501,9 +32507,6 @@ msgstr ""
msgid "You are using PostgreSQL %{pg_version_current}, but PostgreSQL %{pg_version_minimum} is required for this version of GitLab. Please upgrade your environment to a supported PostgreSQL version, see %{pg_requirements_url} for details." msgid "You are using PostgreSQL %{pg_version_current}, but PostgreSQL %{pg_version_minimum} is required for this version of GitLab. Please upgrade your environment to a supported PostgreSQL version, see %{pg_requirements_url} for details."
msgstr "" msgstr ""
msgid "You can %{linkStart}view the blob%{linkEnd} instead."
msgstr ""
msgid "You can also create a project from the command line." msgid "You can also create a project from the command line."
msgstr "" msgstr ""
......
...@@ -471,9 +471,11 @@ describe('DiffFile', () => { ...@@ -471,9 +471,11 @@ describe('DiffFile', () => {
await wrapper.vm.$nextTick(); await wrapper.vm.$nextTick();
expect(wrapper.vm.$el.innerText).toContain( const button = wrapper.find('[data-testid="blob-button"]');
'This source diff could not be displayed because it is too large',
); expect(wrapper.text()).toContain('Changes are too large to be shown.');
expect(button.html()).toContain('View file @');
expect(button.attributes('href')).toBe('/file/view/path');
}); });
}); });
}); });
import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; import { prepareRawDiffFile, getShortShaFromFile } from '~/diffs/utils/diff_file';
function getDiffFiles() { function getDiffFiles() {
const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc'; const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc';
...@@ -143,4 +143,15 @@ describe('diff_file utilities', () => { ...@@ -143,4 +143,15 @@ describe('diff_file utilities', () => {
expect(preppedFile).not.toHaveProp('id'); expect(preppedFile).not.toHaveProp('id');
}); });
}); });
describe('getShortShaFromFile', () => {
it.each`
response | cs
${'12345678'} | ${'12345678abcdogcat'}
${null} | ${undefined}
${'hidogcat'} | ${'hidogcatmorethings'}
`('returns $response for a file with { content_sha: $cs }', ({ response, cs }) => {
expect(getShortShaFromFile({ content_sha: cs })).toBe(response);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment