Commit 7d262a25 authored by Phil Hughes's avatar Phil Hughes

Removes hide_jump_to_next_unresolved_in_threads feature flag

Removes the feature flag as well as the component
parent 8c7ac040
......@@ -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>
<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>
......@@ -34,7 +34,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)
push_frontend_feature_flag(:default_merge_ref_for_diffs, @project)
......
---
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
......@@ -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**
......
......@@ -17598,9 +17598,6 @@ msgstr ""
msgid "MergeRequests|Failed to squash. Should be done manually."
msgstr ""
msgid "MergeRequests|Jump to next unresolved thread"
msgstr ""
msgid "MergeRequests|Reply..."
msgstr ""
......
// 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>
`;
......@@ -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);
......
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',
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment