5 files changed, 218 insertions, 85 deletions

diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 4ab6ceb249a..027df2ec841 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -3,6 +3,7 @@ import axios from '~/lib/utils/axios_utils';
 import Cookies from 'js-cookie';
 import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils';
 import { mergeUrlParams } from '~/lib/utils/url_utility';
+import { getDiffPositionByLineCode } from './utils';
 import * as types from './mutation_types';
 import {
   PARALLEL_DIFF_VIEW_TYPE,
@@ -29,25 +30,53 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .then(handleLocationHash);
 };
 
-export const startRenderDiffsQueue = ({ state, commit }) => {
-  const checkItem = () => {
-    const nextFile = state.diffFiles.find(
-      file => !file.renderIt && (!file.collapsed || !file.text),
-    );
-    if (nextFile) {
-      requestAnimationFrame(() => {
-        commit(types.RENDER_FILE, nextFile);
+// 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) => {
+  const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles);
+
+  Object.values(allLineDiscussions).forEach(discussions => {
+    if (discussions.length > 0) {
+      const { fileHash } = discussions[0];
+      commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, {
+        fileHash,
+        discussions,
+        diffPositionByLineCode,
       });
-      requestIdleCallback(
-        () => {
-          checkItem();
-        },
-        { timeout: 1000 },
-      );
     }
-  };
+  });
+};
+
+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 }) => {
+  const checkItem = () =>
+    new Promise(resolve => {
+      const nextFile = state.diffFiles.find(
+        file => !file.renderIt && (!file.collapsed || !file.text),
+      );
+
+      if (nextFile) {
+        requestAnimationFrame(() => {
+          commit(types.RENDER_FILE, nextFile);
+        });
+        requestIdleCallback(
+          () => {
+            checkItem()
+              .then(resolve)
+              .catch(() => {});
+          },
+          { timeout: 1000 },
+        );
+      } else {
+        resolve();
+      }
+    });
 
-  checkItem();
+  return checkItem();
 };
 
 export const setInlineDiffViewType = ({ commit }) => {
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 4a47646d7fa..968ba3c5e13 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit
 export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
 export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
   return (
-    (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
+    (discussions &&
+      discussions.length &&
+      discussions.find(discussion => discussion.expanded) !== undefined) ||
     false
   );
 };
@@ -64,45 +72,38 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
       discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
   ) || [];
 
-export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => {
-  if (!lineCode || lineCode === undefined) return [];
-  const discussions = rootGetters.discussionsByLineCode;
-  return discussions[lineCode] || [];
-};
-
-export const shouldRenderParallelCommentRow = (state, getters) => line => {
-  const leftLineCode = line.left.lineCode;
-  const rightLineCode = line.right.lineCode;
-  const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode);
-  const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode);
-  const hasDiscussion = leftDiscussions.length || rightDiscussions.length;
+export const shouldRenderParallelCommentRow = state => line => {
+  const hasDiscussion =
+    (line.left && line.left.discussions && line.left.discussions.length) ||
+    (line.right && line.right.discussions && line.right.discussions.length);
 
-  const hasExpandedDiscussionOnLeft = leftDiscussions.length
-    ? leftDiscussions.every(discussion => discussion.expanded)
-    : false;
-  const hasExpandedDiscussionOnRight = rightDiscussions.length
-    ? rightDiscussions.every(discussion => discussion.expanded)
-    : false;
+  const hasExpandedDiscussionOnLeft =
+    line.left && line.left.discussions && line.left.discussions.length
+      ? line.left.discussions.every(discussion => discussion.expanded)
+      : false;
+  const hasExpandedDiscussionOnRight =
+    line.right && line.right.discussions && line.right.discussions.length
+      ? line.right.discussions.every(discussion => discussion.expanded)
+      : false;
 
   if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
     return true;
   }
 
-  const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
-  const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
+  const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode];
+  const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode];
 
   return hasCommentFormOnLeft || hasCommentFormOnRight;
 };
 
