Merge branch 'mr-legacy-diff-notes' into 'master'

Re-enable legacy diff notes on merge request diffs

Closes #48873

See merge request gitlab-org/gitlab-ce!21652

6 files changed, 36 insertions, 14 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
index a0dc381ccc7..0fa14615532 100644
--- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
@@ -22,7 +22,7 @@ export default {
       type: Object,
       required: true,
     },
-    position: {
+    linePosition: {
       type: String,
       required: false,
       default: '',
@@ -81,7 +81,7 @@ export default {
         noteTargetLine: this.noteTargetLine,
         diffViewType: this.diffViewType,
         diffFile: selectedDiffFile,
-        linePosition: this.position,
+        linePosition: this.linePosition,
       });
 
       this.saveNote(postData)
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
index 26417c350cb..3339c56cbb6 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
@@ -92,7 +92,7 @@ export default {
         :diff-file-hash="diffFileHash"
         :line="line.left"
         :note-target-line="line.left"
-        position="left"
+        line-position="left"
       />
     </td>
     <td class="notes_line new"></td>
@@ -111,7 +111,7 @@ export default {
         :diff-file-hash="diffFileHash"
         :line="line.right"
         :note-target-line="line.right"
-        position="right"
+        line-position="right"
       />
     </td>
   </tr>
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index f68afa44837..2795dddfc48 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -7,6 +7,7 @@ export const CONTEXT_LINE_TYPE = 'context';
 export const EMPTY_CELL_TYPE = 'empty-cell';
 export const COMMENT_FORM_TYPE = 'commentForm';
 export const DIFF_NOTE_TYPE = 'DiffNote';
+export const LEGACY_DIFF_NOTE_TYPE = 'LegacyDiffNote';
 export const NOTE_TYPE = 'Note';
 export const NEW_LINE_TYPE = 'new';
 export const OLD_LINE_TYPE = 'old';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 6dc5bf16c65..59a2c09e54f 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -90,16 +90,18 @@ export default {
     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)
+      isDiscussionApplicableToLine({
+        discussion: firstDiscussion,
+        diffPosition,
+        latestDiff: state.latestDiff,
+      })
     ) {
       const targetLine = selectedFile.parallelDiffLines.find(
         line =>
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index b7e52a8f37f..834a94ea42a 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -4,6 +4,7 @@ import {
   LINE_POSITION_LEFT,
   LINE_POSITION_RIGHT,
   TEXT_DIFF_POSITION_TYPE,
+  LEGACY_DIFF_NOTE_TYPE,
   DIFF_NOTE_TYPE,
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
@@ -60,7 +61,10 @@ export function getNoteFormData(params) {
       noteable_type: noteableType,
       noteable_id: noteableData.id,
       commit_id: '',
-      type: DIFF_NOTE_TYPE,
+      type:
+        diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
+          ? DIFF_NOTE_TYPE
+          : LEGACY_DIFF_NOTE_TYPE,
       line_code: noteTargetLine.lineCode,
     },
   };
@@ -230,7 +234,16 @@ export function getDiffPositionByLineCode(diffFiles) {
         const { lineCode, oldLine, newLine } = line;
 
         if (lineCode) {
-          acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine };
+          acc[lineCode] = {
+            baseSha,
+            headSha,
+            startSha,
+            newPath,
+            oldPath,
+            oldLine,
+            newLine,
+            lineCode,
+          };
         }
       });
     }
@@ -241,9 +254,15 @@ export function getDiffPositionByLineCode(diffFiles) {
 
 // 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);
+export function isDiscussionApplicableToLine({ discussion, diffPosition, latestDiff }) {
+  const { lineCode, ...diffPositionCopy } = diffPosition;
 
-  return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition);
+  if (discussion.original_position && discussion.position) {
+    const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter);
+    const refs = convertObjectPropsToCamelCase(discussion.position.formatter);
+
+    return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy);
+  }
+
+  return latestDiff && 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 8ccbdb4c130..0e41ff03d67 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -27,7 +27,7 @@ export const getQuickActionText = note => {
 
 export const reduceDiscussionsToLineCodes = selectedDiscussions =>
   selectedDiscussions.reduce((acc, note) => {
-    if (note.diff_discussion && note.line_code && note.resolvable) {
+    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);