Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When use with create_list, build method returns same instance every time #1499

Closed
shrkw opened this issue Jun 17, 2021 · 3 comments
Closed
Labels

Comments

@shrkw
Copy link

shrkw commented Jun 17, 2021

Description

I want to create Post record and Tag records on one transaction due to schema constraint and validation.
So, I wrote FactoryBot.create(:post, tags: [FactoryBot.build(:tag)]) and works perfect.
However when create multiple records by call create_list, FactoryBot.build(:tag) returns same instance every time, so Tag records are updated multiple times.

D, [2021-06-17T09:38:20.196778 #46727] DEBUG -- :   TRANSACTION (0.1ms)  begin transaction
D, [2021-06-17T09:38:20.197197 #46727] DEBUG -- :   Post Create (0.1ms)  INSERT INTO "posts" DEFAULT VALUES
D, [2021-06-17T09:38:20.198598 #46727] DEBUG -- :   Tag Create (0.2ms)  INSERT INTO "tags" ("post_id") VALUES (?)  [["post_id", 1]]
D, [2021-06-17T09:38:20.199523 #46727] DEBUG -- :   TRANSACTION (0.2ms)  commit transaction
D, [2021-06-17T09:38:20.202593 #46727] DEBUG -- :   TRANSACTION (0.2ms)  begin transaction
D, [2021-06-17T09:38:20.203601 #46727] DEBUG -- :   Post Create (0.5ms)  INSERT INTO "posts" DEFAULT VALUES
D, [2021-06-17T09:38:20.205742 #46727] DEBUG -- :   Tag Update (0.2ms)  UPDATE "tags" SET "post_id" = ? WHERE "tags"."id" = ?  [["post_id", 2], ["id", 1]]
D, [2021-06-17T09:38:20.206156 #46727] DEBUG -- :   TRANSACTION (0.2ms)  commit transaction

Is this designed behavior?

Reproduction Steps

A reproduction script is here.

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "factory_bot", "6.2.0"
  gem "activerecord"
  gem "sqlite3"
end

require "active_record"
require "factory_bot"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
  end
  create_table :tags, force: true do |t|
    t.references :post, foreign_key: true, null: false
  end
end

class Post < ActiveRecord::Base
  has_many :tags, dependent: :destroy
  validates :tags, presence: true
end

class Tag < ActiveRecord::Base
  belongs_to :post
end

FactoryBot.define do
  factory :post do
  end
  factory :tag do
  end
end

class FactoryBotTest < Minitest::Test
  def test_create_post_with_children_on_single_transaction
    post = FactoryBot.create(:post, tags: [FactoryBot.build(:tag)])
    assert_equal post.tags.count, 1
  end
  def test_create_post_list_with_children_on_single_transaction
    posts = FactoryBot.create_list(:post, 2, tags: [FactoryBot.build(:tag)])

    refute_equal posts[0].tags, posts[1].tags
  end
end

Expected behavior

build returns different instance every time if it is called with *_list method.

Actual behavior

build returns same instance.

System configuration

factory_bot version: 6.2.0
rails version: 6.0.3.7
ruby version: 2.7.2

@shrkw shrkw added the bug label Jun 17, 2021
@composerinteralia
Copy link
Collaborator

I see why this is confusing, but I don't think it's a bug in factory_bot. An equivalent way of writing this in ruby that I think makes the problem a little clearer:

tag = FactoryBot.build(:tag)
FactoryBot.create_list(:post, 2, tags: [tag])

We build a tag, and then we pass that one tag to create_list. With this example, I would expect for each post to have the same tag. There's nothing here that would suggest create_list should create new tags along the way.

If you want to execute different code for each element in the list, I believe you can do that in a block passed to create_list. I'm not sure how well documented that is, but it might look something like:

FactoryBot.create_list(:post, 2) do
  post.update!(tags: [FactoryBot.build(:tag)])
end

@shrkw
Copy link
Author

shrkw commented Jun 18, 2021

Hmm... your example cannot pass validates :tags, presence: true validation.

  1) Error:
FactoryBotTest#test_create_post_list_with_children_on_single_transaction:
ActiveRecord::RecordInvalid: Validation failed: Tags can't be blank
    /usr/local/bundle/gems/activerecord-6.1.3.2/lib/active_record/validations.rb:80:in `raise_validation_error'
    /usr/local/bundle/gems/activerecord-6.1.3.2/lib/active_record/validations.rb:53:in `save!'
    /usr/local/bundle/gems/activerecord-6.1.3.2/lib/active_record/transactions.rb:302:in `block in save!'

Should I use prevent create_list method?

    posts = [
      FactoryBot.create(:post, tags: [FactoryBot.build(:tag)]),
      FactoryBot.create(:post, tags: [FactoryBot.build(:tag)])
    ]

@composerinteralia
Copy link
Collaborator

composerinteralia commented Jun 18, 2021

Ah right, I missed your validation (I'm used to tags being a many-to-many relationship rather than a many-to-one). In that case you might consider one of the various solutions we have for has_many associations so that you can do FactoryBot.create(:post) and have factory_bot create the required tags. Avoiding create_list is certainly an option as well, but you would still need to build the tags every time you create a post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants