Adapted so utils + actions don't include any mutations and mutations are always against state

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 }) => {

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

app/assets/javascripts/diffs/store/mutations.js

@@ -84,15 +84,71 @@ export default {
     }));
   },
 
-  [types.SET_LINE_DISCUSSIONS](state, { line, discussions }) {
-    Object.assign(line, {
+  [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, {
+  [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: [],
           });
+        }
+      }
+    }
   },
 };

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

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 });

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

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 = {
+      const state = {
+        diffFiles: [
+          {
             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);
     });
   });
 });