factory_bot
factory_bot copied to clipboard
Improve user feedback if defining static association with a block
Problem this feature will solve
Given:
FactoryBot.define do
user(factory: :author) { association(:author, role: role) }
end
DefinitionProxy#method_missing
will declare the association and ignore any given block
-argument.
Desired solution
DefinitionProxy#method_missing
could warn users (this is just demo code :)):
def method_missing(name, *args, &block) # rubocop:disable Style/MissingRespondToMissing
association_options = args.first
if association_options.nil?
__declare_attribute__(name, block)
elsif __valid_association_options?(association_options)
Kernel.puts 'warn: Dynamically provided attributes are ignored if `factory`-key is set.' if block
association(name, association_options)
else
raise NoMethodError.new(<<~MSG)
undefined method '#{name}' in '#{@definition.name}' factory
Did you mean? '#{name} { #{association_options.inspect} }'
MSG
end
end
Nice idea. What if we forward the block along:
elsif __valid_association_options?(association_options)
- association(name, association_options)
+ association(name, association_options, &block)
So we can get the error already defined in the association method:
https://github.com/thoughtbot/factory_bot/blob/907139808d728dbe8da59856afe068b7c0699b2a/lib/factory_bot/definition_proxy.rb#L152-L156
That's technically a breaking change, so we may need to wait until factory_bot 7.
So we can get the error already defined in the association method:
That look pretty clean to me 🤔 if you are interested, I could create a PR.
Yeah, that would be great. Thanks!
@composerinteralia I see grekko closed the PR. Was this feature implemented already? As I want to give it a try, is this still a good first issue? :)
@sharvy I'm also tracking this issue. I think it was not implemented, see https://github.com/thoughtbot/factory_bot/pull/1518#issuecomment-1587525855.
Hello 👋🏽
I see grekko closed the PR. Was this feature implemented already?
Yeah, I think I've prematurely cleaned up 🧹 the forked repository.
@sharvy I restored the fork and created a new PR which contains the proposed changes. see https://github.com/thoughtbot/factory_bot/pull/1579