From a2262a2c10ee8d22a19a8caf3c038ce59dbf5f99 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Tue, 26 Jan 2021 12:06:32 -0700
Subject: [PATCH] Highlight diffs that are too large to display

This also cleans up past refactors that accidentally
removed the special-case handling for `too large`
files entirely, leaving it to simply render the HTML
returned by the backend in the `error_message`
property.

`too large` files now have the correct "special"
handling, updated with the new styles.
---
 .../diffs/components/diff_file.vue            | 36 ++++++++++++-------
 app/assets/javascripts/diffs/i18n.js          |  3 +-
 .../javascripts/diffs/utils/diff_file.js      |  6 ++++
 .../tor-feature-highlight-too-large-diffs.yml |  5 +++
 doc/development/diffs.md                      |  2 +-
 locale/gitlab.pot                             |  9 +++--
 .../diffs/components/diff_file_spec.js        |  8 +++--
 spec/frontend/diffs/utils/diff_file_spec.js   | 13 ++++++-
 8 files changed, 60 insertions(+), 22 deletions(-)
 create mode 100644 changelogs/unreleased/tor-feature-highlight-too-large-diffs.yml

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index f5a3d69843ae61..269ad7694b7e27 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -1,7 +1,7 @@
 <script>
 import { mapActions, mapGetters, mapState } from 'vuex';
 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 { sprintf } from '~/locale';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
@@ -11,7 +11,7 @@ import DiffFileHeader from './diff_file_header.vue';
 import DiffContent from './diff_content.vue';
 import { diffViewerErrors } from '~/ide/constants';
 
-import { collapsedType, isCollapsed } from '../utils/diff_file';
+import { collapsedType, isCollapsed, getShortShaFromFile } from '../utils/diff_file';
 
 import {
   DIFF_FILE_AUTOMATIC_COLLAPSE,
@@ -29,6 +29,7 @@ export default {
     DiffContent,
     GlButton,
     GlLoadingIcon,
+    GlSprintf,
   },
   directives: {
     SafeHtml,
@@ -83,15 +84,11 @@ export default {
     ...mapState('diffs', ['currentDiffFileId']),
     ...mapGetters(['isNotesFetched']),
     ...mapGetters('diffs', ['getDiffFileDiscussions']),
-    viewBlobLink() {
-      return sprintf(
-        this.$options.i18n.blobView,
-        {
-          linkStart: `<a href="${escape(this.file.view_path)}">`,
-          linkEnd: '</a>',
-        },
-        false,
-      );
+    viewBlobHref() {
+      return escape(this.file.view_path);
+    },
+    shortSha() {
+      return getShortShaFromFile(this.file);
     },
     showLoadingIcon() {
       return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
@@ -100,7 +97,7 @@ export default {
       return hasDiff(this.file);
     },
     isFileTooLarge() {
-      return this.file.viewer.error === diffViewerErrors.too_large;
+      return !this.manuallyCollapsed && this.file.viewer.error === diffViewerErrors.too_large;
     },
     errorMessage() {
       return !this.manuallyCollapsed ? this.file.viewer.error_message : '';
@@ -323,7 +320,20 @@ export default {
           data-testid="loader-icon"
         />
         <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>
         <template v-else>
           <div
diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js
index 0e21d5042c2a16..d719f4e5352dd9 100644
--- a/app/assets/javascripts/diffs/i18n.js
+++ b/app/assets/javascripts/diffs/i18n.js
@@ -9,7 +9,8 @@ export const DIFF_FILE_HEADER = {
 };
 
 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: __(
     "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.",
   ),
diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js
index ce0398e75fcba8..7e6fde320d2689 100644
--- a/app/assets/javascripts/diffs/utils/diff_file.js
+++ b/app/assets/javascripts/diffs/utils/diff_file.js
@@ -1,3 +1,5 @@
+import { truncateSha } from '~/lib/utils/text_utility';
+
 import {
   DIFF_FILE_SYMLINK_MODE,
   DIFF_FILE_DELETED_MODE,
@@ -78,3 +80,7 @@ export function isCollapsed(file) {
 
   return collapsedStates[type];
 }
+
+export function getShortShaFromFile(file) {
+  return file.content_sha ? truncateSha(String(file.content_sha)) : null;
+}
diff --git a/changelogs/unreleased/tor-feature-highlight-too-large-diffs.yml b/changelogs/unreleased/tor-feature-highlight-too-large-diffs.yml
new file mode 100644
index 00000000000000..1a26e2d41718c1
--- /dev/null
+++ b/changelogs/unreleased/tor-feature-highlight-too-large-diffs.yml
@@ -0,0 +1,5 @@
+---
+title: When an MR diff is Too Large, highlight it like other collapsed diffs
+merge_request: 52146
+author:
+type: other
diff --git a/doc/development/diffs.md b/doc/development/diffs.md
index fba8eda0408fee..7a5e4b7a74781b 100644
--- a/doc/development/diffs.md
+++ b/doc/development/diffs.md
@@ -125,7 +125,7 @@ Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits.
 #### Not expandable patches (too large)
 
 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
 Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 484a4cf94b6111..937f32b2188640 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -17208,6 +17208,12 @@ msgstr ""
 msgid "MRApprovals|Commented by"
 msgstr ""
 
+msgid "MRDiffFile|Changes are too large to be shown."
+msgstr ""
+
+msgid "MRDiffFile|View file @ %{commitSha}"
+msgstr ""
+
 msgid "MRDiff|Show changes only"
 msgstr ""
 
@@ -32426,9 +32432,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."
 msgstr ""
 
-msgid "You can %{linkStart}view the blob%{linkEnd} instead."
-msgstr ""
-
 msgid "You can also create a project from the command line."
 msgstr ""
 
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index 57f6e50e68ae0f..9ca466183a39d7 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -471,9 +471,11 @@ describe('DiffFile', () => {
 
       await wrapper.vm.$nextTick();
 
-      expect(wrapper.vm.$el.innerText).toContain(
-        'This source diff could not be displayed because it is too large',
-      );
+      const button = wrapper.find('[data-testid="blob-button"]');
+
+      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');
     });
   });
 });
diff --git a/spec/frontend/diffs/utils/diff_file_spec.js b/spec/frontend/diffs/utils/diff_file_spec.js
index 2de8db28e71cdf..c6cfdfced65079 100644
--- a/spec/frontend/diffs/utils/diff_file_spec.js
+++ b/spec/frontend/diffs/utils/diff_file_spec.js
@@ -1,4 +1,4 @@
-import { prepareRawDiffFile } from '~/diffs/utils/diff_file';
+import { prepareRawDiffFile, getShortShaFromFile } from '~/diffs/utils/diff_file';
 
 function getDiffFiles() {
   const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc';
@@ -143,4 +143,15 @@ describe('diff_file utilities', () => {
       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);
+    });
+  });
 });
-- 
GitLab