Merge branch 'ph/220923/hideResolveDiscussion' into 'master'

Hides resolve buttons for users who don't have permission

See merge request gitlab-org/gitlab!46020

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

@@ -45,7 +45,7 @@ export default {
       return this.discussion.notes.filter(x => x.resolvable);
     },
     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);
     },
   },
 };

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

@@ -121,7 +121,13 @@ export default {
       return this.withBatchComments && this.noteId === '' && !this.discussion.for_commit;
     },
     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() {
       if (this.noteId) {

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

@@ -141,6 +141,8 @@ export default {
     canResolve() {
       if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false;
 
+      if (this.glFeatures.removeResolveNote) return this.note.current_user.can_resolve_discussion;
+
       return (
         this.note.current_user.can_resolve ||
         (this.note.isDraft && this.note.discussion_id !== null)

app/serializers/note_entity.rb

@@ -34,6 +34,10 @@ class NoteEntity < API::Entities::Note
     expose :can_resolve do |note|
       note.resolvable? && can?(current_user, :resolve_note, note)
     end
+
+    expose :can_resolve_discussion do |note|
+      note.discussion.resolvable? && note.discussion.can_resolve?(current_user)
+    end
   end
 
   expose :suggestions, using: SuggestionEntity

spec/frontend/notes/components/discussion_actions_spec.js

@@ -13,11 +13,11 @@ const createDiscussionMock = (props = {}) =>
 const createNoteMock = (props = {}) =>
   Object.assign(JSON.parse(JSON.stringify(discussionMock.notes[0])), props);
 const createResolvableNote = () =>
-  createNoteMock({ resolvable: true, current_user: { can_resolve: true } });
+  createNoteMock({ resolvable: true, current_user: { can_resolve_discussion: true } });
 const createUnresolvableNote = () =>
-  createNoteMock({ resolvable: false, current_user: { can_resolve: false } });
+  createNoteMock({ resolvable: false, current_user: { can_resolve_discussion: false } });
 const createUnallowedNote = () =>
-  createNoteMock({ resolvable: true, current_user: { can_resolve: false } });
+  createNoteMock({ resolvable: true, current_user: { can_resolve_discussion: false } });
 
 describe('DiscussionActions', () => {
   let wrapper;

spec/frontend/notes/components/note_form_spec.js

@@ -272,6 +272,7 @@ describe('issue_note_form component', () => {
       wrapper = createComponentWrapper();
       wrapper.setProps({
         ...props,
+        isDraft: true,
         noteId: '',
         discussion: { ...discussionMock, for_commit: false },
       });
@@ -292,6 +293,27 @@ describe('issue_note_form component', () => {
       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', () => {
       wrapper.setProps({ discussion: { for_commit: true } });
 

spec/frontend/notes/mock_data.js

@@ -202,6 +202,7 @@ export const discussionMock = {
         can_edit: true,
         can_award_emoji: true,
         can_resolve: true,
+        can_resolve_discussion: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,
@@ -249,6 +250,7 @@ export const discussionMock = {
         can_edit: true,
         can_award_emoji: true,
         can_resolve: true,
+        can_resolve_discussion: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,
@@ -296,6 +298,7 @@ export const discussionMock = {
         can_edit: true,
         can_award_emoji: true,
         can_resolve: true,
+        can_resolve_discussion: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,

spec/support/shared_examples/serializers/note_entity_shared_examples.rb

@@ -20,6 +20,39 @@ RSpec.shared_examples 'note entity' do
     it 'does not expose web_url for author' do
       expect(subject[:author]).not_to include(:web_url)
     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
 
   context 'when note was edited' do