Skip to content

Commit 04a3fe3

Browse files
NikolasPilavakisneilvcarvalho
authored andcommitted
Ensure linting transactions don't interfere with ActiveRecord lifecycle
Our FactoryBot linting started failing for a factory despite evidence that the trait combination was producing a valid model in test. We eventually traced that down to the change made in #1726. Introducing a `ActiveRecord::Base.transaction` meant that the `after_commit` hooks of our model weren't running during linting. This is because of the way `ActiveRecord::Base` handles transactions. When changes are inserted for an ActiveRecord within an `ActiveRecord::Base.transaction`, that transaction keeps a list of all the records it contains changes for. as part of the transaction committing, `*_commit` record callbacks are fired `ActiveRecord::Base.transaction`s are `joinable` by default, which means that `ActiveRecord` can choose to join an existing transaction instead of inserting a new one. This means that `*_commit`s that would previously result from the object's lifecycle are not sent until the end of the linting transaction. To reinstate the previous behaviour, this PR ensures that the linting transaction is not joinable and any `*_commit` callbacks are respected.
1 parent 27e97b4 commit 04a3fe3

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

lib/factory_bot/linter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def error_message_type
109109

110110
def in_transaction
111111
if defined?(ActiveRecord::Base)
112-
ActiveRecord::Base.transaction do
112+
ActiveRecord::Base.transaction(joinable: false) do
113113
yield
114114
raise ActiveRecord::Rollback
115115
end

spec/acceptance/lint_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@
4444
expect { FactoryBot.lint }.to_not raise_error
4545
end
4646

47+
it "runs after_commit callbacks when linting in a ActiveRecord::Base transaction" do
48+
define_model "ModelWithAfterCommitCallbacks" do
49+
class_attribute :after_commit_callbacks_received
50+
51+
after_commit do
52+
self.class.after_commit_callbacks_received = true
53+
end
54+
end
55+
56+
FactoryBot.define do
57+
factory :model_with_after_commit_callbacks
58+
end
59+
60+
FactoryBot.lint
61+
expect(ModelWithAfterCommitCallbacks.after_commit_callbacks_received).to be true
62+
end
63+
4764
it "does not raise when all factories are valid" do
4865
define_model "User", name: :string do
4966
validates :name, presence: true

0 commit comments

Comments
 (0)