Add latest changes from gitlab-org/gitlab@master

2 files changed, 182 insertions, 103 deletions

diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index af41bcf6f04..7d79dcfbfe3 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/spec/frontend/diffs/store/actions_spec.js
@@ -53,6 +53,7 @@ import * as utils from '~/diffs/store/utils';
 import * as commonUtils from '~/lib/utils/common_utils';
 import { mergeUrlParams } from '~/lib/utils/url_utility';
 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
+import { diffMetadata } from '../mock_data/diff_metadata';
 import createFlash from '~/flash';
 
 jest.mock('~/flash', () => jest.fn());
@@ -259,13 +260,19 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('fetchDiffFilesMeta', () => {
-    it('should fetch diff meta information', done => {
-      const endpointMetadata = '/fetch/diffs_meta?view=inline';
-      const mock = new MockAdapter(axios);
-      const data = { diff_files: [] };
-      const res = { data };
-      mock.onGet(endpointMetadata).reply(200, res);
+    const endpointMetadata = '/fetch/diffs_metadata.json?view=inline';
+    const noFilesData = { ...diffMetadata };
+    let mock;
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+
+      delete noFilesData.diff_files;
 
+      mock.onGet(endpointMetadata).reply(200, diffMetadata);
+    });
+
+    it('should fetch diff meta information', done => {
       testAction(
         fetchDiffFilesMeta,
         {},
@@ -273,8 +280,8 @@ describe('DiffsStoreActions', () => {
         [
           { type: types.SET_LOADING, payload: true },
           { type: types.SET_LOADING, payload: false },
-          { type: types.SET_MERGE_REQUEST_DIFFS, payload: [] },
-          { type: types.SET_DIFF_DATA, payload: { data } },
+          { type: types.SET_MERGE_REQUEST_DIFFS, payload: diffMetadata.merge_request_diffs },
+          { type: types.SET_DIFF_DATA, payload: noFilesData },
         ],
         [],
         () => {
@@ -390,13 +397,18 @@ describe('DiffsStoreActions', () => {
     });
 
     describe('fetchDiffFilesMeta', () => {
-      it('should fetch diff meta information', done => {
-        const endpointMetadata = '/fetch/diffs_meta';
-        const mock = new MockAdapter(axios);
-        const data = { diff_files: [] };
-        const res = { data };
-        mock.onGet(endpointMetadata).reply(200, res);
+      const endpointMetadata = '/fetch/diffs_metadata.json';
+      const noFilesData = { ...diffMetadata };
+      let mock;
 
+      beforeEach(() => {
+        mock = new MockAdapter(axios);
+
+        delete noFilesData.diff_files;
+
+        mock.onGet(endpointMetadata).reply(200, diffMetadata);
+      });
+      it('should fetch diff meta information', done => {
         testAction(
           fetchDiffFilesMeta,
           {},
@@ -404,8 +416,8 @@ describe('DiffsStoreActions', () => {
           [
             { type: types.SET_LOADING, payload: true },
             { type: types.SET_LOADING, payload: false },
-            { type: types.SET_MERGE_REQUEST_DIFFS, payload: [] },
-            { type: types.SET_DIFF_DATA, payload: { data } },
+            { type: types.SET_MERGE_REQUEST_DIFFS, payload: diffMetadata.merge_request_diffs },
+            { type: types.SET_DIFF_DATA, payload: noFilesData },
           ],
           [],
           () => {
diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js
index 296069afa2c..891de45e268 100644
--- a/spec/frontend/diffs/store/utils_spec.js
+++ b/spec/frontend/diffs/store/utils_spec.js
@@ -14,9 +14,11 @@ import {
 } from '~/diffs/constants';
 import { MERGE_REQUEST_NOTEABLE_TYPE } from '~/notes/constants';
 import diffFileMockData from '../mock_data/diff_file';
+import { diffMetadata } from '../mock_data/diff_metadata';
 import { noteableDataMock } from '../../notes/mock_data';
 
 const getDiffFileMock = () => JSON.parse(JSON.stringify(diffFileMockData));
+const getDiffMetadataMock = () => JSON.parse(JSON.stringify(diffMetadata));
 
 describe('DiffsStoreUtils', () => {
   describe('findDiffFile', () => {
@@ -430,112 +432,177 @@ describe('DiffsStoreUtils', () => {
   });
 
   describe('prepareDiffData', () => {
-    let mock;
-    let preparedDiff;
-    let splitInlineDiff;
-    let splitParallelDiff;
-    let completedDiff;
+    describe('for regular diff files', () => {
+      let mock;
+      let preparedDiff;
+      let splitInlineDiff;
+      let splitParallelDiff;
+      let completedDiff;
+
+      beforeEach(() => {
+        mock = getDiffFileMock();
+
+        preparedDiff = { diff_files: [mock] };
+        splitInlineDiff = {
+          diff_files: [{ ...mock, parallel_diff_lines: undefined }],
+        };
+        splitParallelDiff = {
+          diff_files: [{ ...mock, highlighted_diff_lines: undefined }],
+        };
+        completedDiff = {
+          diff_files: [{ ...mock, highlighted_diff_lines: undefined }],
+        };
 
-    beforeEach(() => {
-      mock = getDiffFileMock();
-      preparedDiff = { diff_files: [mock] };
-      splitInlineDiff = {
-        diff_files: [{ ...mock, parallel_diff_lines: undefined }],
-      };
-      splitParallelDiff = {
-        diff_files: [{ ...mock, highlighted_diff_lines: undefined }],
-      };
-      completedDiff = {
-        diff_files: [{ ...mock, highlighted_diff_lines: undefined }],
-      };
+        preparedDiff.diff_files = utils.prepareDiffData(preparedDiff);
+        splitInlineDiff.diff_files = utils.prepareDiffData(splitInlineDiff);
+        splitParallelDiff.diff_files = utils.prepareDiffData(splitParallelDiff);
+        completedDiff.diff_files = utils.prepareDiffData(completedDiff, [mock]);
+      });
 
-      preparedDiff.diff_files = utils.prepareDiffData(preparedDiff);
-      splitInlineDiff.diff_files = utils.prepareDiffData(splitInlineDiff);
-      splitParallelDiff.diff_files = utils.prepareDiffData(splitParallelDiff);
-      completedDiff.diff_files = utils.prepareDiffData(completedDiff, [mock]);
-    });
+      it('sets the renderIt and collapsed attribute on files', () => {
+        const firstParallelDiffLine = preparedDiff.diff_files[0].parallel_diff_lines[2];
 
-    it('sets the renderIt and collapsed attribute on files', () => {
-      const firstParallelDiffLine = preparedDiff.diff_files[0].parallel_diff_lines[2];
+        expect(firstParallelDiffLine.left.discussions.length).toBe(0);
+        expect(firstParallelDiffLine.left).not.toHaveAttr('text');
+        expect(firstParallelDiffLine.right.discussions.length).toBe(0);
+        expect(firstParallelDiffLine.right).not.toHaveAttr('text');
+        const firstParallelChar = firstParallelDiffLine.right.rich_text.charAt(0);
 
-      expect(firstParallelDiffLine.left.discussions.length).toBe(0);
-      expect(firstParallelDiffLine.left).not.toHaveAttr('text');
-      expect(firstParallelDiffLine.right.discussions.length).toBe(0);
-      expect(firstParallelDiffLine.right).not.toHaveAttr('text');
-      const firstParallelChar = firstParallelDiffLine.right.rich_text.charAt(0);
+        expect(firstParallelChar).not.toBe(' ');
+        expect(firstParallelChar).not.toBe('+');
+        expect(firstParallelChar).not.toBe('-');
 
-      expect(firstParallelChar).not.toBe(' ');
-      expect(firstParallelChar).not.toBe('+');
-      expect(firstParallelChar).not.toBe('-');
+        const checkLine = preparedDiff.diff_files[0].highlighted_diff_lines[0];
 
-      const checkLine = preparedDiff.diff_files[0].highlighted_diff_lines[0];
+        expect(checkLine.discussions.length).toBe(0);
+        expect(checkLine).not.toHaveAttr('text');
+        const firstChar = checkLine.rich_text.charAt(0);
 
-      expect(checkLine.discussions.length).toBe(0);
-      expect(checkLine).not.toHaveAttr('text');
-      const firstChar = checkLine.rich_text.charAt(0);
+        expect(firstChar).not.toBe(' ');
+        expect(firstChar).not.toBe('+');
+        expect(firstChar).not.toBe('-');
 
-      expect(firstChar).not.toBe(' ');
-      expect(firstChar).not.toBe('+');
-      expect(firstChar).not.toBe('-');
+        expect(preparedDiff.diff_files[0].renderIt).toBeTruthy();
+        expect(preparedDiff.diff_files[0].collapsed).toBeFalsy();
+      });
 
-      expect(preparedDiff.diff_files[0].renderIt).toBeTruthy();
-      expect(preparedDiff.diff_files[0].collapsed).toBeFalsy();
-    });
+      it('adds line_code to all lines', () => {
+        expect(
+          preparedDiff.diff_files[0].parallel_diff_lines.filter(line => !line.line_code),
+        ).toHaveLength(0);
+      });
 
-    it('adds line_code to all lines', () => {
-      expect(
-        preparedDiff.diff_files[0].parallel_diff_lines.filter(line => !line.line_code),
-      ).toHaveLength(0);
-    });
+      it('uses right line code if left has none', () => {
+        const firstLine = preparedDiff.diff_files[0].parallel_diff_lines[0];
 
-    it('uses right line code if left has none', () => {
-      const firstLine = preparedDiff.diff_files[0].parallel_diff_lines[0];
+        expect(firstLine.line_code).toEqual(firstLine.right.line_code);
+      });
 
-      expect(firstLine.line_code).toEqual(firstLine.right.line_code);
-    });
+      it('guarantees an empty array for both diff styles', () => {
+        expect(splitInlineDiff.diff_files[0].parallel_diff_lines.length).toEqual(0);
+        expect(splitInlineDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0);
+        expect(splitParallelDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0);
+        expect(splitParallelDiff.diff_files[0].highlighted_diff_lines.length).toEqual(0);
+      });
 
-    it('guarantees an empty array for both diff styles', () => {
-      expect(splitInlineDiff.diff_files[0].parallel_diff_lines.length).toEqual(0);
-      expect(splitInlineDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0);
-      expect(splitParallelDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0);
-      expect(splitParallelDiff.diff_files[0].highlighted_diff_lines.length).toEqual(0);
-    });
+      it('merges existing diff files with newly loaded diff files to ensure split diffs are eventually completed', () => {
+        expect(completedDiff.diff_files.length).toEqual(1);
+        expect(completedDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0);
+        expect(completedDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0);
+      });
 
-    it('merges existing diff files with newly loaded diff files to ensure split diffs are eventually completed', () => {
-      expect(completedDiff.diff_files.length).toEqual(1);
-      expect(completedDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0);
-      expect(completedDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0);
-    });
+      it('leaves files in the existing state', () => {
+        const priorFiles = [mock];
+        const fakeNewFile = {
+          ...mock,
+          content_sha: 'ABC',
+          file_hash: 'DEF',
+        };
+        const updatedFilesList = utils.prepareDiffData({ diff_files: [fakeNewFile] }, priorFiles);
 
-    it('leaves files in the existing state', () => {
-      const priorFiles = [mock];
-      const fakeNewFile = {
-        ...mock,
-        content_sha: 'ABC',
-        file_hash: 'DEF',
-      };
-      const updatedFilesList = utils.prepareDiffData({ diff_files: [fakeNewFile] }, priorFiles);
+        expect(updatedFilesList).toEqual([mock, fakeNewFile]);
+      });
 
-      expect(updatedFilesList).toEqual([mock, fakeNewFile]);
+      it('completes an existing split diff without overwriting existing diffs', () => {
+        // The current state has a file that has only loaded inline lines
+        const priorFiles = [{ ...mock, parallel_diff_lines: [] }];
+        // The next (batch) load loads two files: the other half of that file, and a new file
+        const fakeBatch = [
+          { ...mock, highlighted_diff_lines: undefined },
+          { ...mock, highlighted_diff_lines: undefined, content_sha: 'ABC', file_hash: 'DEF' },
+        ];
+        const updatedFilesList = utils.prepareDiffData({ diff_files: fakeBatch }, priorFiles);
+
+        expect(updatedFilesList).toEqual([
+          mock,
+          expect.objectContaining({
+            content_sha: 'ABC',
+            file_hash: 'DEF',
+          }),
+        ]);
+      });
     });
 
-    it('completes an existing split diff without overwriting existing diffs', () => {
-      // The current state has a file that has only loaded inline lines
-      const priorFiles = [{ ...mock, parallel_diff_lines: [] }];
-      // The next (batch) load loads two files: the other half of that file, and a new file
-      const fakeBatch = [
-        { ...mock, highlighted_diff_lines: undefined },
-        { ...mock, highlighted_diff_lines: undefined, content_sha: 'ABC', file_hash: 'DEF' },
-      ];
-      const updatedFilesList = utils.prepareDiffData({ diff_files: fakeBatch }, priorFiles);
+    describe('for diff metadata', () => {
+      let mock;
+      let preparedDiffFiles;
 
-      expect(updatedFilesList).toEqual([
-        mock,
-        expect.objectContaining({
-          content_sha: 'ABC',
-          file_hash: 'DEF',
-        }),
-      ]);
+      beforeEach(() => {
+        mock = getDiffMetadataMock();
+
+        preparedDiffFiles = utils.prepareDiffData(mock);
+      });
+
+      it('sets the renderIt and collapsed attribute on files', () => {
+        expect(preparedDiffFiles[0].renderIt).toBeTruthy();
+        expect(preparedDiffFiles[0].collapsed).toBeFalsy();
+      });
+
+      it('guarantees an empty array of lines for both diff styles', () => {
+        expect(preparedDiffFiles[0].parallel_diff_lines.length).toEqual(0);
+        expect(preparedDiffFiles[0].highlighted_diff_lines.length).toEqual(0);
+      });
+
+      it('leaves files in the existing state', () => {
+        const fileMock = getDiffFileMock();
+        const metaData = getDiffMetadataMock();
+        const priorFiles = [fileMock];
+        const updatedFilesList = utils.prepareDiffData(metaData, priorFiles);
+
+        expect(updatedFilesList.length).toEqual(2);
+        expect(updatedFilesList[0]).toEqual(fileMock);
+      });
+
+      it('adds a new file to the file that already exists in state', () => {
+        // This is actually buggy behavior:
+        // Because the metadata doesn't include a content_sha,
+        // the de-duplicator in prepareDiffData doesn't realize it
+        // should combine these two.
+
+        // This buggy behavior hasn't caused a defect YET, because
+        // `diffs_metadata.json` is only called the first time the
+        // diffs app starts up, which is:
+        // - after a fresh page load
+        // - after you switch to the changes tab *the first time*
+
+        // This test should begin FAILING and can be reversed to check
+        // for just a single file when this is implemented:
+        // https://gitlab.com/groups/gitlab-org/-/epics/2852#note_304803233
+
+        const fileMock = getDiffFileMock();
+        const metaMock = getDiffMetadataMock();
+        const priorFiles = [{ ...fileMock }];
+        const updatedFilesList = utils.prepareDiffData(metaMock, priorFiles);
+
+        expect(updatedFilesList).toEqual([
+          fileMock,
+          {
+            ...metaMock.diff_files[0],
+            highlighted_diff_lines: [],
+            parallel_diff_lines: [],
+          },
+        ]);
+      });
     });
   });