Commit 6907d7a7 authored by Phil Hughes's avatar Phil Hughes Committed by Kushal Pandya

Remove resolve comment functionality

Removes the resolve button from each note component
Instead only the first note has this button,
when this button is clicked it resolves the whole thread.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/28750
parent ada28f94
import { mapGetters } from 'vuex'; import { mapGetters } from 'vuex';
import { sprintf, s__, __ } from '~/locale'; import { sprintf, s__, __ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default { export default {
mixins: [glFeatureFlagsMixin()],
props: { props: {
discussionId: { discussionId: {
type: String, type: String,
...@@ -54,6 +56,10 @@ export default { ...@@ -54,6 +56,10 @@ export default {
let title = __('Mark as resolved'); let title = __('Mark as resolved');
if (this.glFeatures.removeResolveNote) {
title = __('Resolve thread');
}
if (this.resolvedBy) { if (this.resolvedBy) {
title = sprintf(__('Resolved by %{name}'), { name: this.resolvedBy.name }); title = sprintf(__('Resolved by %{name}'), { name: this.resolvedBy.name });
} }
......
...@@ -127,6 +127,7 @@ export default { ...@@ -127,6 +127,7 @@ export default {
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
:show-reply-button="userCanReply" :show-reply-button="userCanReply"
:discussion-root="true" :discussion-root="true"
:discussion-resolve-path="discussion.resolve_path"
@handleDeleteNote="$emit('deleteNote')" @handleDeleteNote="$emit('deleteNote')"
@startReplying="$emit('startReplying')" @startReplying="$emit('startReplying')"
> >
...@@ -171,6 +172,7 @@ export default { ...@@ -171,6 +172,7 @@ export default {
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
:line="diffLine" :line="diffLine"
:discussion-root="index === 0" :discussion-root="index === 0"
:discussion-resolve-path="discussion.resolve_path"
@handleDeleteNote="$emit('deleteNote')" @handleDeleteNote="$emit('deleteNote')"
> >
<slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot> <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot>
......
...@@ -73,6 +73,11 @@ export default { ...@@ -73,6 +73,11 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
discussionResolvePath: {
type: String,
required: false,
default: '',
},
}, },
data() { data() {
return { return {
...@@ -81,6 +86,7 @@ export default { ...@@ -81,6 +86,7 @@ export default {
isRequesting: false, isRequesting: false,
isResolving: false, isResolving: false,
commentLineStart: {}, commentLineStart: {},
resolveAsThread: this.glFeatures.removeResolveNote,
}; };
}, },
computed: { computed: {
...@@ -133,6 +139,8 @@ export default { ...@@ -133,6 +139,8 @@ export default {
return this.note.isDraft; return this.note.isDraft;
}, },
canResolve() { canResolve() {
if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false;
return ( return (
this.note.current_user.can_resolve || this.note.current_user.can_resolve ||
(this.note.isDraft && this.note.discussion_id !== null) (this.note.isDraft && this.note.discussion_id !== null)
......
import { deprecatedCreateFlash as Flash } from '~/flash'; import { deprecatedCreateFlash as Flash } from '~/flash';
import { __ } from '~/locale'; import { __ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default { export default {
mixins: [glFeatureFlagsMixin()],
computed: { computed: {
discussionResolved() { discussionResolved() {
if (this.discussion) { if (this.discussion) {
const { notes, resolved } = this.discussion; const { notes, resolved } = this.discussion;
if (this.glFeatures.removeResolveNote) {
return resolved;
}
if (notes) { if (notes) {
// Decide resolved state using store. Only valid for discussions. // Decide resolved state using store. Only valid for discussions.
return notes.filter(note => !note.system).every(note => note.resolved); return notes.filter(note => !note.system).every(note => note.resolved);
...@@ -38,7 +44,12 @@ export default { ...@@ -38,7 +44,12 @@ export default {
this.isResolving = true; this.isResolving = true;
const isResolved = this.discussionResolved || resolvedState; const isResolved = this.discussionResolved || resolvedState;
const discussion = this.resolveAsThread; const discussion = this.resolveAsThread;
const endpoint = discussion ? this.discussion.resolve_path : `${this.note.path}/resolve`; let endpoint =
discussion && this.discussion ? this.discussion.resolve_path : `${this.note.path}/resolve`;
if (this.glFeatures.removeResolveNote && this.discussionResolvePath) {
endpoint = this.discussionResolvePath;
}
return this.toggleResolveNote({ endpoint, isResolved, discussion }) return this.toggleResolveNote({ endpoint, isResolved, discussion })
.then(() => { .then(() => {
......
...@@ -40,6 +40,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -40,6 +40,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:highlight_current_diff_row, @project) push_frontend_feature_flag(:highlight_current_diff_row, @project)
push_frontend_feature_flag(:default_merge_ref_for_diffs, @project) push_frontend_feature_flag(:default_merge_ref_for_diffs, @project)
push_frontend_feature_flag(:core_security_mr_widget, @project, default_enabled: true) push_frontend_feature_flag(:core_security_mr_widget, @project, default_enabled: true)
push_frontend_feature_flag(:remove_resolve_note, @project)
record_experiment_user(:invite_members_version_a) record_experiment_user(:invite_members_version_a)
record_experiment_user(:invite_members_version_b) record_experiment_user(:invite_members_version_b)
......
---
title: Remove resolve comment functionality
merge_request: 45549
author:
type: changed
---
name: remove_resolve_note
introduced_by_url:
rollout_issue_url:
type: development
group: group::source code
default_enabled: false
...@@ -8,6 +8,8 @@ RSpec.describe 'Thread Comments Merge Request', :js do ...@@ -8,6 +8,8 @@ RSpec.describe 'Thread Comments Merge Request', :js do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
before do before do
stub_feature_flags(remove_resolve_note: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
......
...@@ -18,6 +18,10 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j ...@@ -18,6 +18,10 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j
end end
end end
before do
stub_feature_flags(remove_resolve_note: false)
end
describe 'as a user with access to the project' do describe 'as a user with access to the project' do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
......
...@@ -14,6 +14,10 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue', ...@@ -14,6 +14,10 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue',
"a[title=\"#{title}\"][href=\"#{url}\"]" "a[title=\"#{title}\"][href=\"#{url}\"]"
end end
before do
stub_feature_flags(remove_resolve_note: false)
end
describe 'As a user with access to the project' do describe 'As a user with access to the project' do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
......
...@@ -15,6 +15,10 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -15,6 +15,10 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
diff_refs: merge_request.diff_refs) diff_refs: merge_request.diff_refs)
end end
before do
stub_feature_flags(remove_resolve_note: false)
end
context 'no threads' do context 'no threads' do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
......
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