diff options
author | Tim Zallmann <tzallmann@gitlab.com> | 2018-09-07 17:13:11 +0200 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2018-09-07 17:13:11 +0200 |
commit | d2cbe07398c3f824ffecbd78ff5749daca678d3e (patch) | |
tree | 80db47a1f53e1333b6252d14e314bb6a758f158e | |
parent | d4d5ed59f98eb3218418c385327224d2512e518e (diff) | |
download | gitlab-ce-d2cbe07398c3f824ffecbd78ff5749daca678d3e.tar.gz |
Adapted so utils + actions don't include any mutations and mutations are always against state
-rw-r--r-- | app/assets/javascripts/diffs/store/actions.js | 58 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/mutation_types.js | 4 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/mutations.js | 72 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/mutations.js | 7 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/utils.js | 4 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/actions_spec.js | 34 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/mutations_spec.js | 94 |
7 files changed, 164 insertions, 109 deletions
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 44ae0f2f17b..184a90c6033 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -31,66 +31,18 @@ export const fetchDiffFiles = ({ state, commit }) => { // This is adding line discussions to the actual lines in the diff tree // once for parallel and once for inline mode -export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { +export const assignDiscussionsToDiff = ({ commit }, allLineDiscussions) => { Object.values(allLineDiscussions).forEach(discussions => { if (discussions.length > 0) { const { fileHash } = discussions[0]; - const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash); - if (selectedFile) { - const targetLine = selectedFile.parallelDiffLines.find( - line => - (line.left && line.left.lineCode === discussions[0].line_code) || - (line.right && line.right.lineCode === discussions[0].line_code), - ); - if (targetLine) { - if (targetLine.left && targetLine.left.lineCode === discussions[0].line_code) { - commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.left, discussions }); - } else { - commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.right, discussions }); - } - } - - if (selectedFile.highlightedDiffLines) { - const targetInlineLine = selectedFile.highlightedDiffLines.find( - line => line.lineCode === discussions[0].line_code, - ); - - if (targetInlineLine) { - commit(types.SET_LINE_DISCUSSIONS, { line: targetInlineLine, discussions }); - } - } - } + commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { fileHash, discussions }); } }); }; -export const removeDiscussionsFromDiff = ({ state, commit }, removeDiscussion) => { - const { fileHash } = removeDiscussion; - const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash); - - if (selectedFile) { - const targetLine = selectedFile.parallelDiffLines.find( - line => - (line.left && line.left.lineCode === removeDiscussion.line_code) || - (line.right && line.right.lineCode === removeDiscussion.line_code), - ); - - if (targetLine) { - if (targetLine.left && targetLine.left.lineCode === removeDiscussion.line_code) { - commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.left); - } else { - commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.right); - } - } - - const targetInlineLine = selectedFile.highlightedDiffLines.find( - line => line.lineCode === removeDiscussion.line_code, - ); - - if (targetInlineLine) { - commit(types.REMOVE_LINE_DISCUSSIONS, targetInlineLine); - } - } +export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { + const { fileHash, line_code } = removeDiscussion; + commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); }; export const startRenderDiffsQueue = ({ state, commit }) => { diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 80f2807682a..f61efbe6e1e 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -9,5 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; export const RENDER_FILE = 'RENDER_FILE'; -export const SET_LINE_DISCUSSIONS = 'SET_LINE_DISCUSSIONS'; -export const REMOVE_LINE_DISCUSSIONS = 'REMOVE_LINE_DISCUSSIONS'; +export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE'; +export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 1dd43f7857a..6fce8ed5df3 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -84,15 +84,71 @@ export default { })); }, - [types.SET_LINE_DISCUSSIONS](state, { line, discussions }) { - Object.assign(line, { - discussions, - }); + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + if (selectedFile) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === discussions[0].line_code) || + (line.right && line.right.lineCode === discussions[0].line_code), + ); + if (targetLine) { + if (targetLine.left && targetLine.left.lineCode === discussions[0].line_code) { + Object.assign(targetLine.left, { + discussions, + }); + } else { + Object.assign(targetLine.right, { + discussions, + }); + } + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === discussions[0].line_code, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions, + }); + } + } + } }, - [types.REMOVE_LINE_DISCUSSIONS](state, line) { - Object.assign(line, { - discussions: [], - }); + [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + if (selectedFile) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === lineCode) || + (line.right && line.right.lineCode === lineCode), + ); + if (targetLine) { + if (targetLine.left && targetLine.left.lineCode === lineCode) { + Object.assign(targetLine.left, { + discussions: [], + }); + } else { + Object.assign(targetLine.right, { + discussions: [], + }); + } + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === lineCode, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions: [], + }); + } + } + } }, }; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 8e1da818e3b..2c04bfea122 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -99,6 +99,10 @@ export default { const discussions = []; discussionsData.forEach(discussion => { + if (discussion.diff_file) { + Object.assign(discussion, { fileHash: discussion.diff_file.file_hash }); + } + // To support legacy notes, should be very rare case. if (discussion.individual_note && discussion.notes.length > 1) { discussion.notes.forEach(n => { @@ -186,6 +190,9 @@ export default { const note = noteData; const selectedDiscussion = state.discussions.find(disc => disc.id === note.id); note.expanded = true; // override expand flag to prevent collapse + if (note.diff_file) { + Object.assign(note, { fileHash: note.diff_file.file_hash }); + } Object.assign(selectedDiscussion, { ...note }); }, diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js index 7608790d042..8ccbdb4c130 100644 --- a/app/assets/javascripts/notes/stores/utils.js +++ b/app/assets/javascripts/notes/stores/utils.js @@ -30,10 +30,6 @@ export const reduceDiscussionsToLineCodes = selectedDiscussions => if (note.diff_discussion && note.line_code && note.resolvable) { // For context about line notes: there might be multiple notes with the same line code const items = acc[note.line_code] || []; - if (note.diff_file) { - // Object.assign(note, { fileHash: note.diff_file.file_hash }); - } - items.push(note); Object.assign(acc, { [note.line_code]: items }); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 44c2eb27e0d..c162fc965ba 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -100,6 +100,7 @@ describe('DiffsStoreActions', () => { }, ], }; + const singleDiscussion = { line_code: 'ABC_1_1', diff_discussion: {}, @@ -107,6 +108,7 @@ describe('DiffsStoreActions', () => { file_hash: 'ABC', }, resolvable: true, + fileHash: 'ABC', }; const discussions = reduceDiscussionsToLineCodes([singleDiscussion]); @@ -117,22 +119,9 @@ describe('DiffsStoreActions', () => { state, [ { - type: types.SET_LINE_DISCUSSIONS, + type: types.SET_LINE_DISCUSSIONS_FOR_FILE, payload: { - line: { - lineCode: 'ABC_1_1', - discussions: [], - }, - discussions: [singleDiscussion], - }, - }, - { - type: types.SET_LINE_DISCUSSIONS, - payload: { - line: { - lineCode: 'ABC_1_1', - discussions: [], - }, + fileHash: 'ABC', discussions: [singleDiscussion], }, }, @@ -187,21 +176,10 @@ describe('DiffsStoreActions', () => { state, [ { - type: types.REMOVE_LINE_DISCUSSIONS, - payload: { - lineCode: 'ABC_1_1', - discussions: [ - { - id: 1, - }, - ], - }, - }, - { - type: types.REMOVE_LINE_DISCUSSIONS, + type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, payload: { + fileHash: 'ABC', lineCode: 'ABC_1_1', - discussions: [], }, }, ], diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 4a042b7675f..9a89bc57404 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -149,40 +149,106 @@ describe('DiffsStoreMutations', () => { }); }); - describe('SET_LINE_DISCUSSIONS', () => { + describe('SET_LINE_DISCUSSIONS_FOR_FILE', () => { it('should add discussions to the given line', () => { - const line = { fileHash: 'ABC', discussions: [] }; + const state = { + diffFiles: [ + { + fileHash: 'ABC', + parallelDiffLines: [ + { + left: { + lineCode: 'ABC_1', + discussions: [], + }, + right: { + lineCode: 'ABC_1', + discussions: [], + }, + }, + ], + highlightedDiffLines: [ + { + lineCode: 'ABC_1', + discussions: [], + }, + ], + }, + ], + }; const discussions = [ { id: 1, + line_code: 'ABC_1', }, { id: 2, + line_code: 'ABC_1', }, ]; - mutations[types.SET_LINE_DISCUSSIONS]({}, { line, discussions }); - expect(line.discussions.length).toEqual(2); - expect(line.discussions[1].id).toEqual(2); + mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash: 'ABC', discussions }); + + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2); + + expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2); }); }); describe('REMOVE_LINE_DISCUSSIONS', () => { it('should remove the existing discussions on the given line', () => { - const line = { - fileHash: 'ABC', - discussions: [ - { - id: 1, - }, + const state = { + diffFiles: [ { - id: 2, + fileHash: 'ABC', + parallelDiffLines: [ + { + left: { + lineCode: 'ABC_1', + discussions: [ + { + id: 1, + line_code: 'ABC_1', + }, + { + id: 2, + line_code: 'ABC_1', + }, + ], + }, + right: { + lineCode: 'ABC_1', + discussions: [], + }, + }, + ], + highlightedDiffLines: [ + { + lineCode: 'ABC_1', + discussions: [ + { + id: 1, + line_code: 'ABC_1', + }, + { + id: 2, + line_code: 'ABC_1', + }, + ], + }, + ], }, ], }; - mutations[types.REMOVE_LINE_DISCUSSIONS]({}, line); - expect(line.discussions.length).toEqual(0); + mutations[types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { + fileHash: 'ABC', + lineCode: 'ABC_1', + }); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(0); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(0); }); }); }); |