Update the code to work with Rails 4.

[rafael]

Hi,

I was playing today with the Gem and notice that there are some changes that are required to make it work in Rails 4. I did these changes and make sure that the specs are passing.

Cheers!

[Jacke]

Thaks. Need approve this PR @tcocca

[tcocca]

Thanks for the effort here to get this working with rails 4. However, #apply_finder_options has been deprecated from rails 4 and will be removed in rails 4.1, https://github.com/rails/activerecord-deprecated_finders/blob/master/lib/active_record/deprecated_finders/relation.rb#L12

Can you update the code to not use that method? Why was .all(options) no longer working?

From this post: http://blog.remarkablelabs.com/2012/12/what-s-new-in-active-record-rails-4-countdown-to-2013

Model.all

No longer will a call to Model.all execute a query immediately and return an array of records. In Rails 4, calls to Model.all is equivalent to now deprecated Model.scoped. This means that more relations can be chained to Model.all and the result will be lazily evaluated.

To me that means we can keep using .all(options) but it must be following by an enumeration method .collect or something else, or a .to_a call, so I'm not sure why we needed to do this:

•     self.followings.unblocked.includes(:follower).all(options).collect{|f| f.follower}
  

•    self.followings.unblocked.includes(:follower).
  

•      apply_finder_options(options,true).
  

•      to_a.collect{|f| f.follower}
  

Thanks again,
~ Tom

[rafael]

Hi Tom,

Thanks for the feedback! . I tried to use .all first, but when you pass options it gives a deprecation warning. It seems that you can't do .all(options) anymore:

DEPRECATION WARNING: Relation#all is deprecated. If you want to eager-load a relation, you can call #load (e.g. `Post.where(published: true).load`). If you want to get an array of records from a relation, you can call #to_a (e.g. `Post.where(published: true).to_a`). (called from irb_binding at (irb):4)

The only workaround that I was able to find to keep the exact same functionality, is to use the deprecated method with the parameter set to false. At least in this way it's less noisy.

I think that in order to make it fully compatible with rails 4, we would have to remove the options parameters altogether and let the user add the scopes on demand:

user.all_follows.limit(10)

instead of :

user.all_follows(limit: 10)

But this is not backwards compatible... What do you think is the best approach here?

Cheers!

[tcocca]

Hi Rafael,

I now see that all "options hash" args are deprecated for finders. So, {:limit => 10, :order => "created_at desc"} is all deprecated ...

I'm wondering if maybe we should do some stuff like this method: https://github.com/tcocca/acts_as_follower/blob/master/lib/acts_as_follower/followable.rb#L24

Maybe something along the lines of:

def followers(options={})
  followers_scope = self.followings.unblocked.includes(:follower)
  if options.has_key?(:limit)
    followers_scope = followers_scope.limit(options[:limit])
  end
  if options.has_key?(:includes)
    followers_scope = followers_scope.includes(options[:includes])
  end
  followers_scope.to_a.collect{|f| f.follower}
end

Where we only support a few items in the options hash, maybe limit, order, where, select, includes, joins (not sure what else ...)

Also, maybe we should also have a method that just returns the arel chain as well instead of the array, so that people can do all their own chaining on it themselves:

def followers_scope
  self.followings.unblocked.includes(:follower)
end

Something like that, then people can use it as they want to? Its almost like providing a "named scope" out of the box for them except its on an instance instead of the class ...

Let me know what you think.

Thanks,
~ Tom

[nfedyashev]

@rafael @tcocca
Your work is much appreciated 💚

[tcocca]

@rafael any thoughts on my comments above?

[rafael]

@tcocca Yeah I think that makes sense! It has been a crazy busy week for me, so I didn't have the chance to review the comments here.

I'm going to update the pull request this week following the suggestions above.

Cheers

[rafael]

@tcocca I just updated the code :) . What do you think?

[rafael]

@tcocca did you have a chance to take a look at the newest commits?

[tcocca]

@rafael sorry, my brother got married over the weekend, I haven't had a chance yet. I will try and look at these tonight or tomorrow. Thanks again

[tcocca]

@rafael @Jacke @nfedyashev I just released acts_as_follower v 0.2.0 which now includes this awesome work @rafael put it on getting the rails 4 support working including backward compatibility, thank you so much for the help with this gem.

http://rubygems.org/gems/acts_as_follower

I also updated the README for the new installation and the documentation for the new scope methods.

[rafael]

Awesome! :)