Merge branch 'ph/removeHideJumpThreadsFeatureFLag' into 'master'

Removes hide_jump_to_next_unresolved_in_threads feature flag

See merge request gitlab-org/gitlab!51474

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

@@ -2,7 +2,6 @@
 import ReplyPlaceholder from './discussion_reply_placeholder.vue';
 import ResolveDiscussionButton from './discussion_resolve_button.vue';
 import ResolveWithIssueButton from './discussion_resolve_with_issue_button.vue';
-import JumpToNextDiscussionButton from './discussion_jump_to_next_button.vue';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
@@ -11,7 +10,6 @@ export default {
     ReplyPlaceholder,
     ResolveDiscussionButton,
     ResolveWithIssueButton,
-    JumpToNextDiscussionButton,
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
@@ -38,9 +36,6 @@ export default {
     },
   },
   computed: {
-    hideJumpToNextUnresolvedInThreads() {
-      return this.glFeatures.hideJumpToNextUnresolvedInThreads;
-    },
     resolvableNotes() {
       return this.discussion.notes.filter((x) => x.resolvable);
     },
@@ -74,15 +69,5 @@ export default {
         :url="resolveWithIssuePath"
       />
     </div>
-    <div
-      v-if="
-        !hideJumpToNextUnresolvedInThreads &&
-        discussion.resolvable &&
-        shouldShowJumpToNextDiscussion
-      "
-      class="btn-group discussion-actions ml-sm-2"
-    >
-      <jump-to-next-discussion-button :from-discussion-id="discussion.id" />
-    </div>
   </div>
 </template>

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

-<script>
-import { GlTooltipDirective, GlIcon } from '@gitlab/ui';
-import discussionNavigation from '../mixins/discussion_navigation';
-
-export default {
-  name: 'JumpToNextDiscussionButton',
-  components: {
-    GlIcon,
-  },
-  directives: {
-    GlTooltip: GlTooltipDirective,
-  },
-  mixins: [discussionNavigation],
-  props: {
-    fromDiscussionId: {
-      type: String,
-      required: true,
-    },
-  },
-};
-</script>
-
-<template>
-  <div class="btn-group" role="group">
-    <button
-      ref="button"
-      v-gl-tooltip
-      class="btn btn-default discussion-next-btn"
-      :title="s__('MergeRequests|Jump to next unresolved thread')"
-      data-track-event="click_button"
-      data-track-label="mr_next_unresolved_thread"
-      data-track-property="click_next_unresolved_thread"
-      @click="jumpToNextRelativeDiscussion(fromDiscussionId)"
-    >
-      <gl-icon name="comment-next" />
-    </button>
-  </div>
-</template>

app/controllers/projects/merge_requests_controller.rb

@@ -33,7 +33,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     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)
-    push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true)
     push_frontend_feature_flag(:merge_request_widget_graphql, @project)
     push_frontend_feature_flag(:unified_diff_components, @project, default_enabled: true)
     push_frontend_feature_flag(:default_merge_ref_for_diffs, @project)

config/feature_flags/development/hide_jump_to_next_unresolved_in_threads.yml

----
-name: hide_jump_to_next_unresolved_in_threads
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37873
-rollout_issue_url: 
-milestone: '13.3'
-type: development
-group: group::code review
-default_enabled: true

doc/user/discussions/index.md

@@ -90,27 +90,6 @@ When a link of a commit reference is found in a thread inside a merge
 request, it will be automatically converted to a link in the context of the
 current merge request.
 
-### Jumping between unresolved threads (deprecated)
-
-> - [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/199718) in GitLab 13.3.
-> - This button's removal is behind a feature flag enabled by default.
-> - For GitLab self-managed instances, GitLab administrators with access to the
-  [GitLab Rails console](../../administration/feature_flags.md) can opt to disable it by running
-  `Feature.disable(:hide_jump_to_next_unresolved_in_threads)` (for the instance) or
-  `Feature.disable(:hide_jump_to_next_unresolved_in_threads, Project.find(<project id>))`
-  (per project.) **(CORE ONLY)**
-
-When a merge request has a large number of comments it can be difficult to track
-what remains unresolved. You can jump between unresolved threads with the
-Jump button next to the Reply field on a thread.
-
-You can also use keyboard shortcuts to navigate among threads:
-
-- Use <kbd>n</kbd> to jump to the next unresolved thread.
-- Use <kbd>p</kbd> to jump to the previous unresolved thread.
-
-!["8/9 threads resolved"](img/threads_resolved.png)
-
 ### Marking a comment or thread as resolved
 
 You can mark a thread as resolved by clicking the **Resolve thread**

locale/gitlab.pot

@@ -17679,9 +17679,6 @@ msgstr ""
 msgid "MergeRequests|Failed to squash. Should be done manually."
 msgstr ""
 
-msgid "MergeRequests|Jump to next unresolved thread"
-msgstr ""
-
 msgid "MergeRequests|Reply..."
 msgstr ""
 

spec/frontend/notes/components/__snapshots__/discussion_jump_to_next_button_spec.js.snap

