Merge branch 'tor/feature/show-bytes-for-undiffable' into 'master'

Show the bytes for file changes when the file is not diffable

See merge request gitlab-org/gitlab!62445

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -354,7 +354,11 @@ export default {
       v-if="!diffFile.submodule && addMergeRequestButtons"
       class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start"
     >
-      <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" />
+      <diff-stats
+        :diff-file="diffFile"
+        :added-lines="diffFile.added_lines"
+        :removed-lines="diffFile.removed_lines"
+      />
       <gl-form-checkbox
         v-if="isReviewable && showLocalFileReviews"
         v-gl-tooltip.hover

app/assets/javascripts/diffs/components/diff_stats.vue

@@ -2,10 +2,16 @@
 import { GlIcon } from '@gitlab/ui';
 import { isNumber } from 'lodash';
 import { n__ } from '~/locale';
+import { isNotDiffable, stats } from '../utils/diff_file';
 
 export default {
   components: { GlIcon },
   props: {
+    diffFile: {
+      type: Object,
+      required: false,
+      default: () => null,
+    },
     addedLines: {
       type: Number,
       required: true,
@@ -33,6 +39,12 @@ export default {
     hasDiffFiles() {
       return isNumber(this.diffFilesLength) && this.diffFilesLength >= 0;
     },
+    notDiffable() {
+      return isNotDiffable(this.diffFile);
+    },
+    fileStats() {
+      return stats(this.diffFile);
+    },
   },
 };
 </script>
@@ -45,23 +57,28 @@ export default {
       'd-none d-sm-inline-flex': !isCompareVersionsHeader,
     }"
   >
-    <div v-if="hasDiffFiles" class="diff-stats-group">
-      <gl-icon name="doc-code" class="diff-stats-icon text-secondary" />
-      <span class="text-secondary bold">{{ diffFilesCountText }} {{ filesText }}</span>
-    </div>
-    <div
-      class="diff-stats-group cgreen d-flex align-items-center"
-      :class="{ bold: isCompareVersionsHeader }"
-    >
-      <span>+</span>
-      <span class="js-file-addition-line">{{ addedLines }}</span>
+    <div v-if="notDiffable" :class="fileStats.classes">
+      {{ fileStats.text }}
     </div>
-    <div
-      class="diff-stats-group cred d-flex align-items-center"
-      :class="{ bold: isCompareVersionsHeader }"
-    >
-      <span>-</span>
-      <span class="js-file-deletion-line">{{ removedLines }}</span>
+    <div v-else class="diff-stats-contents">
+      <div v-if="hasDiffFiles" class="diff-stats-group">
+        <gl-icon name="doc-code" class="diff-stats-icon text-secondary" />
+        <span class="text-secondary bold">{{ diffFilesCountText }} {{ filesText }}</span>
+      </div>
+      <div
+        class="diff-stats-group cgreen d-flex align-items-center"
+        :class="{ bold: isCompareVersionsHeader }"
+      >
+        <span>+</span>
+        <span class="js-file-addition-line">{{ addedLines }}</span>
+      </div>
+      <div
+        class="diff-stats-group cred d-flex align-items-center"
+        :class="{ bold: isCompareVersionsHeader }"
+      >
+        <span>-</span>
+        <span class="js-file-deletion-line">{{ removedLines }}</span>
+      </div>
     </div>
   </div>
 </template>

app/assets/javascripts/diffs/utils/diff_file.js

+import { diffViewerModes as viewerModes } from '~/ide/constants';
+import { changeInPercent, numberToHumanSize } from '~/lib/utils/number_utils';
 import { truncateSha } from '~/lib/utils/text_utility';
 import { uuids } from '~/lib/utils/uuids';
 
@@ -46,6 +48,8 @@ function identifier(file) {
   })[0];
 }
 
+export const isNotDiffable = (file) => file?.viewer?.name === viewerModes.not_diffable;
+
 export function prepareRawDiffFile({ file, allFiles, meta = false }) {
   const additionalProperties = {
     brokenSymlink: fileSymlinkInformation(file, allFiles),
@@ -84,3 +88,35 @@ export function isCollapsed(file) {
 export function getShortShaFromFile(file) {
   return file.content_sha ? truncateSha(String(file.content_sha)) : null;
 }
+
+export function stats(file) {
+  let valid = false;
+  let classes = '';
+  let sign = '';
+  let text = '';
+  let percent = 0;
+  let diff = 0;
+
+  if (file) {
+    percent = changeInPercent(file.old_size, file.new_size);
+    diff = file.new_size - file.old_size;
+    sign = diff >= 0 ? '+' : '';
+    text = `${sign}${numberToHumanSize(diff)} (${sign}${percent}%)`;
+    valid = true;
+
+    if (diff > 0) {
+      classes = 'cgreen';
+    } else if (diff < 0) {
+      classes = 'cred';
+    }
+  }
+
+  return {
+    changed: diff,
+    text,
+    percent,
+    classes,
+    sign,
+    valid,
+  };
+}

app/assets/stylesheets/framework/diffs.scss

@@ -646,6 +646,10 @@ table.code {
   align-items: center;
   padding: 0 1rem;
 
+  .diff-stats-contents {
+    display: contents;
+  }
+
   .diff-stats-group {
     padding: 0 0.25rem;
   }

spec/frontend/diffs/components/diff_stats_spec.js

 import { GlIcon } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import DiffStats from '~/diffs/components/diff_stats.vue';
+import mockDiffFile from '../mock_data/diff_file';
 
 const TEST_ADDED_LINES = 100;
 const TEST_REMOVED_LINES = 200;
@@ -38,9 +39,37 @@ describe('diff_stats', () => {
     });
   });
 
+  describe('bytes changes', () => {
+    let file;
+    const getBytesContainer = () => wrapper.find('.diff-stats > div:first-child');
+
+    beforeEach(() => {
+      file = {
+        ...mockDiffFile,
+        viewer: {
+          ...mockDiffFile.viewer,
+          name: 'not_diffable',
+        },
+      };
+
+      createComponent({ diffFile: file });
+    });
+
+    it("renders the bytes changes instead of line changes when the file isn't diffable", () => {
+      const content = getBytesContainer();
+
+      expect(content.classes('cgreen')).toBe(true);
+      expect(content.text()).toBe('+1.00 KiB (+100%)');
+    });
+  });
+
   describe('line changes', () => {
     const findFileLine = (name) => wrapper.find(name);
 
+    beforeEach(() => {
+      createComponent();
+    });
+
     it('shows the amount of lines added', () => {
       expect(findFileLine('.js-file-addition-line').text()).toBe(TEST_ADDED_LINES.toString());
     });

spec/frontend/diffs/mock_data/diff_file.js

@@ -19,6 +19,8 @@ export default {
   renamed_file: false,
   old_path: 'CHANGELOG',
   new_path: 'CHANGELOG',
+  old_size: 1024,
+  new_size: 2048,
   mode_changed: false,
   a_mode: '100644',
   b_mode: '100644',

spec/frontend/diffs/utils/diff_file_spec.js

-import { prepareRawDiffFile, getShortShaFromFile } from '~/diffs/utils/diff_file';
+import {
+  prepareRawDiffFile,
+  getShortShaFromFile,
+  stats,
+  isNotDiffable,
+} from '~/diffs/utils/diff_file';
+import { diffViewerModes } from '~/ide/constants';
+import mockDiffFile from '../mock_data/diff_file';
 
 function getDiffFiles() {
   const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc';
@@ -154,4 +161,73 @@ describe('diff_file utilities', () => {
       expect(getShortShaFromFile({ content_sha: cs })).toBe(response);
     });
   });
+
+  describe('stats', () => {
+    const noFile = [
+      "returns empty stats when the file isn't provided",
+      undefined,
+      {
+        text: '',
+        percent: 0,
+        changed: 0,
+        classes: '',
+        sign: '',
+        valid: false,
+      },
+    ];
+    const validFile = [
+      'computes the correct stats from a file',
+      mockDiffFile,
+      {
+        changed: 1024,
+        percent: 100,
+        classes: 'cgreen',
+        sign: '+',
+        text: '+1.00 KiB (+100%)',
+        valid: true,
+      },
+    ];
+    const negativeChange = [
+      'computed the correct states from a file with a negative size change',
+      {
+        ...mockDiffFile,
+        new_size: 0,
+        old_size: 1024,
+      },
+      {
+        changed: -1024,
+        percent: -100,
+        classes: 'cred',
+        sign: '',
+        text: '-1.00 KiB (-100%)',
+        valid: true,
+      },
+    ];
+
+    it.each([noFile, validFile, negativeChange])('%s', (_, file, output) => {
+      expect(stats(file)).toEqual(output);
+    });
+  });
+
+  describe('isNotDiffable', () => {
+    it.each`
+      bool     | vw
+      ${true}  | ${diffViewerModes.not_diffable}
+      ${false} | ${diffViewerModes.text}
+      ${false} | ${diffViewerModes.image}
+    `('returns $bool when the viewer is $vw', ({ bool, vw }) => {
+      expect(isNotDiffable({ viewer: { name: vw } })).toBe(bool);
+    });
+
+    it.each`
+      file
+      ${undefined}
+      ${null}
+      ${{}}
+      ${{ viewer: undefined }}
+      ${{ viewer: null }}
+    `('reports `false` when the file is `$file`', ({ file }) => {
+      expect(isNotDiffable(file)).toBe(false);
+    });
+  });
 });