#1845 Finder methods ignore :include in has_many :through - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1845 ✓resolved

Finder methods ignore :include in has_many :through

Reported by Eduardo | February 2nd, 2009 @ 04:07 PM

If an has_many :through association has an :include and :conditions in it, calling a finder method on it will fail if the conditions refer to the included table, because the :include is ignored.

With this simple class definitions:


class Author < ActiveRecord::Base
has_many :authorships
has_many :books_with_preview, :source => :book,
         :through => :authorships, :include => :chapters,
         :conditions => ['chapters.preview = 1']
end

class Authorship < ActiveRecord::Base
belongs_to :author
belongs_to :book
end

class Book < ActiveRecord::Base
has_many :authorship
has_many :chapters
end

class Chapter < ActiveRecord::Base
belongs_to :book
end

you can call


author.books_with_preview

but the following call will fail:


author.books_with_preview.find(:all)

I'm using rails 2.1.0 but I've been told that it's the same in rails 2.2.2. I tested mysql and sqlite, and I get the same error:

ActiveRecord::StatementInvalid: Mysql::Error: Unknown column 'chapters.preview' in 'where clause': SELECT books.* FROM books INNER JOIN authorships ON books.id = authorships.book_id WHERE ((authorships.author_id = 1) AND ((chapters.preview = 1)))

Comments and changes to this ticket

You flagged this item as spam.
JackC February 18th, 2009 @ 05:27 PM
- Tag changed from “:include, :through, finder” to “:include, :through, eagerloading, eager_loading, find, finder”
This is also an issue in 2.3 RC1.

I've also found that all works even the find(:all) doesn't.

It seems to be fixable by inserting the following line at line 52.
```
options[:include] = @reflection.options[:include]
```
But I don't know enough about the ActiveRecord internals to know if this will break something.
You flagged this item as spam.
JackC February 18th, 2009 @ 05:28 PM
Line 52 of active_record/associations/association_collection.rb that is.
CancelProfileIsBroken August 5th, 2009 @ 01:47 PM
- Tag changed from “:include, :through, eagerloading, eager_loading, find, finder” to “:include, :through, bugmash, eagerloading, eager_loading, find, finder”
You flagged this item as spam.
railsbob August 9th, 2009 @ 01:08 PM
verified

This error is reproducible. The error is caused because options[:include] was assigned nil in has_many_through_association.rb.
```
def construct_find_options!(options)
  options[:select]  = construct_select(options[:select])
  options[:from]  ||= construct_from
  options[:joins]   = construct_joins(options[:joins])
  options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? 
end
```
options[:include] should not be nil, because the reverse_merge! prohibits assignment from @reflection.options[:include] in merge_options_from_reflection!(options) method (association_proxy.rb), even if the value is nil.
The solution is not to have options[:include] if the value is nil and appropriate :include options are picked from reflection.

I have attached a patch with corresponding test, which fixed the issue.
- 1845_fixed_has_many_through_find.diff 2.2 KB
You flagged this item as spam.
Hugo Peixoto August 9th, 2009 @ 01:29 PM
+1 railsbob's patch works fine in both 2-3-stable and master, and without the fix the test fails.
Rizwan Reza August 9th, 2009 @ 03:06 PM
verified

+1 The patch works perfectly under master and 2-3-stable. All tests pass.
David Trasbo August 9th, 2009 @ 04:00 PM
+1

Bug verified to be reproducible. Patch applies cleanly to edge Rails and all tests are passing.
Repository August 9th, 2009 @ 05:06 PM
- State changed from “new” to “resolved”
(from [076ca48bd649ddea4dd1a320879c03a9fe7a0a6d]) Ensure hm:t#find does not assign nil to :include [#1845 state:resolved]

Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/076ca48bd649ddea4dd1a320879c03...
Repository August 9th, 2009 @ 05:06 PM
(from [80d8608102cce64805e7573f6b97d8561f7f11ea]) Ensure hm:t#find does not assign nil to :include [#1845 state:resolved]

Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/80d8608102cce64805e7573f6b97d8...
CancelProfileIsBroken August 9th, 2009 @ 05:13 PM
- Tag changed from “:include, :through, bugmash, eagerloading, eager_loading, find, finder” to “:include, :through, eagerloading, eager_loading, find, finder”
- Milestone cleared.
You flagged this item as spam.
Josh N. Abbott November 17th, 2009 @ 07:50 PM
Has this issue really been resolved? I'm running Rails 2.3.4 and when I try to load a has_many :through association, the construct_find_options! method doesn't get called. The find_target method is what gets called and in spite of the fact that it appears that the :include option is getting passed to the subsequently called 'find' method, the associations are being ignored.

Another odd thing is that when I call #count on the association everything loads fine.

So the question is, did this patch actually solve the original problem the ticket was opened for? It seems to me that it didn't.