-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`JumpToNextDiscussionButton matches the snapshot 1`] = `
-<div
-  class="btn-group"
-  role="group"
->
-  <button
-    class="btn btn-default discussion-next-btn"
-    data-track-event="click_button"
-    data-track-label="mr_next_unresolved_thread"
-    data-track-property="click_next_unresolved_thread"
-    title="Jump to next unresolved thread"
-  >
-    <gl-icon-stub
-      name="comment-next"
-      size="16"
-    />
-  </button>
-</div>
-`;

spec/frontend/notes/components/discussion_actions_spec.js

@@ -4,7 +4,6 @@ import DiscussionActions from '~/notes/components/discussion_actions.vue';
 import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
 import ResolveDiscussionButton from '~/notes/components/discussion_resolve_button.vue';
 import ResolveWithIssueButton from '~/notes/components/discussion_resolve_with_issue_button.vue';
-import JumpToNextDiscussionButton from '~/notes/components/discussion_jump_to_next_button.vue';
 import createStore from '~/notes/stores';
 
 // NOTE: clone mock_data so that it is not accidentally mutated
@@ -21,7 +20,7 @@ const createUnallowedNote = () =>
 
 describe('DiscussionActions', () => {
   let wrapper;
-  const createComponentFactory = (shallow = true) => (props, options) => {
+  const createComponentFactory = (shallow = true) => (props) => {
     const store = createStore();
     const mountFn = shallow ? shallowMount : mount;
 
@@ -35,11 +34,6 @@ describe('DiscussionActions', () => {
         shouldShowJumpToNextDiscussion: true,
         ...props,
       },
-      provide: {
-        glFeatures: {
-          hideJumpToNextUnresolvedInThreads: options?.hideJumpToNextUnresolvedInThreads,
-        },
-      },
     });
   };
 
@@ -55,7 +49,6 @@ describe('DiscussionActions', () => {
       expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true);
       expect(wrapper.find(ResolveDiscussionButton).exists()).toBe(true);
       expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(true);
-      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(true);
     });
 
     it('only renders reply placholder if disccusion is not resolvable', () => {
@@ -66,7 +59,6 @@ describe('DiscussionActions', () => {
       expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true);
       expect(wrapper.find(ResolveDiscussionButton).exists()).toBe(false);
       expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(false);
-      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(false);
     });
 
     it('does not render resolve with issue button if resolveWithIssuePath is falsy', () => {
@@ -75,12 +67,6 @@ describe('DiscussionActions', () => {
       expect(wrapper.find(ResolveWithIssueButton).exists()).toBe(false);
     });
 
-    it('does not render jump to next discussion button if shouldShowJumpToNextDiscussion is false', () => {
-      createComponent({ shouldShowJumpToNextDiscussion: false });
-
-      expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(false);
-    });
-
     describe.each`
       desc                         | notes                                                 | shouldRender
       ${'with no notes'}           | ${[]}                                                 | ${true}
@@ -101,13 +87,6 @@ describe('DiscussionActions', () => {
     });
   });
 
-  it('does not render jump to next discussion button if feature flag is enabled', () => {
-    const createComponent = createComponentFactory();
-    createComponent({}, { hideJumpToNextUnresolvedInThreads: true });
-
-    expect(wrapper.find(JumpToNextDiscussionButton).exists()).toBe(false);
-  });
-
   describe('events handling', () => {
     const createComponent = createComponentFactory(false);
 

spec/frontend/notes/components/discussion_jump_to_next_button_spec.js

-import { shallowMount } from '@vue/test-utils';
-import JumpToNextDiscussionButton from '~/notes/components/discussion_jump_to_next_button.vue';
-import { mockTracking } from '../../helpers/tracking_helper';
-
-describe('JumpToNextDiscussionButton', () => {
-  const fromDiscussionId = 'abc123';
-  let wrapper;
-  let trackingSpy;
-  let jumpFn;
-
-  beforeEach(() => {
-    jumpFn = jest.fn();
-    wrapper = shallowMount(JumpToNextDiscussionButton, {
-      propsData: { fromDiscussionId },
-    });
-
-    jest.spyOn(wrapper.vm, 'jumpToNextRelativeDiscussion').mockImplementation(jumpFn);
-
-    trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
-  });
-
-  afterEach(() => {
-    wrapper.destroy();
-  });
-
-  it('matches the snapshot', () => {
-    expect(wrapper.vm.$el).toMatchSnapshot();
-  });
-
-  it('calls jumpToNextRelativeDiscussion when clicked', () => {
-    wrapper.find({ ref: 'button' }).trigger('click');
-
-    expect(jumpFn).toHaveBeenCalledWith(fromDiscussionId);
-  });
-
-  it('sends the correct tracking event when clicked', () => {
-    wrapper.find({ ref: 'button' }).trigger('click');
-
-    expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
-      label: 'mr_next_unresolved_thread',
-      property: 'click_next_unresolved_thread',
-    });
-  });
-});