Merge branch 'id-fe-display-discussions-on-merge-ref' into 'master'

Display notes based on line_codes and positions props

See merge request gitlab-org/gitlab!29512

app/assets/javascripts/diffs/components/diff_table_cell.vue

@@ -99,8 +99,12 @@ export default {
       return this.showCommentButton && this.hasDiscussions;
     },
     shouldRenderCommentButton() {
-      const isDiffHead = parseBoolean(getParameterByName('diff_head'));
-      return !isDiffHead && this.isLoggedIn && this.showCommentButton;
+      if (this.isLoggedIn && this.showCommentButton) {
+        const isDiffHead = parseBoolean(getParameterByName('diff_head'));
+        return !isDiffHead || gon.features?.mergeRefHeadComments;
+      }
+
+      return false;
     },
     isMatchLine() {
       return this.line.type === MATCH_LINE_TYPE;

app/assets/javascripts/diffs/store/mutations.js

@@ -182,15 +182,18 @@ export default {
   [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) {
     const { latestDiff } = state;
 
-    const discussionLineCode = discussion.line_code;
+    const discussionLineCodes = [discussion.line_code, ...(discussion.line_codes || [])];
     const fileHash = discussion.diff_file.file_hash;
     const lineCheck = line =>
-      line.line_code === discussionLineCode &&
-      isDiscussionApplicableToLine({
-        discussion,
-        diffPosition: diffPositionByLineCode[line.line_code],
-        latestDiff,
-      });
+      discussionLineCodes.some(
+        discussionLineCode =>
+          line.line_code === discussionLineCode &&
+          isDiscussionApplicableToLine({
+            discussion,
+            diffPosition: diffPositionByLineCode[line.line_code],
+            latestDiff,
+          }),
+      );
     const mapDiscussions = (line, extraCheck = () => true) => ({
       ...line,
       discussions: extraCheck()

app/assets/javascripts/diffs/store/utils.js

@@ -440,10 +440,13 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD
   const { line_code, ...diffPositionCopy } = diffPosition;
 
   if (discussion.original_position && discussion.position) {
-    const originalRefs = discussion.original_position;
-    const refs = discussion.position;
+    const discussionPositions = [
+      discussion.original_position,
+      discussion.position,
+      ...(discussion.positions || []),
+    ];
 
-    return isEqual(refs, diffPositionCopy) || isEqual(originalRefs, diffPositionCopy);
+    return discussionPositions.some(position => isEqual(position, diffPositionCopy));
   }
 
   // eslint-disable-next-line

app/controllers/projects/merge_requests_controller.rb

@@ -25,6 +25,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline)
     push_frontend_feature_flag(:code_navigation, @project)
     push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
+    push_frontend_feature_flag(:merge_ref_head_comments, @project)
   end
 
   before_action do

spec/frontend/diffs/components/diff_table_cell_spec.js

@@ -85,15 +85,18 @@ describe('DiffTableCell', () => {
 
   describe('comment button', () => {
     it.each`
-      showCommentButton | userData     | query                | expectation
-      ${true}           | ${TEST_USER} | ${'diff_head=false'} | ${true}
-      ${true}           | ${TEST_USER} | ${'diff_head=true'}  | ${false}
-      ${false}          | ${TEST_USER} | ${'bogus'}           | ${false}
-      ${true}           | ${null}      | ${''}                | ${false}
+      showCommentButton | userData     | query                | mergeRefHeadComments | expectation
+      ${true}           | ${TEST_USER} | ${'diff_head=false'} | ${false}             | ${true}
+      ${true}           | ${TEST_USER} | ${'diff_head=true'}  | ${true}              | ${true}
+      ${true}           | ${TEST_USER} | ${'diff_head=true'}  | ${false}             | ${false}
+      ${false}          | ${TEST_USER} | ${'diff_head=true'}  | ${true}              | ${false}
+      ${false}          | ${TEST_USER} | ${'bogus'}           | ${true}              | ${false}
+      ${true}           | ${null}      | ${''}                | ${true}              | ${false}
     `(
       'exists is $expectation - with showCommentButton ($showCommentButton) userData ($userData) query ($query)',
-      ({ showCommentButton, userData, query, expectation }) => {
+      ({ showCommentButton, userData, query, mergeRefHeadComments, expectation }) => {
         store.state.notes.userData = userData;
+        gon.features = { mergeRefHeadComments };
         setWindowLocation({ href: `${TEST_HOST}?${query}` });
         createComponent({ showCommentButton });
 

spec/frontend/diffs/store/mutations_spec.js

@@ -615,6 +615,73 @@ describe('DiffsStoreMutations', () => {
       expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1);
       expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1);
     });
+
+    it('should add discussions by line_codes and positions attributes', () => {
+      const diffPosition = {
+        base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+        new_line: null,
+        new_path: '500-lines-4.txt',
+        old_line: 5,
+        old_path: '500-lines-4.txt',
+        start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+      };
+
+      const state = {
+        latestDiff: true,
+        diffFiles: [
+          {
+            file_hash: 'ABC',
+            parallel_diff_lines: [
+              {
+                left: {
+                  line_code: 'ABC_1',
+                  discussions: [],
+                },
+                right: {
+                  line_code: 'ABC_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlighted_diff_lines: [
+              {
+                line_code: 'ABC_1',
+                discussions: [],
+              },
+            ],
+          },
+        ],
+      };
+      const discussion = {
+        id: 1,
+        line_code: 'ABC_2',
+        line_codes: ['ABC_1'],
+        diff_discussion: true,
+        resolvable: true,
+        original_position: {},
+        position: {},
+        positions: [diffPosition],
+        diff_file: {
+          file_hash: state.diffFiles[0].file_hash,
+        },
+      };
+
+      const diffPositionByLineCode = {
+        ABC_1: diffPosition,
+      };
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        discussion,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions).toHaveLength(1);
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toBe(1);
+
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions).toHaveLength(1);
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toBe(1);
+    });
   });
 
   describe('REMOVE_LINE_DISCUSSIONS', () => {