Merge branch 'multiple-diff-line-discussions-fix' into 'master'

Fixed multiple discussions getting added to diff lines

Closes gitlab-ee#8195

See merge request gitlab-org/gitlab-ce!23541

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) || [],
                 },
               };
             }

changelogs/unreleased/multiple-diff-line-discussions-fix.yml

+---
+title: Fixed duplicate discussions getting added to diff lines
+merge_request:
+author:
+type: fixed

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',