Commit 8bbd8172 authored by James Lopez's avatar James Lopez

Add transactional argument

parent edaabaf4
...@@ -11,8 +11,8 @@ module GroupSaml ...@@ -11,8 +11,8 @@ module GroupSaml
@identity = identity @identity = identity
end end
def execute def execute(transactional: false)
Identity.transaction do with_transaction(transactional) do
identity.destroy! identity.destroy!
remove_group_access remove_group_access
end end
...@@ -20,6 +20,10 @@ module GroupSaml ...@@ -20,6 +20,10 @@ module GroupSaml
private private
def with_transaction(transactional, &block)
transactional ? ::Identity.transaction { yield } : yield
end
def remove_group_access def remove_group_access
return unless group_membership return unless group_membership
return if group.last_owner?(user) return if group.last_owner?(user)
......
...@@ -17,7 +17,7 @@ module API ...@@ -17,7 +17,7 @@ module API
end end
def destroy_identity(identity) def destroy_identity(identity)
GroupSaml::Identity::DestroyService.new(identity).execute GroupSaml::Identity::DestroyService.new(identity).execute(transactional: true)
true true
rescue => e rescue => e
...@@ -34,9 +34,11 @@ module API ...@@ -34,9 +34,11 @@ module API
error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409) error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409)
end end
# rubocop: disable CodeReuse/ActiveRecord
def email_taken?(email, identity) def email_taken?(email, identity)
User.by_any_email(email.downcase).where.not(id: identity.user.id).count > 0 User.by_any_email(email.downcase).where.not(id: identity.user.id).count > 0
end end
# rubocop: enable CodeReuse/ActiveRecord
end end
resource :Users do resource :Users do
......
...@@ -21,6 +21,18 @@ describe GroupSaml::Identity::DestroyService do ...@@ -21,6 +21,18 @@ describe GroupSaml::Identity::DestroyService do
expect(identity).to be_destroyed expect(identity).to be_destroyed
end end
it "does not use a transaction" do
expect(::Identity).to receive(:transaction).and_yield.once
subject.execute
end
it "uses a transaction when transactional is set" do
expect(::Identity).to receive(:transaction).and_yield.twice
subject.execute(transactional: true)
end
it "removes access to the group" do it "removes access to the group" do
expect do expect do
subject.execute subject.execute
......
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