From 2d113a8c377cd0132655bc8e0dacb8fb8aaca3c8 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 3 Dec 2020 22:39:26 -0700 Subject: [PATCH 1/3] Convert vestigial "SET_DIFF_DATA" to "SET_DIFF_METADATA" --- app/assets/javascripts/diffs/store/actions.js | 4 ++-- app/assets/javascripts/diffs/store/mutation_types.js | 2 +- app/assets/javascripts/diffs/store/mutations.js | 12 +++++------- spec/frontend/diffs/store/actions_spec.js | 2 +- spec/frontend/diffs/store/mutations_spec.js | 6 +++--- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index ee319990290481..8c72971682dd1c 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -185,11 +185,11 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { .get(mergeUrlParams(urlParams, state.endpointMetadata)) .then(({ data }) => { const strippedData = { ...data }; - delete strippedData.diff_files; + commit(types.SET_LOADING, false); commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); - commit(types.SET_DIFF_DATA, strippedData); + commit(types.SET_DIFF_METADATA, strippedData); worker.postMessage(prepareDiffData(data, state.diffFiles)); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 251840287993ee..30097239aaa230 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -3,7 +3,7 @@ export const SET_LOADING = 'SET_LOADING'; export const SET_BATCH_LOADING = 'SET_BATCH_LOADING'; export const SET_RETRIEVING_BATCHES = 'SET_RETRIEVING_BATCHES'; -export const SET_DIFF_DATA = 'SET_DIFF_DATA'; +export const SET_DIFF_METADATA = 'SET_DIFF_METADATA'; export const SET_DIFF_DATA_BATCH = 'SET_DIFF_DATA_BATCH'; export const SET_DIFF_FILES = 'SET_DIFF_FILES'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 69ae3f705e3326..ac2c1c9854b8e3 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -66,17 +66,15 @@ export default { updateDiffFilesInState(state, files); }, - [types.SET_DIFF_DATA](state, data) { - let files = state.diffFiles; + [types.SET_DIFF_METADATA](state, data) { + const fileLess = { ...data }; - if (window.location.search.indexOf('diff_id') !== -1 && data.diff_files) { - files = prepareDiffData(data, files); - } + delete fileLess.diff_files; + delete fileLess.diffFiles; Object.assign(state, { - ...convertObjectPropsToCamelCase(data), + ...convertObjectPropsToCamelCase(fileLess), }); - updateDiffFilesInState(state, files); }, [types.SET_DIFF_DATA_BATCH](state, data) { diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index cd79f9489260f5..fef7676e7950f2 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -249,7 +249,7 @@ describe('DiffsStoreActions', () => { { type: types.SET_LOADING, payload: true }, { type: types.SET_LOADING, payload: false }, { type: types.SET_MERGE_REQUEST_DIFFS, payload: diffMetadata.merge_request_diffs }, - { type: types.SET_DIFF_DATA, payload: noFilesData }, + { type: types.SET_DIFF_METADATA, payload: noFilesData }, ], [], () => { diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index c061c5c25906c5..14335053207b16 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -67,8 +67,8 @@ describe('DiffsStoreMutations', () => { }); }); - describe('SET_DIFF_DATA', () => { - it('should not modify the existing state', () => { + describe('SET_DIFF_METADATA', () => { + it('should not modify the existing diff files in state', () => { const state = { diffFiles: [ { @@ -82,7 +82,7 @@ describe('DiffsStoreMutations', () => { diff_files: [diffFileMockData], }; - mutations[types.SET_DIFF_DATA](state, diffMock); + mutations[types.SET_DIFF_METADATA](state, diffMock); expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY]).toEqual([]); }); -- GitLab From 8a9b0351643f2ea826bcddc6f6c434feafed7f86 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 7 Dec 2020 13:18:53 -0700 Subject: [PATCH 2/3] Remove double checks when setting state --- app/assets/javascripts/diffs/store/mutations.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index ac2c1c9854b8e3..90940d8222637f 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -67,13 +67,8 @@ export default { }, [types.SET_DIFF_METADATA](state, data) { - const fileLess = { ...data }; - - delete fileLess.diff_files; - delete fileLess.diffFiles; - Object.assign(state, { - ...convertObjectPropsToCamelCase(fileLess), + ...convertObjectPropsToCamelCase(data), }); }, -- GitLab From c4ad4e85ba76bd118e662af9f691f1db6cc2293e Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 7 Dec 2020 18:24:13 -0700 Subject: [PATCH 3/3] Update tests for new DIFFS_METADATA mutation behavior --- spec/frontend/diffs/store/mutations_spec.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 14335053207b16..13e7cad835dc0d 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -68,23 +68,23 @@ describe('DiffsStoreMutations', () => { }); describe('SET_DIFF_METADATA', () => { - it('should not modify the existing diff files in state', () => { + it('should overwrite state with the camelCased data that is passed in', () => { const state = { - diffFiles: [ - { - content_sha: diffFileMockData.content_sha, - file_hash: diffFileMockData.file_hash, - [INLINE_DIFF_LINES_KEY]: [], - }, - ], + diffFiles: [], }; const diffMock = { diff_files: [diffFileMockData], }; + const metaMock = { + other_key: 'value', + }; mutations[types.SET_DIFF_METADATA](state, diffMock); + expect(state.diffFiles[0]).toEqual(diffFileMockData); - expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY]).toEqual([]); + mutations[types.SET_DIFF_METADATA](state, metaMock); + expect(state.diffFiles[0]).toEqual(diffFileMockData); + expect(state.otherKey).toEqual('value'); }); }); -- GitLab