Merge branch 'tor/defect/correct-suggestion-line' into 'master'

Correct suggestions for multiple comment boxes

See merge request gitlab-org/gitlab!72599

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

@@ -29,6 +29,11 @@ export default {
       required: false,
       default: false,
     },
+    lineRange: {
+      type: Object,
+      required: false,
+      default: null,
+    },
     linePosition: {
       type: String,
       required: false,
@@ -59,6 +64,7 @@ export default {
         <diff-line-note-form
           :diff-file-hash="diffFileHash"
           :line="line"
+          :range="lineRange"
           :note-target-line="line"
           :help-page-path="helpPagePath"
           :line-position="linePosition"

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

@@ -32,6 +32,11 @@ export default {
       type: Object,
       required: true,
     },
+    range: {
+      type: Object,
+      required: false,
+      default: null,
+    },
     linePosition: {
       type: String,
       required: false,
@@ -49,6 +54,7 @@ export default {
   },
   data() {
     return {
+      lines: null,
       commentLineStart: {
         line_code: this.line.line_code,
         type: this.line.type,
@@ -116,10 +122,8 @@ export default {
       return commentLineOptions(lines, this.line, this.line.line_code, side);
     },
     commentLines() {
-      if (!this.selectedCommentPosition) return [];
-
       const lines = [];
-      const { start, end } = this.selectedCommentPosition;
+      const { start, end } = this.lines;
       const diffLines = this.diffFile[INLINE_DIFF_LINES_KEY];
       let isAdding = false;
 
@@ -144,6 +148,9 @@ export default {
       return lines;
     },
   },
+  created() {
+    this.lines = { ...this.range };
+  },
   mounted() {
     if (this.isLoggedIn) {
       const keys = [
@@ -189,6 +196,9 @@ export default {
         this.handleCancelCommentForm(),
       );
     },
+    updateStartLine(line) {
+      this.lines.start = line;
+    },
   },
 };
 </script>
@@ -199,7 +209,9 @@ export default {
       <multiline-comment-form
         v-model="commentLineStart"
         :line="line"
+        :line-range="lines"
         :comment-line-options="commentLineOptions"
+        @input="updateStartLine"
       />
     </div>
     <note-form

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

@@ -6,6 +6,7 @@ import draftCommentsMixin from '~/diffs/mixins/draft_comments';
 import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
 import { hide } from '~/tooltips';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import { pickDirection } from '../utils/diff_line';
 import DiffCommentCell from './diff_comment_cell.vue';
 import DiffExpansionCell from './diff_expansion_cell.vue';
 import DiffRow from './diff_row.vue';
@@ -106,6 +107,16 @@ export default {
       });
       this.idState.dragStart = null;
     },
+    singleLineComment(code, line) {
+      const lineDir = pickDirection({ line, code });
+
+      this.idState.updatedLineRange = {
+        start: lineDir,
+        end: lineDir,
+      };
+
+      this.showCommentForm({ lineCode: lineDir.line_code, fileHash: this.diffFile.file_hash });
+    },
     isHighlighted(line) {
       return isHighlighted(
         this.highlightedRow,
@@ -169,7 +180,7 @@ export default {
         :index="index"
         :is-highlighted="isHighlighted(line)"
         :file-line-coverage="fileLineCoverage"
-        @showCommentForm="(lineCode) => showCommentForm({ lineCode, fileHash: diffFile.file_hash })"
+        @showCommentForm="(code) => singleLineComment(code, line)"
         @setHighlightedRow="setHighlightedRow"
         @toggleLineDiscussions="
           ({ lineCode, expanded }) =>
@@ -193,6 +204,7 @@ export default {
           <diff-comment-cell
             v-if="line.left && (line.left.renderDiscussion || line.left.hasCommentForm)"
             :line="line.left"
+            :line-range="idState.updatedLineRange"
             :diff-file-hash="diffFile.file_hash"
             :help-page-path="helpPagePath"
             line-position="left"
@@ -206,6 +218,7 @@ export default {
           <diff-comment-cell
             v-if="line.right && (line.right.renderDiscussion || line.right.hasCommentForm)"
             :line="line.right"
+            :line-range="idState.updatedLineRange"
             :diff-file-hash="diffFile.file_hash"
             :line-index="index"
             :help-page-path="helpPagePath"

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

+export function pickDirection({ line, code } = {}) {
+  const { left, right } = line;
+  let direction = left || right;
+
+  if (right?.line_code === code) {
+    direction = right;
+  }
+
+  return direction;
+}

app/assets/javascripts/notes/components/multiline_comment_form.vue

 <script>
 import { GlFormSelect, GlSprintf } from '@gitlab/ui';
-import { mapActions, mapState } from 'vuex';
+import { mapActions } from 'vuex';
 import { getSymbol, getLineClasses } from './multiline_comment_utils';
 
 export default {
@@ -27,13 +27,12 @@ export default {
     };
   },
   computed: {
-    ...mapState({ selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition }),
     lineNumber() {
       return this.commentLineOptions[this.commentLineOptions.length - 1].text;
     },
   },
   created() {
-    const line = this.selectedCommentPosition?.start || this.lineRange?.start || this.line;
+    const line = this.lineRange?.start || this.line;
 
     this.commentLineStart = {
       line_code: line.line_code,
@@ -42,7 +41,6 @@ export default {
       new_line: line.new_line,
     };
 
-    if (this.selectedCommentPosition) return;
     this.highlightSelection();
   },
   destroyed() {

app/assets/javascripts/notes/components/note_form.vue

@@ -334,13 +334,13 @@ export default {
           :markdown-docs-path="markdownDocsPath"
           :quick-actions-docs-path="quickActionsDocsPath"
           :line="line"
+          :lines="lines"
           :note="discussionNote"
           :can-suggest="canSuggest"
           :add-spacing-classes="false"
           :help-page-path="helpPagePath"
           :show-suggest-popover="showSuggestPopover"
           :textarea-value="updatedNoteBody"
-          :lines="lines"
           @handleSuggestDismissed="() => $emit('handleSuggestDismissed')"
         >
           <template #textarea>

spec/frontend/diffs/components/diff_line_note_form_spec.js

@@ -26,8 +26,9 @@ describe('DiffLineNoteForm', () => {
       propsData: {
         diffFileHash: diffFile.file_hash,
         diffLines,
-        line: diffLines[0],
-        noteTargetLine: diffLines[0],
+        line: diffLines[1],
+        range: { start: diffLines[0], end: diffLines[1] },
+        noteTargetLine: diffLines[1],
       },
     });
   };
@@ -67,7 +68,7 @@ describe('DiffLineNoteForm', () => {
         expect(window.confirm).not.toHaveBeenCalled();
         wrapper.vm.$nextTick(() => {
           expect(wrapper.vm.cancelCommentForm).toHaveBeenCalledWith({
-            lineCode: diffLines[0].line_code,
+            lineCode: diffLines[1].line_code,
             fileHash: wrapper.vm.diffFileHash,
           });
 
@@ -88,13 +89,13 @@ describe('DiffLineNoteForm', () => {
           start: {
             line_code: wrapper.vm.commentLineStart.line_code,
             type: wrapper.vm.commentLineStart.type,
-            new_line: 1,
+            new_line: 2,
             old_line: null,
           },
           end: {
             line_code: wrapper.vm.line.line_code,
             type: wrapper.vm.line.type,
-            new_line: 1,
+            new_line: 2,
             old_line: null,
           },
         };
@@ -120,7 +121,7 @@ describe('DiffLineNoteForm', () => {
 
   describe('mounted', () => {
     it('should init autosave', () => {
-      const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
+      const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2';
       wrapper = createComponent();
 
       expect(wrapper.vm.autosave).toBeDefined();

spec/frontend/diffs/utils/diff_line_spec.js

+import { pickDirection } from '~/diffs/utils/diff_line';
+
+describe('diff_line utilities', () => {
+  describe('pickDirection', () => {
+    const left = {
+      line_code: 'left',
+    };
+    const right = {
+      line_code: 'right',
+    };
+    const defaultLine = {
+      left,
+      right,
+    };
+
+    it.each`
+      code       | pick     | line           | pickDescription
+      ${'left'}  | ${left}  | ${defaultLine} | ${'the left line'}
+      ${'right'} | ${right} | ${defaultLine} | ${'the right line'}
+      ${'junk'}  | ${left}  | ${defaultLine} | ${'the default: the left line'}
+      ${'junk'}  | ${right} | ${{ right }}   | ${"the right line if there's no left line to default to"}
+      ${'right'} | ${left}  | ${{ left }}    | ${"the left line when there isn't a right line to match"}
+    `(
+      'when provided a line and a line code `$code`, picks $pickDescription',
+      ({ code, line, pick }) => {
+        expect(pickDirection({ line, code })).toBe(pick);
+      },
+    );
+  });
+});

spec/frontend/notes/components/multiline_comment_form_spec.js

@@ -50,18 +50,6 @@ describe('MultilineCommentForm', () => {
       expect(wrapper.vm.commentLineStart).toEqual(lineRange.start);
       expect(setSelectedCommentPosition).toHaveBeenCalled();
     });
-
-    it('sets commentLineStart to selectedCommentPosition', () => {
-      const notes = {
-        selectedCommentPosition: {
-          start: { ...testLine },
-        },
-      };
-      const wrapper = createWrapper({}, { notes });
-
-      expect(wrapper.vm.commentLineStart).toEqual(wrapper.vm.selectedCommentPosition.start);
-      expect(setSelectedCommentPosition).not.toHaveBeenCalled();
-    });
   });
 
   describe('destroyed', () => {