diff --git a/app/models/member.rb b/app/models/member.rb index d95c8d1b41fc02c6ebefcd5b5b9ed429009cd49c..2daf597318d772268e2ee4b40192509b8bbe7c2d 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -756,7 +756,9 @@ def log_previous_state_on_update # This method is overridden in the test environment, see stubbed_member.rb def refresh_member_authorized_projects - UserProjectAccessChangedService.new(user_id).execute + return UserProjectAccessChangedService.new(user_id).execute unless bot? && destroyed? + + UserProjectAccessChangedService.new(user_id).execute(priority: UserProjectAccessChangedService::MEDIUM_PRIORITY) end # rubocop: enable CodeReuse/ServiceClass @@ -841,6 +843,10 @@ def project_bot? user&.project_bot? end + def bot? + user&.bot? + end + def log_invitation_token_cleanup return true unless Gitlab.com? && invite? && invite_accepted_at? diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 10583e8a964c575f0c1d4de67e6246dbeaf532ea..8fbb59b6bbce4a16cd20a9a0f99dd40ba8ff5888 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -1589,14 +1589,47 @@ end describe 'destroying a record', :delete, :sidekiq_inline do - it "refreshes user's authorized projects" do - project = create(:project, :private) - user = create(:user) - member = project.add_reporter(user) + let(:project) { create(:project, :private) } - member.destroy! + context 'when member is a human user' do + let(:user) { create(:user) } + + it "refreshes user's authorized projects" do + member = project.add_reporter(user) + + member.destroy! + + expect(user.authorized_projects).not_to include(project) + end + end + + # TODO: move specs to GroupMember / ProjectMember model + context 'when member is a bot user' do + let(:group) { create(:group) } + let(:project) { create(:project, :private) } + let(:bot_user) { create(:user, :project_bot) } + + it 'calls UserProjectAccessChangedService with medium priority' do + member = group.add_reporter(bot_user) - expect(user.authorized_projects).not_to include(project) + expect_next_instance_of(UserProjectAccessChangedService, bot_user.id) do |service| + expect(service).to receive(:execute).with( + priority: UserProjectAccessChangedService::MEDIUM_PRIORITY + ) + end + + member.destroy! + end + + it "refreshes user's authorized projects" do + member = project.add_reporter(bot_user) + + expect(bot_user.authorized_projects).to include(project) + + member.destroy! + + expect(bot_user.authorized_projects.reload).not_to include(project) + end end end diff --git a/spec/support/helpers/stubbed_member.rb b/spec/support/helpers/stubbed_member.rb index 7e7d7c10f00b829ff6960e9e42e9e29f4aaec416..a59b1ddf5914a9848f5a82873ad576d186e0de5a 100644 --- a/spec/support/helpers/stubbed_member.rb +++ b/spec/support/helpers/stubbed_member.rb @@ -9,26 +9,26 @@ module StubbedMember extend ActiveSupport::Concern module Member - private + # private - def refresh_member_authorized_projects - # In stubbed_member the original methods stubbed would call .perform_async - # so the affected workers would not be in a transaction in a non-test environment. - Gitlab::ExclusiveLease.skipping_transaction_check do - AuthorizedProjectsWorker.new.perform(user_id) - end - end + # def refresh_member_authorized_projects + # # In stubbed_member the original methods stubbed would call .perform_async + # # so the affected workers would not be in a transaction in a non-test environment. + # Gitlab::ExclusiveLease.skipping_transaction_check do + # AuthorizedProjectsWorker.new.perform(user_id) + # end + # end end module ProjectMember - private + # private - def execute_project_authorizations_refresh - # In stubbed_member the original methods stubbed would call .perform_async - # so the affected workers would not be in a transaction in a non-test environment. - Gitlab::ExclusiveLease.skipping_transaction_check do - AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker.new.perform(project.id, user.id) - end - end + # def execute_project_authorizations_refresh + # # In stubbed_member the original methods stubbed would call .perform_async + # # so the affected workers would not be in a transaction in a non-test environment. + # Gitlab::ExclusiveLease.skipping_transaction_check do + # AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker.new.perform(project.id, user.id) + # end + # end end end