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

Already on GitHub? Sign in to your account

Fix Rails4 deprecation errors when getting a collection from an association #928

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

kenmazaika commented Mar 27, 2013

NOTE: this isn't ready to merge yet. it's untested and there are broken tests.

Before I spent too much time tidying this up, I figured I'd show you the fix that I made.

When upgrading to Rails4 I found that formtastic was producing this deprecation warning:

DEPRECATION WARNING: Model.scoped is deprecated. Please use Model.all instead. (called from collection_from_association at /Users/kmazaika/.rbenv/versions/2.0.0-p0/lib/ruby/gems/2.0.0/bundler/gems/formtastic-dc16ffc3dd5f/lib/formtastic/inputs/base/collections.rb:93)

Digging into it, it seems there are two problems here:

1 - .scoped(scope_conditions). Calling the scoped method is deprecated in Rails4 as indicated by the message.
2 - Using conditions with relationships is also deprecated in Rails4. Scopes should be used instead.

For example having a relationship defined by:

  has_many :yolos, :conditions => "swag_id is not null"

Produces the following deprecation message:

DEPRECATION WARNING: The following options in your Fomo.has_many :yolos declaration are deprecated: :conditions. Please use a scope block instead. For example, the following:

    has_many :spam_comments, conditions: { spam: true }, class_name: 'Comment'

should be rewritten as the following:

    has_many :spam_comments, -> { where spam: true }, class_name: 'Comment'

So the correct way to write this would be:

  has_many :yolos, -> { where("swag_id is not null") }

Before I fix the tests and write new ones, I wanted to run this by you guys. Let me know if you have feedback, and I'll adjust the pull request accordingly.

@kenmazaika kenmazaika commented on an outdated diff Mar 27, 2013

lib/formtastic/inputs/base/collections.rb
end
+
+ # Apply Rails3 style conditions
+ conditions_from_reflection = (reflection.respond_to?(:options) && reflection.options[:conditions]) || {}
+ conditions_from_reflection = conditions_from_reflection.call if conditions_from_reflection.is_a?(Proc)
+ scope_conditions = conditions_from_reflection.empty? ? nil : {:conditions => conditions_from_reflection}
+
+ collection = collection.scoped(scope_conditions) if scope_conditions.present?
+
+ # Apply Rails4 style conditions (they're now scopes).
+ scope_from_reflection = reflection.respond_to?(:scope) && reflection.scope
+ collection = collection.where(scope_from_reflection.call) if scope_from_reflection.present?
@kenmazaika

kenmazaika Mar 27, 2013

Contributor

the above line is completely untested and probably doesn't work, but sort of gets the gist across.

Contributor

kenmazaika commented Apr 2, 2013

I made a git repo that illustrates the issues here. https://github.com/kenmazaika/lolsome Check out the README.

I also made it correctly deal with scopes. This stuff is still untested / breaking tests, so that needs to be fixed, but I think this code should be on the right track.

Collaborator

twalpole commented Apr 5, 2013

Running the standard tests against rails 4 beta1 and edge don't generate any deprecation warnings for me, other than the source :rubygems warning. So either we're not testing the use case you're using or we're swallowing the deprecation somewhere. Maybe the first step here should be to get a test into the current code that actually triggers the deprecation.

preston commented Jul 2, 2013

Anyone have an update on which branch we should use for Rails 4 projects? Just curious since this has been open for a few months.

Owner

justinfrench commented Jul 4, 2013

@preston master is the closest we have to Rails 4 compatible right now. I'm taking a look this evening, but I don't have time to diligently maintain this gem right now. Any help appreciated!

Owner

justinfrench commented Jul 4, 2013

Closing this PR as it's incomplete and I believe the same stuff is covered in 77bc2ae and 2f0f01c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment