Add latest changes from gitlab-org/gitlab@master

4 files changed, 230 insertions, 10 deletions

diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js
index 266bbc1eadd..4b4a710df2d 100644
--- a/spec/javascripts/diffs/components/app_spec.js
+++ b/spec/javascripts/diffs/components/app_spec.js
@@ -10,6 +10,7 @@ import CompareVersions from '~/diffs/components/compare_versions.vue';
 import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue';
 import CommitWidget from '~/diffs/components/commit_widget.vue';
 import TreeList from '~/diffs/components/tree_list.vue';
+import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants';
 import createDiffsStore from '../create_diffs_store';
 import diffsMockData from '../mock_data/merge_request_diffs';
 
@@ -41,7 +42,6 @@ describe('diffs/components/app', () => {
         changesEmptyStateIllustration: '',
         dismissEndpoint: '',
         showSuggestPopover: true,
-        useSingleDiffStyle: false,
         ...props,
       },
       store,
@@ -53,6 +53,12 @@ describe('diffs/components/app', () => {
     });
   }
 
+  function getOppositeViewType(currentViewType) {
+    return currentViewType === INLINE_DIFF_VIEW_TYPE
+      ? PARALLEL_DIFF_VIEW_TYPE
+      : INLINE_DIFF_VIEW_TYPE;
+  }
+
   beforeEach(() => {
     // setup globals (needed for component to mount :/)
     window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']);
@@ -82,9 +88,146 @@ describe('diffs/components/app', () => {
       spyOn(wrapper.vm, 'startRenderDiffsQueue');
       spyOn(wrapper.vm, 'unwatchDiscussions');
       store.state.diffs.retrievingBatches = true;
+      store.state.diffs.diffFiles = [];
       wrapper.vm.$nextTick(done);
     });
 
+    describe('when the diff view type changes and it should load a single diff view style', () => {
+      const noLinesDiff = {
+        highlighted_diff_lines: [],
+        parallel_diff_lines: [],
+      };
+      const parallelLinesDiff = {
+        highlighted_diff_lines: [],
+        parallel_diff_lines: ['line'],
+      };
+      const inlineLinesDiff = {
+        highlighted_diff_lines: ['line'],
+        parallel_diff_lines: [],
+      };
+      const fullDiff = {
+        highlighted_diff_lines: ['line'],
+        parallel_diff_lines: ['line'],
+      };
+
+      function expectFetchToOccur({
+        vueInstance,
+        done = () => {},
+        batch = false,
+        existingFiles = 1,
+      } = {}) {
+        vueInstance.$nextTick(() => {
+          expect(vueInstance.diffFiles.length).toEqual(existingFiles);
+
+          if (!batch) {
+            expect(vueInstance.fetchDiffFiles).toHaveBeenCalled();
+            expect(vueInstance.fetchDiffFilesBatch).not.toHaveBeenCalled();
+          } else {
+            expect(vueInstance.fetchDiffFiles).not.toHaveBeenCalled();
+            expect(vueInstance.fetchDiffFilesBatch).toHaveBeenCalled();
+          }
+
+          done();
+        });
+      }
+
+      beforeEach(() => {
+        wrapper.vm.glFeatures.singleMrDiffView = true;
+      });
+
+      it('fetches diffs if it has none', done => {
+        wrapper.vm.isLatestVersion = () => false;
+
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, batch: false, existingFiles: 0, done });
+      });
+
+      it('fetches diffs if it has both view styles, but no lines in either', done => {
+        wrapper.vm.isLatestVersion = () => false;
+
+        store.state.diffs.diffFiles.push(noLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, done });
+      });
+
+      it('fetches diffs if it only has inline view style', done => {
+        wrapper.vm.isLatestVersion = () => false;
+
+        store.state.diffs.diffFiles.push(inlineLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, done });
+      });
+
+      it('fetches diffs if it only has parallel view style', done => {
+        wrapper.vm.isLatestVersion = () => false;
+
+        store.state.diffs.diffFiles.push(parallelLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, done });
+      });
+
+      it('fetches batch diffs if it has none', done => {
+        wrapper.vm.glFeatures.diffsBatchLoad = true;
+
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, existingFiles: 0, done });
+      });
+
+      it('fetches batch diffs if it has both view styles, but no lines in either', done => {
+        wrapper.vm.glFeatures.diffsBatchLoad = true;
+
+        store.state.diffs.diffFiles.push(noLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done });
+      });
+
+      it('fetches batch diffs if it only has inline view style', done => {
+        wrapper.vm.glFeatures.diffsBatchLoad = true;
+
+        store.state.diffs.diffFiles.push(inlineLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done });
+      });
+
+      it('fetches batch diffs if it only has parallel view style', done => {
+        wrapper.vm.glFeatures.diffsBatchLoad = true;
+
+        store.state.diffs.diffFiles.push(parallelLinesDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done });
+      });
+
+      it('does not fetch diffs if it has already fetched both styles of diff', () => {
+        wrapper.vm.glFeatures.diffsBatchLoad = false;
+
+        store.state.diffs.diffFiles.push(fullDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expect(wrapper.vm.diffFiles.length).toEqual(1);
+        expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled();
+        expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
+      });
+
+      it('does not fetch batch diffs if it has already fetched both styles of diff', () => {
+        wrapper.vm.glFeatures.diffsBatchLoad = true;
+
+        store.state.diffs.diffFiles.push(fullDiff);
+        store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
+
+        expect(wrapper.vm.diffFiles.length).toEqual(1);
+        expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled();
+        expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
+      });
+    });
+
     it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => {
       wrapper.vm.glFeatures.diffsBatchLoad = false;
       wrapper.vm.fetchData(false);
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index 98a5348c3bc..436d7338361 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -120,7 +120,7 @@ describe('DiffsStoreActions', () => {
 
   describe('fetchDiffFiles', () => {
     it('should fetch diff files', done => {
-      const endpoint = '/fetch/diff/files?w=1';
+      const endpoint = '/fetch/diff/files?view=inline&w=1';
       const mock = new MockAdapter(axios);
       const res = { diff_files: 1, merge_request_diffs: [] };
       mock.onGet(endpoint).reply(200, res);
@@ -128,7 +128,7 @@ describe('DiffsStoreActions', () => {
       testAction(
         fetchDiffFiles,
         {},
-        { endpoint },
+        { endpoint, diffFiles: [], showWhitespace: false, diffViewType: 'inline' },
         [
           { type: types.SET_LOADING, payload: true },
           { type: types.SET_LOADING, payload: false },
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 93dbf03e1ed..24405dcc796 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -52,7 +52,14 @@ describe('DiffsStoreMutations', () => {
 
   describe('SET_DIFF_DATA', () => {
     it('should set diff data type properly', () => {
-      const state = {};
+      const state = {
+        diffFiles: [
+          {
+            content_sha: diffFileMockData.content_sha,
+            file_hash: diffFileMockData.file_hash,
+          },
+        ],
+      };
       const diffMock = {
         diff_files: [diffFileMockData],
       };
@@ -62,9 +69,41 @@ describe('DiffsStoreMutations', () => {
       const firstLine = state.diffFiles[0].parallel_diff_lines[0];
 
       expect(firstLine.right.text).toBeUndefined();
+      expect(state.diffFiles.length).toEqual(1);
       expect(state.diffFiles[0].renderIt).toEqual(true);
       expect(state.diffFiles[0].collapsed).toEqual(false);
     });
+
+    describe('given diffsBatchLoad feature flag is enabled', () => {
+      beforeEach(() => {
+        gon.features = { diffsBatchLoad: true };
+      });
+
+      afterEach(() => {
+        delete gon.features;
+      });
+
+      it('should not modify the existing state', () => {
+        const state = {
+          diffFiles: [
+            {
+              content_sha: diffFileMockData.content_sha,
+              file_hash: diffFileMockData.file_hash,
+              highlighted_diff_lines: [],
+            },
+          ],
+        };
+        const diffMock = {
+          diff_files: [diffFileMockData],
+        };
+
+        mutations[types.SET_DIFF_DATA](state, diffMock);
+
+        // If the batch load is enabled, there shouldn't be any processing
+        // done on the existing state object, so we shouldn't have this.
+        expect(state.diffFiles[0].parallel_diff_lines).toBeUndefined();
+      });
+    });
   });
 
   describe('SET_DIFFSET_DIFF_DATA_BATCH_DATA', () => {
@@ -168,11 +207,17 @@ describe('DiffsStoreMutations', () => {
     it('should update the state with the given data for the given file hash', () => {
       const fileHash = 123;
       const state = {
-        diffFiles: [{}, { file_hash: fileHash, existing_field: 0 }],
+        diffFiles: [{}, { content_sha: 'abc', file_hash: fileHash, existing_field: 0 }],
       };
       const data = {
         diff_files: [
-          { file_hash: fileHash, extra_field: 1, existing_field: 1, viewer: { name: 'text' } },
+          {
+            content_sha: 'abc',
+            file_hash: fileHash,
+            extra_field: 1,
+            existing_field: 1,
+            viewer: { name: 'text' },
+          },
         ],
       };
 
@@ -208,7 +253,7 @@ describe('DiffsStoreMutations', () => {
                   discussions: [],
                 },
                 right: {
-                  line_code: 'ABC_1',
+                  line_code: 'ABC_2',
                   discussions: [],
                 },
               },
@@ -274,7 +319,7 @@ describe('DiffsStoreMutations', () => {
                   discussions: [],
                 },
                 right: {
-                  line_code: 'ABC_1',
+                  line_code: 'ABC_2',
                   discussions: [],
                 },
               },
@@ -352,7 +397,7 @@ describe('DiffsStoreMutations', () => {
                   discussions: [],
                 },
                 right: {
-                  line_code: 'ABC_1',
+                  line_code: 'ABC_2',
                   discussions: [],
                 },
               },
@@ -448,6 +493,7 @@ describe('DiffsStoreMutations', () => {
                 discussions: [],
               },
             ],
+            parallel_diff_lines: [],
           },
         ],
       };
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 65eb4c9d2a3..638b4510221 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -314,11 +314,29 @@ describe('DiffsStoreUtils', () => {
   });
 
   describe('prepareDiffData', () => {
+    let mock;
     let preparedDiff;
+    let splitInlineDiff;
+    let splitParallelDiff;
+    let completedDiff;
 
     beforeEach(() => {
-      preparedDiff = { diff_files: [getDiffFileMock()] };
+      mock = getDiffFileMock();
+      preparedDiff = { diff_files: [mock] };
+      splitInlineDiff = {
+        diff_files: [Object.assign({}, mock, { parallel_diff_lines: undefined })],
+      };
+      splitParallelDiff = {
+        diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })],
+      };
+      completedDiff = {
+        diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })],
+      };
+
       utils.prepareDiffData(preparedDiff);
+      utils.prepareDiffData(splitInlineDiff);
+      utils.prepareDiffData(splitParallelDiff);
+      utils.prepareDiffData(completedDiff, [mock]);
     });
 
     it('sets the renderIt and collapsed attribute on files', () => {
@@ -359,6 +377,19 @@ describe('DiffsStoreUtils', () => {
 
       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('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);
+    });
   });
 
   describe('isDiscussionApplicableToLine', () => {