Commit 9d572085 authored by Phil Hughes's avatar Phil Hughes Committed by Rémy Coutable

Hides resolve buttons for users who don't have permission

This hides any resolve action from users who don't
have the correct permissions to resolve discussions.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/220923
parent 161ba793
...@@ -45,7 +45,7 @@ export default { ...@@ -45,7 +45,7 @@ export default {
return this.discussion.notes.filter(x => x.resolvable); return this.discussion.notes.filter(x => x.resolvable);
}, },
userCanResolveDiscussion() { userCanResolveDiscussion() {
return this.resolvableNotes.every(note => note.current_user && note.current_user.can_resolve); return this.resolvableNotes.every(note => note.current_user?.can_resolve_discussion);
}, },
}, },
}; };
......
...@@ -121,7 +121,13 @@ export default { ...@@ -121,7 +121,13 @@ export default {
return this.withBatchComments && this.noteId === '' && !this.discussion.for_commit; return this.withBatchComments && this.noteId === '' && !this.discussion.for_commit;
}, },
showResolveDiscussionToggle() { showResolveDiscussionToggle() {
return (this.discussion?.id && this.discussion.resolvable) || this.isDraft; if (!this.discussion?.notes) return false;
return (
this.discussion?.notes
.filter(n => n.resolvable)
.some(n => n.current_user?.can_resolve_discussion) || this.isDraft
);
}, },
noteHash() { noteHash() {
if (this.noteId) { if (this.noteId) {
......
...@@ -141,6 +141,8 @@ export default { ...@@ -141,6 +141,8 @@ export default {
canResolve() { canResolve() {
if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false; if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false;
if (this.glFeatures.removeResolveNote) return this.note.current_user.can_resolve_discussion;
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)
......
...@@ -34,6 +34,10 @@ class NoteEntity < API::Entities::Note ...@@ -34,6 +34,10 @@ class NoteEntity < API::Entities::Note
expose :can_resolve do |note| expose :can_resolve do |note|
note.resolvable? && can?(current_user, :resolve_note, note) note.resolvable? && can?(current_user, :resolve_note, note)
end end
expose :can_resolve_discussion do |note|
note.discussion.resolvable? && note.discussion.can_resolve?(current_user)
end
end end
expose :suggestions, using: SuggestionEntity expose :suggestions, using: SuggestionEntity
......
...@@ -13,11 +13,11 @@ const createDiscussionMock = (props = {}) => ...@@ -13,11 +13,11 @@ const createDiscussionMock = (props = {}) =>
const createNoteMock = (props = {}) => const createNoteMock = (props = {}) =>
Object.assign(JSON.parse(JSON.stringify(discussionMock.notes[0])), props); Object.assign(JSON.parse(JSON.stringify(discussionMock.notes[0])), props);
const createResolvableNote = () => const createResolvableNote = () =>
createNoteMock({ resolvable: true, current_user: { can_resolve: true } }); createNoteMock({ resolvable: true, current_user: { can_resolve_discussion: true } });
const createUnresolvableNote = () => const createUnresolvableNote = () =>
createNoteMock({ resolvable: false, current_user: { can_resolve: false } }); createNoteMock({ resolvable: false, current_user: { can_resolve_discussion: false } });
const createUnallowedNote = () => const createUnallowedNote = () =>
createNoteMock({ resolvable: true, current_user: { can_resolve: false } }); createNoteMock({ resolvable: true, current_user: { can_resolve_discussion: false } });
describe('DiscussionActions', () => { describe('DiscussionActions', () => {
let wrapper; let wrapper;
......
...@@ -272,6 +272,7 @@ describe('issue_note_form component', () => { ...@@ -272,6 +272,7 @@ describe('issue_note_form component', () => {
wrapper = createComponentWrapper(); wrapper = createComponentWrapper();
wrapper.setProps({ wrapper.setProps({
...props, ...props,
isDraft: true,
noteId: '', noteId: '',
discussion: { ...discussionMock, for_commit: false }, discussion: { ...discussionMock, for_commit: false },
}); });
...@@ -292,6 +293,27 @@ describe('issue_note_form component', () => { ...@@ -292,6 +293,27 @@ describe('issue_note_form component', () => {
expect(wrapper.find('.js-resolve-checkbox').exists()).toBe(true); expect(wrapper.find('.js-resolve-checkbox').exists()).toBe(true);
}); });
it('hides resolve checkbox', async () => {
wrapper.setProps({
isDraft: false,
discussion: {
...discussionMock,
notes: [
...discussionMock.notes.map(n => ({
...n,
resolvable: true,
current_user: { ...n.current_user, can_resolve_discussion: false },
})),
],
for_commit: false,
},
});
await wrapper.vm.$nextTick();
expect(wrapper.find('.js-resolve-checkbox').exists()).toBe(false);
});
it('hides actions for commits', () => { it('hides actions for commits', () => {
wrapper.setProps({ discussion: { for_commit: true } }); wrapper.setProps({ discussion: { for_commit: true } });
......
...@@ -202,6 +202,7 @@ export const discussionMock = { ...@@ -202,6 +202,7 @@ export const discussionMock = {
can_edit: true, can_edit: true,
can_award_emoji: true, can_award_emoji: true,
can_resolve: true, can_resolve: true,
can_resolve_discussion: true,
}, },
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1', discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true, emoji_awardable: true,
...@@ -249,6 +250,7 @@ export const discussionMock = { ...@@ -249,6 +250,7 @@ export const discussionMock = {
can_edit: true, can_edit: true,
can_award_emoji: true, can_award_emoji: true,
can_resolve: true, can_resolve: true,
can_resolve_discussion: true,
}, },
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1', discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true, emoji_awardable: true,
...@@ -296,6 +298,7 @@ export const discussionMock = { ...@@ -296,6 +298,7 @@ export const discussionMock = {
can_edit: true, can_edit: true,
can_award_emoji: true, can_award_emoji: true,
can_resolve: true, can_resolve: true,
can_resolve_discussion: true,
}, },
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1', discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true, emoji_awardable: true,
......
...@@ -20,6 +20,39 @@ RSpec.shared_examples 'note entity' do ...@@ -20,6 +20,39 @@ RSpec.shared_examples 'note entity' do
it 'does not expose web_url for author' do it 'does not expose web_url for author' do
expect(subject[:author]).not_to include(:web_url) expect(subject[:author]).not_to include(:web_url)
end end
it 'exposes permission fields on current_user' do
expect(subject[:current_user]).to include(:can_edit, :can_award_emoji, :can_resolve, :can_resolve_discussion)
end
describe ':can_resolve_discussion' do
context 'discussion is resolvable' do
before do
expect(note.discussion).to receive(:resolvable?).and_return(true)
end
context 'user can resolve' do
it 'is true' do
expect(note.discussion).to receive(:can_resolve?).with(user).and_return(true)
expect(subject[:current_user][:can_resolve_discussion]).to be_truthy
end
end
context 'user cannot resolve' do
it 'is false' do
expect(note.discussion).to receive(:can_resolve?).with(user).and_return(false)
expect(subject[:current_user][:can_resolve_discussion]).to be_falsey
end
end
end
context 'discussion is not resolvable' do
it 'is false' do
expect(note.discussion).to receive(:resolvable?).and_return(false)
expect(subject[:current_user][:can_resolve_discussion]).to be_falsey
end
end
end
end end
context 'when note was edited' do context 'when note was edited' do
......
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