Updated LegacyDiffNote logic

5 files changed, 30 insertions, 11 deletions

diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 72da64bd4db..a11ac2b292b 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,6 +1,5 @@
 import Vue from 'vue';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { isLegacyDiffNote } from '~/notes/stores/utils';
 import {
   findDiffFile,
   addLineReferences,
@@ -87,18 +86,18 @@ export default {
   },
 
   [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
+    if (!state.latestDiff) return;
+
     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 || isLegacyDiffNote(firstDiscussion);
     const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
 
     if (
       selectedFile &&
       isDiffDiscussion &&
       hasLineCode &&
-      isResolvable &&
       diffPosition &&
       isDiscussionApplicableToLine(firstDiscussion, diffPosition)
     ) {
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index d521e4584ad..36053d8db44 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -61,7 +61,10 @@ export function getNoteFormData(params) {
       noteable_type: noteableType,
       noteable_id: noteableData.id,
       commit_id: '',
-      type: diffFile.diffRefs.startSha ? DIFF_NOTE_TYPE : LEGACY_DIFF_NOTE_TYPE,
+      type:
+        diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
+          ? DIFF_NOTE_TYPE
+          : LEGACY_DIFF_NOTE_TYPE,
       line_code: noteTargetLine.lineCode,
     },
   };
@@ -261,5 +264,5 @@ export function isDiscussionApplicableToLine(discussion, diffPosition) {
     return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy);
   }
 
-  return lineCode === discussion.line_code;
+  return discussion.active && lineCode === discussion.line_code;
 }
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index eea6eb2b1af..0e41ff03d67 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -2,7 +2,6 @@ import AjaxCache from '~/lib/utils/ajax_cache';
 
 const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
 
-export const isLegacyDiffNote = note => !note.resolvable && !note.position;
 export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0];
 
 export const getQuickActionText = note => {
@@ -28,7 +27,7 @@ export const getQuickActionText = note => {
 
 export const reduceDiscussionsToLineCodes = selectedDiscussions =>
   selectedDiscussions.reduce((acc, note) => {
-    if (note.diff_discussion && note.line_code && (note.resolvable || isLegacyDiffNote(note))) {
+    if (note.diff_discussion && note.line_code) {
       // For context about line notes: there might be multiple notes with the same line code
       const items = acc[note.line_code] || [];
       items.push(note);
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 8e94b21f737..9a5d8dfbd15 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => {
       };
 
       const state = {
+        latestDiff: true,
         diffFiles: [
           {
             fileHash: 'ABC',
@@ -243,6 +244,7 @@ describe('DiffsStoreMutations', () => {
       };
 
       const state = {
+        latestDiff: true,
         diffFiles: [
           {
             fileHash: 'ABC',
@@ -272,11 +274,13 @@ describe('DiffsStoreMutations', () => {
           id: 1,
           line_code: 'ABC_1',
           diff_discussion: true,
+          active: true,
         },
         {
           id: 2,
           line_code: 'ABC_1',
           diff_discussion: true,
+          active: true,
         },
       ];
 
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 4b5955dd4b5..fd740c5e798 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -156,6 +156,7 @@ describe('DiffsStoreUtils', () => {
     it('should create legacy note form data', () => {
       const diffFile = getDiffFileMock();
       delete diffFile.diffRefs.startSha;
+      delete diffFile.diffRefs.headSha;
 
       noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE;
 
@@ -177,7 +178,7 @@ describe('DiffsStoreUtils', () => {
       const position = JSON.stringify({
         base_sha: diffFile.diffRefs.baseSha,
         start_sha: undefined,
-        head_sha: diffFile.diffRefs.headSha,
+        head_sha: undefined,
         old_path: diffFile.oldPath,
         new_path: diffFile.newPath,
         position_type: TEXT_DIFF_POSITION_TYPE,
@@ -188,7 +189,7 @@ describe('DiffsStoreUtils', () => {
       const postData = {
         view: options.diffViewType,
         line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE,
-        merge_request_diff_head_sha: diffFile.diffRefs.headSha,
+        merge_request_diff_head_sha: undefined,
         in_reply_to_discussion_id: '',
         note_project_id: '',
         target_type: options.noteableType,
@@ -359,8 +360,21 @@ describe('DiffsStoreUtils', () => {
       ).toBe(false);
     });
 
-    it('returns true when line codes match and discussion does not contain position', () => {
-      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1' };
+    it('returns true when line codes match and discussion does not contain position and is not active', () => {
+      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: false };
+      delete discussion.original_position;
+      delete discussion.position;
+
+      expect(
+        utils.isDiscussionApplicableToLine(discussion, {
+          ...diffPosition,
+          lineCode: 'ABC_1',
+        }),
+      ).toBe(false);
+    });
+
+    it('returns true when line codes match and discussion does not contain position and is active', () => {
+      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true };
       delete discussion.original_position;
       delete discussion.position;