From 0fba7cca36ea352b3cd8c0e8a34a61453d2c7e07 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 25 Oct 2018 12:04:34 +0100
Subject: [PATCH] Update the state, not a param

Also fixed a bug where discussions wouldn't be assigned to a line
when switching from discussion tab to changes tab
---
 .../javascripts/diffs/components/app.vue      |  4 +-
 app/assets/javascripts/diffs/store/actions.js | 22 +-----
 .../javascripts/diffs/store/mutations.js      | 79 +++++++++++--------
 3 files changed, 53 insertions(+), 52 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 9ddca8548e0..a8d615dd8f0 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -63,7 +63,7 @@ export default {
       plainDiffPath: state => state.diffs.plainDiffPath,
       emailPatchPath: state => state.diffs.emailPatchPath,
     }),
-    ...mapState('diffs', ['showTreeList']),
+    ...mapState('diffs', ['showTreeList', 'isLoading']),
     ...mapGetters('diffs', ['isParallelView']),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
     targetBranch() {
@@ -152,7 +152,7 @@ export default {
       }
     },
     setDiscussions() {
-      if (this.isNotesFetched && !this.assignedDiscussions) {
+      if (this.isNotesFetched && !this.assignedDiscussions && !this.isLoading) {
         requestIdleCallback(
           () =>
             this.assignDiscussionsToDiff().then(() => {
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index e79351454ab..ca8ae605cb4 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -37,29 +37,13 @@ export const fetchDiffFiles = ({ state, commit }) => {
 // once for parallel and once for inline mode
 export const assignDiscussionsToDiff = (
   { commit, state, rootState },
-  discussionsToAssign = rootState.notes.discussions,
+  discussions = rootState.notes.discussions,
 ) => {
   const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles);
-  const discussionsByDiffFile = discussionsToAssign
-    .filter(discussion => discussion.diff_discussion)
-    .reduce((acc, discussion) => {
-      const discussions = (
-        acc[discussion.diff_file.file_hash] || { discussions: [] }
-      ).discussions.concat(discussion);
 
-      return {
-        ...acc,
-        [discussion.diff_file.file_hash]: {
-          diffFile: state.diffFiles.find(file => file.fileHash === discussion.diff_file.file_hash),
-          discussions,
-        },
-      };
-    }, {});
-
-  Object.values(discussionsByDiffFile).forEach(({ discussions, diffFile }) => {
+  discussions.filter(discussion => discussion.diff_discussion).forEach(discussion => {
     commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, {
-      diffFile,
-      discussions,
+      discussion,
       diffPositionByLineCode,
     });
   });
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 6a57a075579..5a8aebd2086 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -90,49 +90,66 @@ export default {
     }));
   },
 
-  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { diffFile, discussions, diffPositionByLineCode }) {
+  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode }) {
     const { latestDiff } = state;
 
-    discussions.forEach(discussion => {
-      const discussionLineCode = discussion.line_code;
-      const lineCheck = ({ lineCode }) =>
-        lineCode === discussionLineCode &&
-        isDiscussionApplicableToLine({
-          discussion,
-          diffPosition: diffPositionByLineCode[lineCode],
-          latestDiff,
-        });
+    const discussionLineCode = discussion.line_code;
+    const fileHash = discussion.diff_file.file_hash;
+    const lineCheck = ({ lineCode }) =>
+      lineCode === discussionLineCode &&
+      isDiscussionApplicableToLine({
+        discussion,
+        diffPosition: diffPositionByLineCode[lineCode],
+        latestDiff,
+      });
 
-      if (diffFile.highlightedDiffLines) {
-        const line = diffFile.highlightedDiffLines.find(lineCheck);
+    state.diffFiles = state.diffFiles.map(diffFile => {
+      if (diffFile.fileHash === fileHash) {
+        const file = { ...diffFile };
 
-        if (line) {
-          Object.assign(line, {
-            discussions: line.discussions.concat(discussion),
+        if (file.highlightedDiffLines) {
+          file.highlightedDiffLines = file.highlightedDiffLines.map(line => {
+            if (lineCheck(line)) {
+              return {
+                ...line,
+                discussions: line.discussions.concat(discussion),
+              };
+            }
+
+            return line;
           });
         }
-      }
 
-      if (diffFile.parallelDiffLines) {
-        const { left, right } = diffFile.parallelDiffLines.find(
-          parallelLine =>
-            (parallelLine.left && lineCheck(parallelLine.left)) ||
-            (parallelLine.right && lineCheck(parallelLine.right)),
-        );
-        const line = left && left.lineCode === discussionLineCode ? left : right;
+        if (file.parallelDiffLines) {
+          file.parallelDiffLines = file.parallelDiffLines.map(line => {
+            const left = line.left && lineCheck(line.left);
+            const right = line.right && lineCheck(line.right);
 
-        if (line) {
-          Object.assign(line, {
-            discussions: line.discussions.concat(discussion),
+            if (left || right) {
+              return {
+                left: {
+                  ...line.left,
+                  discussions: left ? line.left.discussions.concat(discussion) : [],
+                },
+                right: {
+                  ...line.right,
+                  discussions: right ? line.right.discussions.concat(discussion) : [],
+                },
+              };
+            }
+
+            return line;
           });
         }
+
+        if (!file.parallelDiffLines || !file.highlightedDiffLines) {
+          file.discussions = file.discussions.concat(discussion);
+        }
+
+        return file;
       }
 
-      if (!diffFile.parallelDiffLines || !diffFile.highlightedDiffLines) {
-        Object.assign(diffFile, {
-          discussions: diffFile.discussions.concat(discussion),
-        });
-      }
+      return diffFile;
     });
   },