From 2560757ea2c41ec859a196f679cd87d9274768d2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 2 May 2021 12:56:06 +0100 Subject: [PATCH 1/2] Change Enterprise after_create callback to after_create_commit As a general rule, if you're triggering an email job as part of an after create/save callback, it should use after commit instead. Why? The transaction can't finish until after the record is persisted (the data is committed) which includes the logic in all callbacks. So for example if the transaction fails after the email job has been placed it will be rolled back, but the email job will already be in the queue, and it'll be referencing a record that doesn't actually exist (due to the rollback). --- app/models/enterprise.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index f77e634e96..b749096b94 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -105,7 +105,7 @@ class Enterprise < ApplicationRecord after_touch :touch_distributors after_create :set_default_contact after_create :relate_to_owners_enterprises - after_create :send_welcome_email + after_create_commit :send_welcome_email after_rollback :restore_permalink From 7c2d77a3ee78c1c158fd43cfd5f9fb42bd07e9cf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 2 May 2021 13:41:04 +0100 Subject: [PATCH 2/2] Fix assertion in ModelSet test The mismatched use of hash attributes as strings and hash attributes as symbols here (attrs['name'] and attrs[:name]) meant that the conditional was not returning the expected results in the test. --- spec/services/sets/model_set_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/sets/model_set_spec.rb b/spec/services/sets/model_set_spec.rb index d9154c5bf1..97528f5df4 100644 --- a/spec/services/sets/model_set_spec.rb +++ b/spec/services/sets/model_set_spec.rb @@ -51,7 +51,7 @@ describe Sets::ModelSet do attributes = { collection_attributes: { '1' => { name: 'deleteme' } } } ms = Sets::ModelSet.new(Enterprise, Enterprise.all, attributes, nil, - proc { |attrs| attrs['name'] == 'deleteme' }) + proc { |attrs| attrs[:name] == 'deleteme' }) expect { ms.save }.to change(Enterprise, :count).by(0) end