Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Need to pass args to the build class initialize method #42

Closed
jeffrafter opened this Issue · 5 comments

3 participants

@jeffrafter

Factory girl doesn't work with classes where you override the initialize method and require params. This is common with "default" plugins like default_value_for. This patch fixes it by allowing you to set a constructor_args block that will retrieve the args that should be passed to the build class.

http://github.com/jeffrafter/factory_girl/commit/3cb82f8a7311527246d3f9c4c67d6816b26c1682

I didn't realize that rake gemspec would rev the version, so I accidentally revved the gemspec version in this commit. Let me know if that needs adjusting.

@jferris
Owner

Thanks for putting this patch together. I like this general idea, but I'd prefer a solution that would allow you to override constructor params in the factory invocation; something like:

Factory.define :post do |factory|
  factory.title     { "This is a title" }
  factory.body      { "This is a body"  }
  factory.published { false }
  factory.initialize! { |attributes| [attributes.title, attributes.body] }
end

Factory(:post, :title => 'New title')

# This would be equivilent to:

post = Post.new("New Title", "This is a body")
post.published = false
post.save!

That's harder to do, but I hope to support something like that eventually.

@aviflombaum

This is still an issue with factory_girl2.0.0.beta1. I happen to support jeffrafter's solution, sending the initialize parameters after the build seems more natural than within the factory itself. Any idea if/when this will be resolved?

@jferris
Owner

aviflombaum, I don't want to pull in the patch because it means you can't override the arguments in factory invocations. That means that I'll have to come up with another solution later and deprecate this one. This is something that I have personally run into, so I'll put together a version of my own eventually, but I can't make any promises as to when that would be.

@jeffrafter

@jferris, I am not sure what you mean, you can definitely pass in overrides in the invocation, the documentation sample and tests show that. Or is the problem that you have to predefine the hash that gets passed in? If so I think that makes some sense but it is still possible given the current minimalist patch. Also, I feel like this is a pretty edge case that most people won't use anyway... Maybe it is something that belongs in the default_values plugin.

@jferris
Owner

@jeffrafter, I had to take another look to see how you'd override the constructor arguments. Sorry for missing that earlier.

However, I still think the API should be prettier, more intuitive, and less error-prone. In particular, I don't like that the constructor args block would have to modify the overrides hash. Also, because it uses overrides directly, it wouldn't be able to build on other features in factory_girl (ie, factory inheritence and attribute aliases).

I also don't consider this a minimalist edge case. It doesn't come up frequently when using ActiveRecord, but I think this missing feature is one of the major reasons factory_girl is rarely used with non-ActiveRecord Ruby objects.

It should be possible to build on top of what you've already written to provide the syntax I described above. I can see how what you've written would work well for you, but I'm reluctant to pull it into factory_girl until the API is improved.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.