Commit 904f5cac authored by Thong Kuah's avatar Thong Kuah

Merge branch 'yorick/block-user-cross-db-query' into 'master'

Fix cross database transaction when blocking users

See merge request gitlab-org/gitlab!75195
parents 4c778e0d 783bd2b7
...@@ -391,9 +391,11 @@ class User < ApplicationRecord ...@@ -391,9 +391,11 @@ class User < ApplicationRecord
# this state transition object in order to do a rollback. # this state transition object in order to do a rollback.
# For this reason the tradeoff is to disable this cop. # For this reason the tradeoff is to disable this cop.
after_transition any => :blocked do |user| after_transition any => :blocked do |user|
user.run_after_commit do
Ci::DropPipelineService.new.execute_async_for_all(user.pipelines, :user_blocked, user) Ci::DropPipelineService.new.execute_async_for_all(user.pipelines, :user_blocked, user)
Ci::DisableUserPipelineSchedulesService.new.execute(user) Ci::DisableUserPipelineSchedulesService.new.execute(user)
end end
end
after_transition any => :deactivated do |user| after_transition any => :deactivated do |user|
next unless Gitlab::CurrentSettings.user_deactivation_emails_enabled next unless Gitlab::CurrentSettings.user_deactivation_emails_enabled
......
...@@ -1856,15 +1856,31 @@ RSpec.describe User do ...@@ -1856,15 +1856,31 @@ RSpec.describe User do
end end
context 'when user has running CI pipelines' do context 'when user has running CI pipelines' do
let(:service) { double }
let(:pipelines) { build_list(:ci_pipeline, 3, :running) } let(:pipelines) { build_list(:ci_pipeline, 3, :running) }
it 'aborts all running pipelines and related jobs' do it 'drops all running pipelines and related jobs' do
drop_service = double
disable_service = double
expect(user).to receive(:pipelines).and_return(pipelines) expect(user).to receive(:pipelines).and_return(pipelines)
expect(Ci::DropPipelineService).to receive(:new).and_return(service) expect(Ci::DropPipelineService).to receive(:new).and_return(drop_service)
expect(service).to receive(:execute_async_for_all).with(pipelines, :user_blocked, user) expect(drop_service).to receive(:execute_async_for_all).with(pipelines, :user_blocked, user)
expect(Ci::DisableUserPipelineSchedulesService).to receive(:new).and_return(disable_service)
expect(disable_service).to receive(:execute).with(user)
user.block!
end
it 'does not drop running pipelines if the transaction rolls back' do
expect(Ci::DropPipelineService).not_to receive(:new)
expect(Ci::DisableUserPipelineSchedulesService).not_to receive(:new)
User.transaction do
user.block user.block
raise ActiveRecord::Rollback
end
end end
end end
......
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