#287 Associations' dynamic finders fail with :order depends on :include - 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.

#287 ✓resolved

Associations' dynamic finders fail with :order depends on :include

Reported by Brandon Keepers | May 31st, 2008 @ 11:43 PM | in 2.1.1

This ticket was originally submitted on trac

Dynamic finders generate invalid queries when they are called on an association like: has_many :posts, :include => :comments, :order_by => 'comments.position' because :include is getting ignored. (using non-dynamic #find() works correctly)

I've attached test cases that cause this error and a patch to fix it. Tested against Trunk.

dynamic_finders_on_associations_with_include_and_order.diff 7.3 KB

Comments and changes to this ticket

You flagged this item as spam.
Daniel Morrison June 9th, 2008 @ 04:04 PM
+1
Rick June 9th, 2008 @ 04:17 PM
- Milestone set to “2.1.1”
- State changed from “new” to “open”
- Assigned user set to “Rick”
Can you create a git patch? See the docs on contributing.
You flagged this item as spam.
Brandon Keepers June 9th, 2008 @ 04:33 PM
Updated patch for latest changes in edge rails and exported using git-format-patch
- dynamic_finder_with_eager_loading.diff 7.3 KB
You flagged this item as spam.
Brandon Keepers June 9th, 2008 @ 04:41 PM
Doh, just realized I committed the change to the mysql connection info. Path updated
- dynamic_finder_with_eager_loading.diff 6.6 KB
Rick June 9th, 2008 @ 05:11 PM
- State changed from “open” to “committed”
Hey, I've got this pushed to my own fork. I started pushing some config.gem tweaks there so others could play with them, before they were finalized in rails core. If I was smart, I would've put them in a branch so that I don't have other patches like this one stuck in limbo. We'll merge it back in for 2.1.1 though.
Thanks.
Jeremy Kemper June 9th, 2008 @ 08:08 PM
Could you cherry-pick to master, Rick?
You flagged this item as spam.
Brandon Keepers June 17th, 2008 @ 07:42 PM
Any way we can get this pushed or cherry-picked into the main repo?
Jeremy Kemper June 17th, 2008 @ 07:53 PM
It should be cherry-picked to 2-1-stable as well.
Rick June 19th, 2008 @ 06:02 PM
Pushed, but I don't see a 2-1 stable branch.
Rick June 19th, 2008 @ 06:02 PM
- State changed from “committed” to “resolved”