From 7f64a65b7964570c0d3c0261f22cfd4fa4bf9533 Mon Sep 17 00:00:00 2001
From: jboyson1 <jboyson@gitlab.com>
Date: Fri, 24 Apr 2020 13:24:29 -0500
Subject: [PATCH] Omit line_range from diff position comparison

---
 app/assets/javascripts/diffs/store/utils.js | 15 +++++++++++++--
 spec/frontend/diffs/store/utils_spec.js     | 12 ++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index dd8dec49a37823..b46b8d95d5f359 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -437,7 +437,11 @@ export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) {
 // 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, latestDiff }) {
-  const { line_code, ...diffPositionCopy } = diffPosition;
+  const { line_code, ...dp } = diffPosition;
+  // Removing `line_range` from diffPosition because the backend does not
+  // yet consistently return this property. This check can be removed,
+  // once this is addressed. see https://gitlab.com/gitlab-org/gitlab/-/issues/213010
+  const { line_range: dpNotUsed, ...diffPositionCopy } = dp;
 
   if (discussion.original_position && discussion.position) {
     const discussionPositions = [
@@ -446,7 +450,14 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD
       ...(discussion.positions || []),
     ];
 
-    return discussionPositions.some(position => isEqual(position, diffPositionCopy));
+    const removeLineRange = position => {
+      const { line_range: pNotUsed, ...positionNoLineRange } = position;
+      return positionNoLineRange;
+    };
+
+    return discussionPositions
+      .map(removeLineRange)
+      .some(position => isEqual(position, diffPositionCopy));
   }
 
   // eslint-disable-next-line
diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js
index 1adcdab272a538..422332bab28e9e 100644
--- a/spec/frontend/diffs/store/utils_spec.js
+++ b/spec/frontend/diffs/store/utils_spec.js
@@ -503,11 +503,16 @@ describe('DiffsStoreUtils', () => {
       },
     };
 
+    // When multi line comments are fully implemented `line_code` will be
+    // included in all requests. Until then we need to ensure the logic does
+    // not change when it is included only in the "comparison" argument.
+    const lineRange = { start_line_code: 'abc_1_1', end_line_code: 'abc_1_2' };
+
     it('returns true when the discussion is up to date', () => {
       expect(
         utils.isDiscussionApplicableToLine({
           discussion: discussions.upToDateDiscussion1,
-          diffPosition,
+          diffPosition: { ...diffPosition, line_range: lineRange },
           latestDiff: true,
         }),
       ).toBe(true);
@@ -517,7 +522,7 @@ describe('DiffsStoreUtils', () => {
       expect(
         utils.isDiscussionApplicableToLine({
           discussion: discussions.outDatedDiscussion1,
-          diffPosition,
+          diffPosition: { ...diffPosition, line_range: lineRange },
           latestDiff: true,
         }),
       ).toBe(false);
@@ -534,6 +539,7 @@ describe('DiffsStoreUtils', () => {
           diffPosition: {
             ...diffPosition,
             lineCode: 'ABC_1',
+            line_range: lineRange,
           },
           latestDiff: true,
         }),
@@ -551,6 +557,7 @@ describe('DiffsStoreUtils', () => {
           diffPosition: {
             ...diffPosition,
             line_code: 'ABC_1',
+            line_range: lineRange,
           },
           latestDiff: true,
         }),
@@ -568,6 +575,7 @@ describe('DiffsStoreUtils', () => {
           diffPosition: {
             ...diffPosition,
             lineCode: 'ABC_1',
+            line_range: lineRange,
           },
           latestDiff: false,
         }),
-- 
GitLab