Update deprecated ActiveRecord includes syntax

[jcf]

No description provided.

[karmi]

Good, but I think we need to tread more carefully here. It's bad the former syntax was deprecated, since it allowed not only includes, but also conditions, etc. I'm afraid we can't just change it like this, and we definitely unit and integration tests for this.

[jcf]

The chainable API is the preferred way of doing conditions etc. now and has been since Rails 3.0.

What's your stance on Rails version support in Tire? I think this change will cover Rails 3 & 4, and can look into the tests sometime early next week if you want something like this mergeable into upstream.

[karmi]

@jcf I know the hash interface has been deprecated. I haven't really put much thought how to support the chainable interface in the hash options to search. Maybe the only use case is include(), maybe there are other use cases...

[Old doc: http://apidock.com/rails/v2.3.8/ActiveRecord/Base/find/class]

[jcf]

I remember the good old…

User.find(:all, :conditions => {:name => 'Karel'}, :includes => [:elastic_search])

…API. If only I could count the number of hashes I must have initialised during my time with Rails 2. Fond memories. 😄

The first thing that comes to mind is taking a loader block that Tire could use, perhaps yielding the list of IDs to the block so the user can implement find in its entirety.

loader = ->(ids) { Article.includes(:comments).where(published: true).find(ids) }
Article.search(loader: loader) do |s|
  s.query # etc.
end

Or perhaps add a method to the class to do the finding with a list of IDs?

class Article
  def self.find_for_tire(ids)
    includes(:comments).where(id: ids)
  end
end

I'm sure there's a better way but that's what popped into my mind.

P.S. I've force pushed up to my includes branch as I was using @options[:load] instead of @options[:load][:include].

Thanks for sharing Tire with the Ruby community. Have a great weekend!

[karmi]

Yeah, the good old days :)

I like the idea with lambda, since that could allow to leave the old option supported, and evaluate the lambda (polymorphism FTW) as the "finder", exactly as you propose:

Article.search 'love', load: { include: "comments" }

Article.search 'love', load: lambda { |ids| includes("comments").find(ids) }

Also see #702 and #602 where we discuss some strategies how to mitigate the issue of "record no longer in db".

Also related: #762.