factory_bot
factory_bot copied to clipboard
Confusion around aliases for `{attribute}` and `{attribute}_id`
We have had several issues over the years related to FactoryBot.aliases: #522 #734 #851 #989 #1096 #1138, #1417. In most of these issues people do not realize that factory_bot assumes{attribute}_id will be the foreign key of an {attribute} ActiveRecord association.
At the very least I think we need better documentation around this. We could probably also offer better configuration for this, and maybe move anything specific to ActiveRecord into factory_bot_rails.
Just wasted a few hours because of this. The workaround I applied (based on other issues linked and other research) was as below.
factory :person
name
email
...
transient do
thing_id { 0 }
thing { nil } # Thing.to_s or similar
end
after :create do |person, options|
person.thing_id = options.thing_id
person.thing = options.thing
person.save!
end
end
Feedback on this would be great. Thanks
Yup, that seems to match the solution in #1096. I think you could use after(:build) instead of an after(:create), then get rid of the call to save!.
Is there a planned "official" solution to this? While hacking transient and after callbacks works in most cases I haven't been able to get it to work properly with a hanami/model backed model (the object is frozen during initialization) and instead resorted to hacking an non_aliasable method on FactoryBot then using that in a monkey patched definition of FactoryBot.aliases_for:
module FactoryBot
def self.non_aliasable
%w[processor processor_id]
end
def self.aliases_for(attribute)
aliases.map do |(pattern, replace)|
if pattern.match(attribute.to_s) && !non_aliasable.include?(attribute.to_s)
attribute.to_s.sub(pattern, replace).to_sym
end
end.compact << attribute
end
end
This has worked great so far for the project I'm working on, but it doesn't really feel maintainable (as it's a list stored in a module). I'm also not familiar enough with the code to add a method to the DSL to allow defining these on a per model basis but I think that would be the most flexible solution (but may require a rewrite so FactoryBot.aliases_for is builder aware?). Are there any opinions on this approach?
I haven't spent too much time thinking about an official solution for this. I could imagine something like:
factory :person do
skip_aliases
# ...
end
to opt-out of aliases for just that one factory, but I haven't fully thought that through.
In the meantime, you could try using initialize_with instead of the after callback:
factory :person do
name { "Daniel" }
transient do
thing_id { 0 }
thing { "thing" }
end
initialize_with do
new(attributes.merge(thing_id: thing_id, thing: thing))
end
end
And maybe something like
factory :person do
skip_aliases :thing, :thing_id
# ...
end
to just skip specific aliases for a factory.
What a nightmare this was to troubleshoot. Count me and a few other coworkers in on wishing factory bot wasn't opinionated on all _id columns. :tada:
The workarounds posted above help with building/creating, but the attributes are excluded from the attributes_for response. 😞
I'd really like a way to define these as attributes (which they are), and have factory_bot treat them as plain-text column names if they are not associations. The skip_aliases suggestion by @composerinteralia would work, but why does defining an attribute on a factory not automatically accomplish this? There is a separate method for defining associations, so I'd expect that defining attributes to have different behavior.
If you have a column name that ends with _id i.e. thing_id and thing is not model is itself then make sure that model related to that table is explicitly specifying table name in the model file. In my case, I had the following issue.
FactoryBot.define do
factory :domain_account, class: "Domain::Account" do
user
thing_id { SecureRandom.hex(13) }
name { "John Doe" }
sequence(:email_address) { |n| "test+#{n}@example.com" }
end
end
causing me an exception that thing association doesn't exist but by adding self.table_name to the model resolved the issue.
# app/models/domain/account.rb
class Domain::Account < ApplicationRecord
belongs_to :user
self.table_name = "domain_accounts"
end
Not too sure if other people are having a problem because of namespaced models. Interested to see if this resolves the problem for anyone else.
I spent way to much time understanding what was happening... I dont have a suggestion but we definitely need an improvement here!
In my case, thing was a boolean and thing_id was a string not related at all to a rails model.
Adding this to the factory was enough for me:
after :build do |user, options|
user.thing_id = options.thing_id
user.thing = options.thing
end
It was more important to rename from girl to bot to make some snowflakes happy instead of solving real bugs
We have a Code of conduct that we expect all contributors to follow.
Just bumping this to say that I just ran into this as well. I have a model that has a platform column, and a platform_id column. For example:
{
platform: 'stripe',
platform_id: 'sub_123456789',
}
{
platform: 'apple',
platform_id: 'SOME_APPLE_ITEM_ID',
}
I just spent a good 2 hours trying to figure out why this was happening:
FactoryBot.build(:product, platform: 'apple', platform_id: 'abc123').attributes.slice('platform', 'platform_id')
=> {"platform"=>"apple", "platform_id"=>nil}
Two years ago I opened this issue https://github.com/thoughtbot/factory_bot/issues/1512, and now I lost some time investigating why my tests aren't passing 😅.
I think a solution could be:
- remove
_idalias from FactoryBot - If this feature is still wanted, move it to FactoryBot Rails
- handle only foreign key attributes, not all attributes with
_idsuffix - this can be made with:
ActiveRecord::Base.connection.foreign_keys(TABLE_NAME_HERE).any? { |fk| fk.options[:name] == ATTR_HERE }
- handle only foreign key attributes, not all attributes with
In talking with Shopify on the Rails Discord about something different, I learned that they don't use actual foreign keys in the database because they do online schema migrations with 0 downtime.
So, it's probably best to simply check the model and see if the field is a foreign key in an association. The whole point of this code is to treat the _id field and association the same, so just checking associations rather than columns makes more sense anyways.
model = Foo
field = 'bar_id'
foreign_key = model.reflect_on_association(field.delete_suffix('_id'))&.foreign_key
needs_alias = foreign_key.is_a?(Array) ? foreign_key.include?(field) : foreign_key == field
If needs_alias is true, do what it does today. If it's false, just treat it as a regular field instead of assuming it's a foreign key to an association.
Note: The array check is for Rails 7.1 composite primary keys.