#222 named_scope :order on association with :order is unintuitive - 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.

#222 ✓stale

named_scope :order on association with :order is unintuitive

Reported by Henrik Nyh | May 19th, 2008 @ 04:58 PM

If I have

class Foo < ActiveRecord::Base
  has_many :bars, :order => 'one'
end

class Bar < ActiveRecord::Base
  named_scope :by_two, :order => 'two'
  named_scope :by_three, :order => 'three'
end

then

>>> a_foo.bars.by_two

will generate SQL containing "ORDER BY one, two".

This is unintuitive to me: I would either expect to override the order entirely (ORDER BY two) or for the last scope to have precedence (ORDER BY two, one).

>>> a_foo.bars.by_two.by_three

will generate the same SQL: "ORDER BY one, two".

This is unintuitive as well; I would expect it to either use the last given order (ORDER by three) or all orders, with right-to-left precedence (ORDER BY three, two, one).

Comments and changes to this ticket

josh July 17th, 2008 @ 01:50 AM
- Assigned user set to “Pratik”
- Tag set to “activerecord, enhancement, named_scope”
You flagged this item as spam.
Henrik Nyh July 17th, 2008 @ 11:00 PM
Related issue:
If you have a named scope with a :select, doing
Foo.that_named_scope.find(:all, :select => 'something')
will use the select from the named scope, not the one in the find.
Incite August 22nd, 2008 @ 09:58 AM
- no changes were found...
- allow_ordering_with_named_scope.patch 808 Bytes
You flagged this item as spam.
Incite August 22nd, 2008 @ 09:58 AM
I've come across similar.. in my case I didn't get a comma separated order statement, just the first order param passed in the scope chain.

The attached patch seems to address this issue. It specifically deals with :order alongside :includes, :conditions, and merges them with the last ordering being most important.
Pratik August 22nd, 2008 @ 01:10 PM
- State changed from “new” to “incomplete”
Patch is missing tests.

Thanks.
You flagged this item as spam.
Incite August 25th, 2008 @ 02:12 AM
Here are some tests.
- allow_ordering_with_named_scope_with_tests.patch 4.9 KB
You flagged this item as spam.
Henrik Nyh August 25th, 2008 @ 09:23 AM
David, thanks for doing this.

Confirmed that the tests pass.

It does not, however, seem to handle the case where the association is defined with an order, like "has_many :bars, :order => 'one'" above.

Attaching a failing test to this effect that goes on top of David's latest patch.

Not saying the patch should not be accepted without this -- better to fix a little than nothing at all -- but if it's easy to get this, too, that would rock.
- test_ordered_association_and_scope_order.diff 1.2 KB
You flagged this item as spam.
Incite August 25th, 2008 @ 04:07 PM
Henrik,

You can get the test working by ordering by 'comments.body', and changing the add_order! function in active_record/base.rb, as attached. It assumes you'll want to apply a named scope on an association..

This breaks using a finder on a named scope, though.
- prevent_association_ordering_from_clobbering_scoped.diff 696 Bytes
You flagged this item as spam.
Henrik Nyh August 25th, 2008 @ 04:23 PM
David: Thanks! "breaks using a finder on a named scope" only in the sense that the :order from named scopes will always beat the order from associations/finds, right?

So basically one has to choose whether to let associations/finds or named scopes get priority?

Seems like one could modify the calls to add_order! (or check the caller, but that's nasty), so we can tell whether the non-scope order is from an association or a find.

Basically, the add_order! calls from associations.rb could be passed a forth parameter like "association = true", and then association orders could be lower prio than scope orders, but find orders would be higher prio.

Does this make sense? Is it too complicated? I personally would much prefer moderate complication if it means ActiveRecord behaves more intuitively.

I'd be happy to try this out and submit a patch, just wanted to check if it makes sense to someone more familiar with the codebase.
You flagged this item as spam.
Incite August 25th, 2008 @ 10:30 PM
Yes, the named scope ordering beats associations/finds with my changes.

One problem being that in our tests, it's only base.rb#construct_finder_sql that calls add_order! and you don't know much context from there.

I think you're probably right about the priority being dynamic finder < named scope < association ordering.
You flagged this item as spam.
Wildgoose February 6th, 2009 @ 04:13 PM
I just bumped into this in rails 2.2.2 - sad to see it was never merged

Personally I think the reason for the complication suggested above is because the order should in fact simply be from left to right. So if you have

obj.sort_a.sort_b.sort_c

then you get "order a, b, c"

This meets all the requirements above. I don't obviously see that it's consistent that scopes should win out over an earlier specified sort order (although I do agree there are occasions that would be nice). If it's desired to force sort overrides then I think this needs an extra syntax, the general rule of scopes, etc, is that they are cumulative from left to right and so should the sort options to be consistent

Hope to see this in 2.4/3?
You flagged this item as spam.
Andrew Vit March 15th, 2009 @ 06:12 PM
The current behaviour also prevents us from explicitly setting the order on top of a named scope:
```
obj.sort_a :order => "b,c"
#=> "order a"
```
I would expect that either these be chained for "order a,b,c" or else have the explicit order clobber the one in the named scope.
You flagged this item as spam.
Andrew Vit March 15th, 2009 @ 06:50 PM
It seems this also applies to default_scope, which is potentially very confusing because it hides the source of the problem.
Ryan Bigg June 21st, 2010 @ 02:03 AM
Can anybody still confirm this is an issue that needs patching? It's a pretty old ticket.
rails February 26th, 2011 @ 12:00 AM
- State changed from “incomplete” to “open”
- Importance changed from “” to “”
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
rails February 26th, 2011 @ 12:00 AM
- State changed from “open” to “stale”