diff options
author | Phil Hughes <me@iamphill.com> | 2018-12-04 16:29:05 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-12-05 11:42:41 +0000 |
commit | 6dfede5a6d69f230b049d339af19462573a046e4 (patch) | |
tree | ac4e63af10bc48220059d24a44cce4a9d1b0efaf | |
parent | 1c9b10016a30dc8b8a7aa2a64eb0175973661087 (diff) | |
download | gitlab-ce-6dfede5a6d69f230b049d339af19462573a046e4.tar.gz |
Fixed multiple discussions getting added to diff lines
Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/8195
-rw-r--r-- | app/assets/javascripts/diffs/store/mutations.js | 12 | ||||
-rw-r--r-- | changelogs/unreleased/multiple-diff-line-discussions-fix.yml | 5 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/mutations_spec.js | 78 |
3 files changed, 92 insertions, 3 deletions
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index f0895661bf2..331fb052371 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -130,7 +130,7 @@ export default { if (file.highlighted_diff_lines) { file.highlighted_diff_lines = file.highlighted_diff_lines.map(line => { - if (lineCheck(line)) { + if (!line.discussions.some(({ id }) => discussion.id === id) && lineCheck(line)) { return { ...line, discussions: line.discussions.concat(discussion), @@ -150,11 +150,17 @@ export default { return { left: { ...line.left, - discussions: left ? line.left.discussions.concat(discussion) : [], + discussions: + left && !line.left.discussions.some(({ id }) => id === discussion.id) + ? line.left.discussions.concat(discussion) + : (line.left && line.left.discussions) || [], }, right: { ...line.right, - discussions: right && !left ? line.right.discussions.concat(discussion) : [], + discussions: + right && !left && !line.right.discussions.some(({ id }) => id === discussion.id) + ? line.right.discussions.concat(discussion) + : (line.right && line.right.discussions) || [], }, }; } diff --git a/changelogs/unreleased/multiple-diff-line-discussions-fix.yml b/changelogs/unreleased/multiple-diff-line-discussions-fix.yml new file mode 100644 index 00000000000..870a8ab3815 --- /dev/null +++ b/changelogs/unreleased/multiple-diff-line-discussions-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fixed duplicate discussions getting added to diff lines +merge_request: +author: +type: fixed diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 7a06c178f0b..23e8761bc55 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -199,6 +199,84 @@ describe('DiffsStoreMutations', () => { expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); }); + it('should not duplicate discussions on line', () => { + const diffPosition = { + base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', + head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130', + new_line: null, + new_path: '500-lines-4.txt', + old_line: 5, + old_path: '500-lines-4.txt', + start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', + }; + + const state = { + latestDiff: true, + diffFiles: [ + { + file_hash: 'ABC', + parallel_diff_lines: [ + { + left: { + line_code: 'ABC_1', + discussions: [], + }, + right: { + line_code: 'ABC_1', + discussions: [], + }, + }, + ], + highlighted_diff_lines: [ + { + line_code: 'ABC_1', + discussions: [], + }, + ], + }, + ], + }; + const discussion = { + id: 1, + line_code: 'ABC_1', + diff_discussion: true, + resolvable: true, + original_position: diffPosition, + position: diffPosition, + diff_file: { + file_hash: state.diffFiles[0].file_hash, + }, + }; + + const diffPositionByLineCode = { + ABC_1: diffPosition, + }; + + mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { + discussion, + diffPositionByLineCode, + }); + + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); + + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + + mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { + discussion, + diffPositionByLineCode, + }); + + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); + + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + }); + it('should add legacy discussions to the given line', () => { const diffPosition = { base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', |