-export const shouldRenderInlineCommentRow = (state, getters) => line => {
+export const shouldRenderInlineCommentRow = state => line => {
   if (state.diffLineCommentForms[line.lineCode]) return true;
 
-  const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
-  if (lineDiscussions.length === 0) {
+  if (!line.discussions || line.discussions.length === 0) {
     return false;
   }
 
-  return lineDiscussions.every(discussion => discussion.expanded);
+  return line.discussions.every(discussion => discussion.expanded);
 };
 
 // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index c999d637d50..f61efbe6e1e 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/app/assets/javascripts/diffs/store/mutation_types.js
@@ -9,3 +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_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 0522e32c410..6dc5bf16c65 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,8 +1,13 @@
 import Vue from 'vue';
-import _ from 'underscore';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
-import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
+import {
+  findDiffFile,
+  addLineReferences,
+  removeMatchLine,
+  addContextLines,
+  prepareDiffData,
+  isDiscussionApplicableToLine,
+} from './utils';
 import * as types from './mutation_types';
 
 export default {
@@ -17,38 +22,7 @@ export default {
 
   [types.SET_DIFF_DATA](state, data) {
     const diffData = convertObjectPropsToCamelCase(data, { deep: true });
-    let showingLines = 0;
-    const filesLength = diffData.diffFiles.length;
-    let i;
-    for (i = 0; i < filesLength; i += 1) {
-      const file = diffData.diffFiles[i];
-      if (file.parallelDiffLines) {
-        const linesLength = file.parallelDiffLines.length;
-        let u = 0;
-        for (u = 0; u < linesLength; u += 1) {
-          const line = file.parallelDiffLines[u];
-          if (line.left) delete line.left.text;
-          if (line.right) delete line.right.text;
-        }
-      }
-
-      if (file.highlightedDiffLines) {
-        const linesLength = file.highlightedDiffLines.length;
-        let u;
-        for (u = 0; u < linesLength; u += 1) {
-          const line = file.highlightedDiffLines[u];
-          delete line.text;
-        }
-      }
-
-      if (file.highlightedDiffLines) {
-        showingLines += file.parallelDiffLines.length;
-      }
-      Object.assign(file, {
-        renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
-        collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
-      });
-    }
+    prepareDiffData(diffData);
 
     Object.assign(state, {
       ...diffData,
@@ -98,19 +72,93 @@ export default {
 
   [types.ADD_COLLAPSED_DIFFS](state, { file, data }) {
     const normalizedData = convertObjectPropsToCamelCase(data, { deep: true });
+    prepareDiffData(normalizedData);
     const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash);
-
-    if (newFileData) {
-      const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash);
-      state.diffFiles.splice(index, 1, newFileData);
-    }
+    const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash);
+    Object.assign(selectedFile, { ...newFileData });
   },
 
   [types.EXPAND_ALL_FILES](state) {
-    // eslint-disable-next-line no-param-reassign
     state.diffFiles = state.diffFiles.map(file => ({
       ...file,
       collapsed: false,
     }));
   },
+
+  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
+    const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
+    const firstDiscussion = discussions[0];
+    const isDiffDiscussion = firstDiscussion.diff_discussion;
+    const hasLineCode = firstDiscussion.line_code;
+    const isResolvable = firstDiscussion.resolvable;
+    const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
+
+    if (
+      selectedFile &&
+      isDiffDiscussion &&
+      hasLineCode &&
+      isResolvable &&
+      diffPosition &&
+      isDiscussionApplicableToLine(firstDiscussion, diffPosition)
+    ) {
+      const targetLine = selectedFile.parallelDiffLines.find(
+        line =>
+          (line.left && line.left.lineCode === firstDiscussion.line_code) ||
+          (line.right && line.right.lineCode === firstDiscussion.line_code),
+      );
+      if (targetLine) {
+        if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) {
+          Object.assign(targetLine.left, {
+            discussions,
+          });
+        } else {
+          Object.assign(targetLine.right, {
+            discussions,
+          });
+        }
+      }
+
+      if (selectedFile.highlightedDiffLines) {
+        const targetInlineLine = selectedFile.highlightedDiffLines.find(
+          line => line.lineCode === firstDiscussion.line_code,
+        );
+
+        if (targetInlineLine) {
+          Object.assign(targetInlineLine, {
+            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) {
+        const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
+
+        Object.assign(targetLine[side], {
+          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/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 82082ac508a..b7e52a8f37f 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -8,6 +8,8 @@ import {
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
   MATCH_LINE_TYPE,
+  LINES_TO_BE_RENDERED_DIRECTLY,
+  MAX_LINES_TO_BE_RENDERED,
 } from '../constants';
 
 export function findDiffFile(files, hash) {
@@ -161,6 +163,11 @@ export function addContextLines(options) {
  * @returns {Object}
  */
 export function trimFirstCharOfLineContent(line = {}) {
+  // eslint-disable-next-line no-param-reassign
+  delete line.text;
+  // eslint-disable-next-line no-param-reassign
+  line.discussions = [];
+
   const parsedLine = Object.assign({}, line);
 
   if (line.richText) {
@@ -174,7 +181,44 @@ export function trimFirstCharOfLineContent(line = {}) {
   return parsedLine;
 }
 
-export function getDiffRefsByLineCode(diffFiles) {
+// This prepares and optimizes the incoming diff data from the server
+// by setting up incremental rendering and removing unneeded data
+export function prepareDiffData(diffData) {
+  const filesLength = diffData.diffFiles.length;
+  let showingLines = 0;
+  for (let i = 0; i < filesLength; i += 1) {
+    const file = diffData.diffFiles[i];
+
+    if (file.parallelDiffLines) {
+      const linesLength = file.parallelDiffLines.length;
+      for (let u = 0; u < linesLength; u += 1) {
+        const line = file.parallelDiffLines[u];
+        if (line.left) {
+          line.left = trimFirstCharOfLineContent(line.left);
+        }
+        if (line.right) {
+          line.right = trimFirstCharOfLineContent(line.right);
+        }
+      }
+    }
+
+    if (file.highlightedDiffLines) {
+      const linesLength = file.highlightedDiffLines.length;
+      for (let u = 0; u < linesLength; u += 1) {
+        const line = file.highlightedDiffLines[u];
+        Object.assign(line, { ...trimFirstCharOfLineContent(line) });
+      }
+      showingLines += file.parallelDiffLines.length;
+    }
+
+    Object.assign(file, {
+      renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
+      collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+    });
+  }
+}
+
+export function getDiffPositionByLineCode(diffFiles) {
   return diffFiles.reduce((acc, diffFile) => {
     const { baseSha, headSha, startSha } = diffFile.diffRefs;
     const { newPath, oldPath } = diffFile;
@@ -194,3 +238,12 @@ export function getDiffRefsByLineCode(diffFiles) {
     return acc;
   }, {});
 }
+
+// This method will check whether the discussion is still applicable
+// to the diff line in question regarding different versions of the MR
+export function isDiscussionApplicableToLine(discussion, diffPosition) {
+  const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter);
+  const refs = convertObjectPropsToCamelCase(discussion.position.formatter);
+
+  return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition);
+}