#6011 except(:order).order(...) is not working in scopes - 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.

#6011 open

except(:order).order(...) is not working in scopes

Reported by Vitalii Khustochka | November 19th, 2010 @ 03:30 AM | in 3.1

Method reorder that existed before was removed in master and deprecated in 3.0.3 (https://github.com/rails/rails/commit/e0b76d6151821527f16b3f163abde.... Instead must be used except(:order).order(...)

While this works in regular relations it is not working in scopes (does not override the default scope).

E.g. if class DeveloperOrderedBySalary has default_scope :order => 'salary DESC' and named scope

scope :reordered_by_name, except(:order).order('name DESC')

I expect DeveloperOrderedBySalary.reordered_by_name.all to return developers reordered by name while it does not. The test I once created for this was removed. Instead there is a test named test_named_scope_appends_default but I think this name is incorrect as soon as it really tests that ordering defined in named scope is appended to default scope's ordering (i.e. :order => 'salary DESC, name DESC' is expected).

Unfortunately I have no idea how to fix this but I'm attaching a patch with test which I consider appropriate (now failing).

test-reorder-scope.diff 2.2 KB

Comments and changes to this ticket

You flagged this item as spam.
Vitalii Khustochka November 19th, 2010 @ 03:33 AM
Typo in the description: "Instead there is a test named test_named_scope_overwrites_default but I think this name is incorrect ..."

test_named_scope_appends_default is the new name I gave it in the patch
You flagged this item as spam.
Vitalii Khustochka November 19th, 2010 @ 05:11 AM
Looks like this happens because reorder_flag was removed. Named scope defined with except(:order).order(xxx) now has no difference with the one defined with order(xxx). So when merging order_value they are merged like they were defined by regular order i.e. are being appended to default_scope's order instead of overwriting them.

When a chain of methods is run on the relation except works correctly because it effectively removes the current order_value of this very relation, but in the scope it does not.
Santiago Pastorino November 19th, 2010 @ 04:10 PM
- Milestone set to “3.1”
- Importance changed from “” to “Low”
You flagged this item as spam.
2kan December 23rd, 2010 @ 01:43 AM
It's not because reorder_flag was removed but yes it is because of how except and merge are working now. I gonna attach a patch (against edge) tomorrow, I want to find the best way to fix it.
rails March 24th, 2011 @ 12:00 AM
- State changed from “new” to “open”
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 March 24th, 2011 @ 12:00 AM
- State changed from “open” to “stale”
You flagged this item as spam.
2kan March 24th, 2011 @ 11:16 AM
- State changed from “stale” to “open”
[state:open]
Santiago Pastorino March 24th, 2011 @ 03:03 PM
2kan have you tried it with the latest master code?. Is still an issue?
You flagged this item as spam.
2kan March 24th, 2011 @ 06:43 PM
Santiago, yes it is. (Just checked). Somehow (not sure) it is because of: https://rails.lighthouseapp.com/projects/8994/tickets/6290-patch-ex...

And because of how the default_scope applies (I think I've figured out why I doesn't work).