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

Confusion around aliases for {attribute} and {attribute}_id #1142

Open
composerinteralia opened this issue Jun 12, 2018 · 18 comments · May be fixed by #1709
Open

Confusion around aliases for {attribute} and {attribute}_id #1142

composerinteralia opened this issue Jun 12, 2018 · 18 comments · May be fixed by #1709

Comments

@composerinteralia
Copy link
Collaborator

composerinteralia commented Jun 12, 2018

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.

@shyam-habarakada
Copy link

shyam-habarakada commented Aug 12, 2018

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

@composerinteralia
Copy link
Collaborator Author

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!.

@agirdler-lumoslabs
Copy link

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?

@composerinteralia
Copy link
Collaborator Author

composerinteralia commented Jan 1, 2019

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.

@jakswa
Copy link

jakswa commented Apr 8, 2019

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. 🎉

@jsmartt
Copy link

jsmartt commented Jun 14, 2019

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.

@viral810
Copy link

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.

@jvenezia
Copy link

jvenezia commented Apr 28, 2020

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

@fdutey

This comment has been minimized.

@composerinteralia
Copy link
Collaborator Author

We have a Code of conduct that we expect all contributors to follow.

@natematykiewicz
Copy link

natematykiewicz commented Jul 8, 2022

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}

@stephannv
Copy link

stephannv commented Jun 28, 2023

Two years ago I opened this issue #1512, and now I lost some time investigating why my tests aren't passing 😅.

I think a solution could be:

  • remove _id alias from FactoryBot
  • If this feature is still wanted, move it to FactoryBot Rails
    • handle only foreign key attributes, not all attributes with _id suffix
    • this can be made with:
      ActiveRecord::Base.connection.foreign_keys(TABLE_NAME_HERE).any? { |fk| fk.options[:name] == ATTR_HERE }

@natematykiewicz
Copy link

natematykiewicz commented Jul 18, 2023

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.

@gap777
Copy link

gap777 commented Aug 16, 2024

Is there really no workaround to this?

@natematykiewicz
Copy link

natematykiewicz commented Aug 16, 2024

@gap777 I was able to work around this issue:

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}

By adding this in the factory:

after :build do |product, options|
  product.platform_id = options.platform_id
  product.platform = options.platform
end

@gap777
Copy link

gap777 commented Aug 16, 2024

Grrrr... trying that.. not working for me...
Did you declare transients as well?

@natematykiewicz
Copy link

No, just regular FactoryBot attributes:

platform { 'stripe' }
platform_id { "price_#{SecureRandom.base58(24)}" }

@gap777
Copy link

gap777 commented Aug 16, 2024

@composerinteralia's suggestion worked for me (#1142 (comment)):

transient do
    thing_id { 0 }
    thing { "thing" }
  end

  initialize_with do
    new(attributes.merge(thing_id: thing_id, thing: thing))
  end

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

Successfully merging a pull request may close this issue.