Commit edaa0a3e authored by Justin Boyson's avatar Justin Boyson Committed by Fatih Acet

Update diffs app to use file count from api

Update actions to return data
Remove diffFilesLength getter
Use local state and props for diffFilesLength
Update tests
parent 62b58078
...@@ -95,6 +95,7 @@ export default { ...@@ -95,6 +95,7 @@ export default {
return { return {
treeWidth, treeWidth,
diffFilesLength: 0,
}; };
}, },
computed: { computed: {
...@@ -241,7 +242,8 @@ export default { ...@@ -241,7 +242,8 @@ export default {
fetchData(toggleTree = true) { fetchData(toggleTree = true) {
if (this.glFeatures.diffsBatchLoad) { if (this.glFeatures.diffsBatchLoad) {
this.fetchDiffFilesMeta() this.fetchDiffFilesMeta()
.then(() => { .then(({ real_size }) => {
this.diffFilesLength = parseInt(real_size, 10);
if (toggleTree) this.hideTreeListIfJustOneFile(); if (toggleTree) this.hideTreeListIfJustOneFile();
this.startDiffRendering(); this.startDiffRendering();
...@@ -264,7 +266,8 @@ export default { ...@@ -264,7 +266,8 @@ export default {
}); });
} else { } else {
this.fetchDiffFiles() this.fetchDiffFiles()
.then(() => { .then(({ real_size }) => {
this.diffFilesLength = parseInt(real_size, 10);
if (toggleTree) { if (toggleTree) {
this.hideTreeListIfJustOneFile(); this.hideTreeListIfJustOneFile();
} }
...@@ -351,6 +354,7 @@ export default { ...@@ -351,6 +354,7 @@ export default {
:merge-request-diff="mergeRequestDiff" :merge-request-diff="mergeRequestDiff"
:target-branch="targetBranch" :target-branch="targetBranch"
:is-limited-container="isLimitedContainer" :is-limited-container="isLimitedContainer"
:diff-files-length="diffFilesLength"
/> />
<hidden-files-warning <hidden-files-warning
......
...@@ -42,9 +42,13 @@ export default { ...@@ -42,9 +42,13 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
diffFilesLength: {
type: Number,
required: true,
},
}, },
computed: { computed: {
...mapGetters('diffs', ['hasCollapsedFile', 'diffFilesLength']), ...mapGetters('diffs', ['hasCollapsedFile']),
...mapState('diffs', [ ...mapState('diffs', [
'commit', 'commit',
'showTreeList', 'showTreeList',
......
...@@ -64,6 +64,7 @@ export const fetchDiffFiles = ({ state, commit }) => { ...@@ -64,6 +64,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
const urlParams = { const urlParams = {
w: state.showWhitespace ? '0' : '1', w: state.showWhitespace ? '0' : '1',
}; };
let returnData;
if (state.useSingleDiffStyle) { if (state.useSingleDiffStyle) {
urlParams.view = state.diffViewType; urlParams.view = state.diffViewType;
...@@ -87,9 +88,13 @@ export const fetchDiffFiles = ({ state, commit }) => { ...@@ -87,9 +88,13 @@ export const fetchDiffFiles = ({ state, commit }) => {
worker.postMessage(state.diffFiles); worker.postMessage(state.diffFiles);
returnData = res.data;
return Vue.nextTick(); return Vue.nextTick();
}) })
.then(handleLocationHash) .then(() => {
handleLocationHash();
return returnData;
})
.catch(() => worker.terminate()); .catch(() => worker.terminate());
}; };
...@@ -147,6 +152,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { ...@@ -147,6 +152,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
prepareDiffData(data); prepareDiffData(data);
worker.postMessage(data.diff_files); worker.postMessage(data.diff_files);
return data;
}) })
.catch(() => worker.terminate()); .catch(() => worker.terminate());
}; };
......
...@@ -95,8 +95,6 @@ export const allBlobs = (state, getters) => ...@@ -95,8 +95,6 @@ export const allBlobs = (state, getters) =>
return acc; return acc;
}, []); }, []);
export const diffFilesLength = state => state.diffFiles.length;
export const getCommentFormForDiffFile = state => fileHash => export const getCommentFormForDiffFile = state => fileHash =>
state.commentForms.find(form => form.fileHash === fileHash); state.commentForms.find(form => form.fileHash === fileHash);
......
...@@ -179,14 +179,17 @@ export default { ...@@ -179,14 +179,17 @@ export default {
const mapDiscussions = (line, extraCheck = () => true) => ({ const mapDiscussions = (line, extraCheck = () => true) => ({
...line, ...line,
discussions: extraCheck() discussions: extraCheck()
? line.discussions ? line.discussions &&
line.discussions
.filter(() => !line.discussions.some(({ id }) => discussion.id === id)) .filter(() => !line.discussions.some(({ id }) => discussion.id === id))
.concat(lineCheck(line) ? discussion : line.discussions) .concat(lineCheck(line) ? discussion : line.discussions)
: [], : [],
}); });
const setDiscussionsExpanded = line => { const setDiscussionsExpanded = line => {
const isLineNoteTargeted = line.discussions.some( const isLineNoteTargeted =
line.discussions &&
line.discussions.some(
disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`), disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`),
); );
......
---
title: Fix MR diffs file count increments while batch loading
merge_request: 21764
author:
type: fixed
...@@ -28,6 +28,7 @@ describe('CompareVersions', () => { ...@@ -28,6 +28,7 @@ describe('CompareVersions', () => {
propsData: { propsData: {
mergeRequestDiffs: diffsMockData, mergeRequestDiffs: diffsMockData,
mergeRequestDiff: diffsMockData[0], mergeRequestDiff: diffsMockData[0],
diffFilesLength: 0,
targetBranch, targetBranch,
...props, ...props,
}, },
......
...@@ -77,7 +77,7 @@ describe('diffs/components/app', () => { ...@@ -77,7 +77,7 @@ describe('diffs/components/app', () => {
beforeEach(done => { beforeEach(done => {
const fetchResolver = () => { const fetchResolver = () => {
store.state.diffs.retrievingBatches = false; store.state.diffs.retrievingBatches = false;
return Promise.resolve(); return Promise.resolve({ real_size: 100 });
}; };
spyOn(window, 'requestIdleCallback').and.callFake(fn => fn()); spyOn(window, 'requestIdleCallback').and.callFake(fn => fn());
createComponent(); createComponent();
...@@ -229,6 +229,7 @@ describe('diffs/components/app', () => { ...@@ -229,6 +229,7 @@ describe('diffs/components/app', () => {
}); });
it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => { it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => {
expect(wrapper.vm.diffFilesLength).toEqual(0);
wrapper.vm.glFeatures.diffsBatchLoad = false; wrapper.vm.glFeatures.diffsBatchLoad = false;
wrapper.vm.fetchData(false); wrapper.vm.fetchData(false);
...@@ -238,12 +239,14 @@ describe('diffs/components/app', () => { ...@@ -238,12 +239,14 @@ describe('diffs/components/app', () => {
expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
expect(wrapper.vm.diffFilesLength).toEqual(100);
done(); done();
}); });
}); });
it('calls batch methods if diffsBatchLoad is enabled, and not latest version', 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.glFeatures.diffsBatchLoad = true;
wrapper.vm.isLatestVersion = () => false; wrapper.vm.isLatestVersion = () => false;
wrapper.vm.fetchData(false); wrapper.vm.fetchData(false);
...@@ -254,11 +257,13 @@ describe('diffs/components/app', () => { ...@@ -254,11 +257,13 @@ describe('diffs/components/app', () => {
expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();
expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
expect(wrapper.vm.diffFilesLength).toEqual(100);
done(); done();
}); });
}); });
it('calls batch methods if diffsBatchLoad is enabled, and latest version', 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.glFeatures.diffsBatchLoad = true;
wrapper.vm.fetchData(false); wrapper.vm.fetchData(false);
...@@ -268,6 +273,7 @@ describe('diffs/components/app', () => { ...@@ -268,6 +273,7 @@ describe('diffs/components/app', () => {
expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();
expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled();
expect(wrapper.vm.diffFilesLength).toEqual(100);
done(); done();
}); });
}); });
......
...@@ -141,6 +141,13 @@ describe('DiffsStoreActions', () => { ...@@ -141,6 +141,13 @@ describe('DiffsStoreActions', () => {
done(); done();
}, },
); );
fetchDiffFiles({ state: { endpoint }, commit: () => null })
.then(data => {
expect(data).toEqual(res);
done();
})
.catch(done.fail);
}); });
}); });
......
...@@ -263,14 +263,6 @@ describe('Diffs Module Getters', () => { ...@@ -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', () => { describe('currentDiffIndex', () => {
it('returns index of currently selected diff in diffList', () => { it('returns index of currently selected diff in diffList', () => {
localState.diffFiles = [{ file_hash: '111' }, { file_hash: '222' }, { file_hash: '333' }]; localState.diffFiles = [{ file_hash: '111' }, { file_hash: '222' }, { file_hash: '333' }];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment