Commit 3b78bf7a authored by Serena Fang's avatar Serena Fang Committed by Bob Van Landuyt

Move bot removal into removeexpiredmembers worker

Instead of making a new worker, move project bot removal logic into
remove expired members worker
parent 17c90904
......@@ -7,11 +7,19 @@ class RemoveExpiredMembersWorker # rubocop:disable Scalability/IdempotentWorker
feature_category :authentication_and_authorization
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
def perform
Member.expired.find_each do |member|
Member.expired.preload(:user).find_each do |member|
Members::DestroyService.new.execute(member, skip_authorization: true)
expired_user = member.user
if expired_user.project_bot?
Users::DestroyService.new(nil).execute(expired_user, skip_authorization: true)
end
rescue => ex
logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}")
end
end
# rubocop: enable CodeReuse/ActiveRecord
end
---
title: Project Access Tokens - Delete project bot after token expires
merge_request: 45828
author:
type: fixed
......@@ -31,6 +31,50 @@ RSpec.describe RemoveExpiredMembersWorker do
end
end
context 'project bots' do
let(:project) { create(:project) }
context 'expired project bot', :sidekiq_inline do
let_it_be(:expired_project_bot) { create(:user, :project_bot) }
before do
project.add_user(expired_project_bot, :maintainer, expires_at: 1.day.from_now)
travel_to(3.days.from_now)
end
it 'removes expired project bot membership' do
expect { worker.perform }.to change { Member.count }.by(-1)
expect(Member.find_by(user_id: expired_project_bot.id)).to be_nil
end
it 'deletes expired project bot' do
worker.perform
expect(User.exists?(expired_project_bot.id)).to be(false)
end
end
context 'non-expired project bot' do
let_it_be(:other_project_bot) { create(:user, :project_bot) }
before do
project.add_user(other_project_bot, :maintainer, expires_at: 10.days.from_now)
travel_to(3.days.from_now)
end
it 'does not remove expired project bot that expires in the future' do
expect { worker.perform }.to change { Member.count }.by(0)
expect(other_project_bot.reload).to be_present
end
it 'does not delete project bot expiring in the future' do
worker.perform
expect(User.exists?(other_project_bot.id)).to be(true)
end
end
end
context 'group members' do
let_it_be(:expired_group_member) { create(:group_member, expires_at: 1.day.from_now, access_level: GroupMember::DEVELOPER) }
let_it_be(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
......
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