Merge branch '55347-mr-diffs-file-count-increments-while-batch-loading' into 'master'

Resolve "MR diffs file count increments while batch loading"

Closes #55347

See merge request gitlab-org/gitlab!21764

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

@@ -95,6 +95,7 @@ export default {
 
     return {
       treeWidth,
+      diffFilesLength: 0,
     };
   },
   computed: {
@@ -241,7 +242,8 @@ export default {
     fetchData(toggleTree = true) {
       if (this.glFeatures.diffsBatchLoad) {
         this.fetchDiffFilesMeta()
-          .then(() => {
+          .then(({ real_size }) => {
+            this.diffFilesLength = parseInt(real_size, 10);
             if (toggleTree) this.hideTreeListIfJustOneFile();
 
             this.startDiffRendering();
@@ -264,7 +266,8 @@ export default {
           });
       } else {
         this.fetchDiffFiles()
-          .then(() => {
+          .then(({ real_size }) => {
+            this.diffFilesLength = parseInt(real_size, 10);
             if (toggleTree) {
               this.hideTreeListIfJustOneFile();
             }
@@ -351,6 +354,7 @@ export default {
         :merge-request-diff="mergeRequestDiff"
         :target-branch="targetBranch"
         :is-limited-container="isLimitedContainer"
+        :diff-files-length="diffFilesLength"
       />
 
       <hidden-files-warning

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

@@ -42,9 +42,13 @@ export default {
       required: false,
       default: false,
     },
+    diffFilesLength: {
+      type: Number,
+      required: true,
+    },
   },
   computed: {
-    ...mapGetters('diffs', ['hasCollapsedFile', 'diffFilesLength']),
+    ...mapGetters('diffs', ['hasCollapsedFile']),
     ...mapState('diffs', [
       'commit',
       'showTreeList',

app/assets/javascripts/diffs/store/actions.js

@@ -64,6 +64,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
   const urlParams = {
     w: state.showWhitespace ? '0' : '1',
   };
+  let returnData;
 
   if (state.useSingleDiffStyle) {
     urlParams.view = state.diffViewType;
@@ -87,9 +88,13 @@ export const fetchDiffFiles = ({ state, commit }) => {
 
       worker.postMessage(state.diffFiles);
 
+      returnData = res.data;
       return Vue.nextTick();
     })
-    .then(handleLocationHash)
+    .then(() => {
+      handleLocationHash();
+      return returnData;
+    })
     .catch(() => worker.terminate());
 };
 
@@ -147,6 +152,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
 
       prepareDiffData(data);
       worker.postMessage(data.diff_files);
+      return data;
     })
     .catch(() => worker.terminate());
 };

app/assets/javascripts/diffs/store/getters.js

@@ -95,8 +95,6 @@ export const allBlobs = (state, getters) =>
     return acc;
   }, []);
 
-export const diffFilesLength = state => state.diffFiles.length;
-
 export const getCommentFormForDiffFile = state => fileHash =>
   state.commentForms.find(form => form.fileHash === fileHash);
 

app/assets/javascripts/diffs/store/mutations.js

@@ -179,16 +179,19 @@ export default {
     const mapDiscussions = (line, extraCheck = () => true) => ({
       ...line,
       discussions: extraCheck()
-        ? line.discussions
+        ? line.discussions &&
+          line.discussions
             .filter(() => !line.discussions.some(({ id }) => discussion.id === id))
             .concat(lineCheck(line) ? discussion : line.discussions)
         : [],
     });
 
     const setDiscussionsExpanded = line => {
-      const isLineNoteTargeted = line.discussions.some(
-        disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`),
-      );
+      const isLineNoteTargeted =
+        line.discussions &&
+        line.discussions.some(
+          disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`),
+        );
 
       return {
         ...line,

changelogs/unreleased/55347-mr-diffs-file-count-increments-while-batch-loading.yml

+---
+title: Fix MR diffs file count increments while batch loading
+merge_request: 21764
+author:
+type: fixed

spec/frontend/diffs/components/compare_versions_spec.js

@@ -28,6 +28,7 @@ describe('CompareVersions', () => {
       propsData: {
         mergeRequestDiffs: diffsMockData,
         mergeRequestDiff: diffsMockData[0],
+        diffFilesLength: 0,
         targetBranch,
         ...props,
       },

spec/javascripts/diffs/components/app_spec.js

@@ -77,7 +77,7 @@ describe('diffs/components/app', () => {
     beforeEach(done => {
       const fetchResolver = () => {
         store.state.diffs.retrievingBatches = false;
-        return Promise.resolve();
+        return Promise.resolve({ real_size: 100 });
       };
       spyOn(window, 'requestIdleCallback').and.callFake(fn => fn());
       createComponent();
@@ -229,6 +229,7 @@ describe('diffs/components/app', () => {
     });
 
     it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => {
+      expect(wrapper.vm.diffFilesLength).toEqual(0);
       wrapper.vm.glFeatures.diffsBatchLoad = false;
       wrapper.vm.fetchData(false);
 
@@ -238,12 +239,14 @@ describe('diffs/components/app', () => {
         expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
         expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
+        expect(wrapper.vm.diffFilesLength).toEqual(100);
 
         done();
       });
     });
 
     it('calls batch methods if diffsBatchLoad is enabled, and not latest version', done => {
+      expect(wrapper.vm.diffFilesLength).toEqual(0);
       wrapper.vm.glFeatures.diffsBatchLoad = true;
       wrapper.vm.isLatestVersion = () => false;
       wrapper.vm.fetchData(false);
@@ -254,11 +257,13 @@ describe('diffs/components/app', () => {
         expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();
         expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
+        expect(wrapper.vm.diffFilesLength).toEqual(100);
         done();
       });
     });
 
     it('calls batch methods if diffsBatchLoad is enabled, and latest version', done => {
+      expect(wrapper.vm.diffFilesLength).toEqual(0);
       wrapper.vm.glFeatures.diffsBatchLoad = true;
       wrapper.vm.fetchData(false);
 
@@ -268,6 +273,7 @@ describe('diffs/components/app', () => {
         expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();
         expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
+        expect(wrapper.vm.diffFilesLength).toEqual(100);
         done();
       });
     });

spec/javascripts/diffs/store/actions_spec.js

@@ -141,6 +141,13 @@ describe('DiffsStoreActions', () => {
           done();
         },
       );
+
+      fetchDiffFiles({ state: { endpoint }, commit: () => null })
+        .then(data => {
+          expect(data).toEqual(res);
+          done();
+        })
+        .catch(done.fail);
     });
   });
 

spec/javascripts/diffs/store/getters_spec.js

@@ -263,14 +263,6 @@ describe('Diffs Module Getters', () => {
     });
   });
 
-  describe('diffFilesLength', () => {
-    it('returns length of diff files', () => {
-      localState.diffFiles.push('test', 'test 2');
-
-      expect(getters.diffFilesLength(localState)).toBe(2);
-    });
-  });
-
   describe('currentDiffIndex', () => {
     it('returns index of currently selected diff in diffList', () => {
       localState.diffFiles = [{ file_hash: '111' }, { file_hash: '222' }, { file_hash: '333' }];