Merge branch 'kerrizor/remove_multiline_comments-feature-flag' into 'master'

Remove multiline_comments feature flag

See merge request gitlab-org/gitlab!51763

app/assets/javascripts/batch_comments/components/draft_note.vue

@@ -3,7 +3,6 @@
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { GlButton } from '@gitlab/ui';
 import NoteableNote from '~/notes/components/noteable_note.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import PublishButton from './publish_button.vue';
 
 export default {
@@ -12,7 +11,6 @@ export default {
     PublishButton,
     GlButton,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     draft: {
       type: Object,
@@ -63,14 +61,14 @@ export default {
       this.isEditingDraft = false;
     },
     handleMouseEnter(draft) {
-      if (this.glFeatures.multilineComments && draft.position) {
+      if (draft.position) {
         this.setSelectedCommentPositionHover(draft.position.line_range);
       }
     },
     handleMouseLeave(draft) {
-      // Even though position isn't used here we still don't want to unecessarily call a mutation
+      // Even though position isn't used here we still don't want to unnecessarily call a mutation
       // The lack of position tells us that highlighting is irrelevant in this context
-      if (this.glFeatures.multilineComments && draft.position) {
+      if (draft.position) {
         this.setSelectedCommentPositionHover();
       }
     },

app/assets/javascripts/batch_comments/components/preview_item.vue

@@ -3,7 +3,6 @@ import { mapGetters } from 'vuex';
 import { GlSprintf, GlIcon } from '@gitlab/ui';
 import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants';
 import { sprintf, __ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import {
   getStartLineNumber,
   getEndLineNumber,
@@ -16,7 +15,7 @@ export default {
     GlIcon,
     GlSprintf,
   },
-  mixins: [resolvedStatusMixin, glFeatureFlagsMixin()],
+  mixins: [resolvedStatusMixin],
   props: {
     draft: {
       type: Object,
@@ -71,6 +70,10 @@ export default {
       return this.draft.position || this.discussion.position;
     },
     startLineNumber() {
+      if (this.position?.position_type === IMAGE_DIFF_POSITION_TYPE) {
+        // eslint-disable-next-line @gitlab/require-i18n-strings
+        return `${this.position.x}x ${this.position.y}y`;
+      }
       return getStartLineNumber(this.position?.line_range);
     },
     endLineNumber() {
@@ -90,16 +93,12 @@ export default {
   <span>
     <span class="review-preview-item-header">
       <gl-icon class="flex-shrink-0" :name="iconName" />
-      <span
-        class="bold text-nowrap"
-        :class="{ 'gl-align-items-center': glFeatures.multilineComments }"
-      >
+      <span class="bold text-nowrap gl-align-items-center">
         <span class="review-preview-item-header-text block-truncated">
           {{ titleText }}
         </span>
         <template v-if="showLinePosition">
-          <template v-if="!glFeatures.multilineComments">:{{ linePosition }}</template>
-          <template v-else-if="startLineNumber === endLineNumber">
+          <template v-if="startLineNumber === endLineNumber">
             :<span :class="getLineClasses(startLineNumber)">{{ startLineNumber }}</span>
           </template>
           <gl-sprintf v-else :message="__(':%{startLine} to %{endLine}')">

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

@@ -165,7 +165,7 @@ export default {
 
 <template>
   <div class="content discussion-form discussion-form-container discussion-notes">
-    <div v-if="glFeatures.multilineComments" class="gl-mb-3 gl-text-gray-500 gl-pb-3">
+    <div class="gl-mb-3 gl-text-gray-500 gl-pb-3">
       <multiline-comment-form
         v-model="commentLineStart"
         :line="line"

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

@@ -4,7 +4,6 @@ import { __ } from '~/locale';
 import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue';
 import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue';
 import SystemNote from '~/vue_shared/components/notes/system_note.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { SYSTEM_NOTE } from '../constants';
 import NoteableNote from './noteable_note.vue';
 import ToggleRepliesWidget from './toggle_replies_widget.vue';
@@ -18,7 +17,6 @@ export default {
     NoteEditedText,
     DiscussionNotesRepliesWrapper,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     discussion: {
       type: Object,
@@ -96,14 +94,14 @@ export default {
       return note.isPlaceholderNote ? note.notes[0] : note;
     },
     handleMouseEnter(discussion) {
-      if (this.glFeatures.multilineComments && discussion.position) {
+      if (discussion.position) {
         this.setSelectedCommentPositionHover(discussion.position.line_range);
       }
     },
     handleMouseLeave(discussion) {
-      // Even though position isn't used here we still don't want to unecessarily call a mutation
+      // Even though position isn't used here we still don't want to unnecessarily call a mutation
       // The lack of position tells us that highlighting is irrelevant in this context
-      if (this.glFeatures.multilineComments && discussion.position) {
+      if (discussion.position) {
         this.setSelectedCommentPositionHover();
       }
     },

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

@@ -3,7 +3,6 @@ import $ from 'jquery';
 import { mapGetters, mapActions } from 'vuex';
 import { escape } from 'lodash';
 import { GlSprintf, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { truncateSha } from '~/lib/utils/text_utility';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import httpStatusCodes from '~/lib/utils/http_status';
@@ -38,7 +37,7 @@ export default {
   directives: {
     SafeHtml,
   },
-  mixins: [noteable, resolvable, glFeatureFlagsMixin()],
+  mixins: [noteable, resolvable],
   props: {
     note: {
       type: Object,
@@ -160,7 +159,6 @@ export default {
     },
     showMultiLineComment() {
       if (
-        !this.glFeatures.multilineComments ||
         !this.discussionRoot ||
         this.startLineNumber.length === 0 ||
         this.endLineNumber.length === 0

app/controllers/projects/merge_requests_controller.rb

@@ -30,7 +30,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   before_action :authenticate_user!, only: [:assign_related_issues]
   before_action :check_user_can_push_to_source_branch!, only: [:rebase]
   before_action only: [:show] do
-    push_frontend_feature_flag(:multiline_comments, @project, default_enabled: true)
     push_frontend_feature_flag(:file_identifier_hash)
     push_frontend_feature_flag(:batch_suggestions, @project, default_enabled: true)
     push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true)

config/feature_flags/development/multiline_comments.yml

----
-name: multiline_comments
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37114
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/211255
-milestone: '13.2'
-type: development
-group: group::code review
-default_enabled: true

doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md

@@ -195,12 +195,7 @@ to expand the diff lines and leave a comment, just as you would for a changed li
 ### Commenting on multiple lines
 
 > - [Introduced](https://gitlab.com/gitlab-org/ux-research/-/issues/870) in GitLab 13.2.
-> - It's deployed behind a feature flag, enabled by default.
-> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/221268) on GitLab 13.3.
-> - It's enabled on GitLab.com.
-> - It can be disabled or enabled per-project.
-> - It's recommended for production use.
-> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-multiline-comments). **(FREE SELF)**
+> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/299121) in GitLab 13.9.
 
 GitLab provides a way to select which lines of code a comment refers to. After starting a comment
 a dropdown selector is shown to select the first line that this comment refers to.
@@ -216,25 +211,6 @@ above it.
 
 ![Multiline comment selection displayed above comment](img/multiline-comment-saved.png)
 
-### Enable or disable multiline comments **(FREE SELF)**
-
-The multiline comments feature is under development but ready for production use.
-It is deployed behind a feature flag that is **disabled by default**.
-[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
-can opt to enable it for your instance.
-
-To disable it:
-
-```ruby
-Feature.disable(:multiline_comments)
-```
-
-To enable it:
-
-```ruby
-Feature.enable(:multiline_comments)
-```
-
 ## Pipeline status in merge requests widgets
 
 If you've set up [GitLab CI/CD](../../../ci/README.md) in your project,

ee/spec/frontend/diffs/components/diff_line_note_form_spec.js

@@ -38,6 +38,7 @@ describe('EE DiffLineNoteForm', () => {
               diff_refs: {
                 head_sha: headSha || null,
               },
+              highlighted_diff_lines: [],
             })),
           },
         },

spec/frontend/batch_comments/components/draft_note_spec.js

@@ -21,14 +21,11 @@ describe('Batch comments draft note component', () => {
 
   const getList = () => getByRole(wrapper.element, 'list');
 
-  const createComponent = (propsData = { draft }, features = {}) => {
+  const createComponent = (propsData = { draft }) => {
     wrapper = shallowMount(localVue.extend(DraftNote), {
       store,
       propsData,
       localVue,
-      provide: {
-        glFeatures: { multilineComments: true, ...features },
-      },
     });
 
     jest.spyOn(wrapper.vm.$store, 'dispatch').mockImplementation();
@@ -145,16 +142,14 @@ describe('Batch comments draft note component', () => {
 
   describe('multiline comments', () => {
     describe.each`
-      desc                          | props                 | features                        | event           | expectedCalls
-      ${'with `draft.position`'}    | ${draftWithLineRange} | ${{}}                           | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
-      ${'with `draft.position`'}    | ${draftWithLineRange} | ${{}}                           | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
-      ${'with `draft.position`'}    | ${draftWithLineRange} | ${{ multilineComments: false }} | ${'mouseenter'} | ${[]}
-      ${'with `draft.position`'}    | ${draftWithLineRange} | ${{ multilineComments: false }} | ${'mouseleave'} | ${[]}
-      ${'without `draft.position`'} | ${{}}                 | ${{}}                           | ${'mouseenter'} | ${[]}
-      ${'without `draft.position`'} | ${{}}                 | ${{}}                           | ${'mouseleave'} | ${[]}
-    `('$desc and features $features', ({ props, event, features, expectedCalls }) => {
+      desc                          | props                 | event           | expectedCalls
+      ${'with `draft.position`'}    | ${draftWithLineRange} | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
+      ${'with `draft.position`'}    | ${draftWithLineRange} | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
+      ${'without `draft.position`'} | ${{}}                 | ${'mouseenter'} | ${[]}
+      ${'without `draft.position`'} | ${{}}                 | ${'mouseleave'} | ${[]}
+    `('$desc', ({ props, event, expectedCalls }) => {
       beforeEach(() => {
-        createComponent({ draft: { ...draft, ...props } }, features);
+        createComponent({ draft: { ...draft, ...props } });
         jest.spyOn(store, 'dispatch');
       });
 

spec/frontend/batch_comments/components/preview_item_spec.js

@@ -56,17 +56,30 @@ describe('Batch comments draft preview item component', () => {
       createComponent(false, {
         file_path: 'index.js',
         file_hash: 'abc',
-        position: { new_line: 1 },
+        position: {
+          line_range: {
+            start: {
+              new_line: 1,
+              type: 'new',
+            },
+          },
+        },
       });
 
-      expect(vm.$el.querySelector('.bold').textContent).toContain(':1');
+      expect(vm.$el.querySelector('.bold').textContent).toContain(':+1');
     });
 
     it('renders old line position', () => {
       createComponent(false, {
         file_path: 'index.js',
         file_hash: 'abc',
-        position: { old_line: 2 },
+        position: {
+          line_range: {
+            start: {
+              old_line: 2,
+            },
+          },
+        },
       });
 
       expect(vm.$el.querySelector('.bold').textContent).toContain(':2');

spec/frontend/diffs/components/diff_line_note_form_spec.js

@@ -17,6 +17,7 @@ describe('DiffLineNoteForm', () => {
     const store = createStore();
     store.state.notes.userData.id = 1;
     store.state.notes.noteableData = noteableDataMock;
+    store.state.diffs.diffFiles = [diffFile];
 
     store.replaceState({ ...store.state, ...args.state });
 

spec/frontend/notes/components/discussion_notes_spec.js

@@ -23,7 +23,7 @@ describe('DiscussionNotes', () => {
   let wrapper;
 
   const getList = () => getByRole(wrapper.element, 'list');
-  const createComponent = (props, features = {}) => {
+  const createComponent = (props) => {
     wrapper = shallowMount(DiscussionNotes, {
       store,
       propsData: {
@@ -38,9 +38,6 @@ describe('DiscussionNotes', () => {
       slots: {
         'avatar-badge': '<span class="avatar-badge-slot-content" />',
       },
-      provide: {
-        glFeatures: { multilineComments: true, ...features },
-      },
     });
   };
 
@@ -177,16 +174,14 @@ describe('DiscussionNotes', () => {
   });
 
   describe.each`
-    desc                               | props                                         | features                        | event           | expectedCalls
-    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{}}                           | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
-    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{}}                           | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
-    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{ multilineComments: false }} | ${'mouseenter'} | ${[]}
-    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{ multilineComments: false }} | ${'mouseleave'} | ${[]}
-    ${'without `discussion.position`'} | ${{}}                                         | ${{}}                           | ${'mouseenter'} | ${[]}
-    ${'without `discussion.position`'} | ${{}}                                         | ${{}}                           | ${'mouseleave'} | ${[]}
-  `('$desc and features $features', ({ props, event, features, expectedCalls }) => {
+    desc                               | props                                         | event           | expectedCalls
+    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
+    ${'with `discussion.position`'}    | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
+    ${'without `discussion.position`'} | ${{}}                                         | ${'mouseenter'} | ${[]}
+    ${'without `discussion.position`'} | ${{}}                                         | ${'mouseleave'} | ${[]}
+  `('$desc', ({ props, event, expectedCalls }) => {
     beforeEach(() => {
-      createComponent(props, features);
+      createComponent(props);
       jest.spyOn(store, 'dispatch');
     });
 

spec/frontend/notes/components/noteable_note_spec.js

@@ -8,15 +8,6 @@ import NoteActions from '~/notes/components/note_actions.vue';
 import NoteBody from '~/notes/components/note_body.vue';
 import { noteableDataMock, notesDataMock, note } from '../mock_data';
 
-jest.mock('~/vue_shared/mixins/gl_feature_flags_mixin', () => () => ({
-  inject: {
-    glFeatures: {
-      from: 'glFeatures',
-      default: () => ({ multilineComments: true }),
-    },
-  },
-}));
-
 describe('issue_note', () => {
   let store;
   let wrapper;