From 04c0d12d1a6cfaa54d2e5f510922b9d27c5c0a77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <me@andr3.net>
Date: Sat, 8 Sep 2018 06:37:41 +0000
Subject: Resolve "Merge requests show outdated discussions on changes tab"

---
 app/assets/javascripts/diffs/store/actions.js   | 11 +++++++++--
 app/assets/javascripts/diffs/store/mutations.js | 19 ++++++++++++++++---
 app/assets/javascripts/diffs/store/utils.js     | 11 ++++++++++-
 3 files changed, 35 insertions(+), 6 deletions(-)

(limited to 'app/assets/javascripts/diffs')

diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 184a90c6033..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,
@@ -31,11 +32,17 @@ 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 = ({ commit }, allLineDiscussions) => {
+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 });
+      commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, {
+        fileHash,
+        discussions,
+        diffPositionByLineCode,
+      });
     }
   });
 };
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 676c4dab1dd..6dc5bf16c65 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -6,6 +6,7 @@ import {
   removeMatchLine,
   addContextLines,
   prepareDiffData,
+  isDiscussionApplicableToLine,
 } from './utils';
 import * as types from './mutation_types';
 
@@ -84,10 +85,22 @@ export default {
     }));
   },
 
-  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions }) {
+  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
     const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
-    if (selectedFile) {
-      const firstDiscussion = discussions[0];
+    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) ||
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 6b1659a412c..4139a999574 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -217,7 +217,7 @@ export function prepareDiffData(diffData) {
   }
 }
 
-export function getDiffRefsByLineCode(diffFiles) {
+export function getDiffPositionByLineCode(diffFiles) {
   return diffFiles.reduce((acc, diffFile) => {
     const { baseSha, headSha, startSha } = diffFile.diffRefs;
     const { newPath, oldPath } = diffFile;
@@ -237,3 +237,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);
+}
-- 
cgit v1.2.1