#2346 named_scope doesn't override default_scope's :order key - Ruby on Rails

This project is archived and is in readonly mode.

#2346 open

named_scope doesn't override default_scope's :order key

Reported by Alexander Podgorbunsky | March 26th, 2009 @ 01:04 PM | in 2.3.10

There's a problem of not overriding deafult_scope's keys that don't merge (:order for instance) by same keys of named_scopes


class Post
  default_scope :order => 'posts.created_at asc'
  named_scope :recent, :order => 'posts.created_at desc'
end

Post.recent # sorts posts in ascending order

This behavior appeared as a result of resolving #1960

Here's the the patch to fix it Patch also fixes test that was broken too (see comments on #1960)

Added code checks if scoping that was current on creation of Scope object is in scoped_methods stack on execution of scope, and if so - doesn't apply it second time (as it's been already implicitly accounted for during merging named scope's scoping with current_scoped_methods). This eliminates a problem of overriding keys of named scope by default scope

0001-fixes-the-problem-of-not-overriding-order-key-in-de.patch 2.3 KB

Comments and changes to this ticket

You flagged this item as spam.
Max Lapshin March 26th, 2009 @ 04:52 PM
+1 Fix this, please.
You flagged this item as spam.
Yaroslav Markin March 26th, 2009 @ 04:55 PM
+1 since 2.3 named_scope merging is a problem for all folks using default_scope
You flagged this item as spam.
Jérôme March 26th, 2009 @ 06:37 PM
+1
You flagged this item as spam.
Anton Ageev March 26th, 2009 @ 06:40 PM
+1
You flagged this item as spam.
Samsonov Ivan March 26th, 2009 @ 09:00 PM
+1
You flagged this item as spam.
igonef March 27th, 2009 @ 06:31 AM
+1
You flagged this item as spam.
Akira Matsuda March 27th, 2009 @ 11:20 AM
+1
Eloy Duran March 27th, 2009 @ 11:31 AM
I'm gonna be the party pooper here, but simply saying "+1" does nothing good for the ticket. Please see the contribution guide linked in the help text =>.

The part about commenting:

Reviewing Changes Are you happy with the tests, can you follow what they're testing, is there anything missing Does the documentation still seem right to you Do you like the implementation, can you think of a nicer or faster way to implement a part of their change Once you're happy it's a good change, comment on the lighthouse ticket indicating your approval. Your comment should indicate that you like the change and what you like about it. Something like: I like the way you've restructured that code in generate_finder_sql, much nicer. The tests look good too. If your comment simply says +1, then odds are other reviewers aren't going to take it too seriously. Show that you took the time to review the patch. Once three people have approved it, add the verified tag. This will bring it to the attention of a committer who'll then review the changes looking for the same kinds of things.
You flagged this item as spam.
Yaroslav Markin March 27th, 2009 @ 11:38 AM
@Eloy AFAIR old Rails Trac used to have something like "+1 only if you've tested it yourself", and I think this is the case here — tests pass, works as expected, "works for me".

When someone comes with "something's wrong with this", well, different story.
Eloy Duran March 27th, 2009 @ 11:47 AM
@Eloy AFAIR old Rails Trac used to have something like "+1 only if you've tested it yourself", and I think this is the case here — tests pass, works as expected, "works for me".

Well I don't know about the past, I'm just reading what it currently says:

Your comment should indicate that you like the change and what you like about it. Something like: I like the way you've restructured that code in generate_finder_sql, much nicer. The tests look good too. If your comment simply says +1, then odds are other reviewers aren't going to take it too seriously. Show that you took the time to review the patch.

So in both cases (good/problem) only a +/- 1 won't be taken too seriously. Which I can understand as it's basically not constructive criticism. Note that I'm also not claiming it's deconstructive criticism, it's neither :)
Pratik March 27th, 2009 @ 11:54 AM
I agree with Eloy. Being the asignee here, I wouldn't take seriously the massive influx of +1s in a very short time.

That is not to say there is anything wrong with the patch itself. Patch looks ok and I'll apply the next time I am applying pending patches.
You flagged this item as spam.
Yaroslav Markin April 15th, 2009 @ 10:04 PM
- Tag changed from “2.3, activerecord, bug, default_scope, named_scope, patch” to “2.3, activerecord, bug, default_scope, named_scope, patch, verified”
Repository May 1st, 2009 @ 10:45 PM
- State changed from “new” to “committed”
(from [db0bfe4ede3cdfc2e4ccdb2a89525a914e6d0913]) Default scope :order should be overridden by named scopes.

[#2346 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...
Repository May 1st, 2009 @ 10:45 PM
(from [628b4ad679b1971427a20461e8c2332d492e4655]) Default scope :order should be overridden by named scopes.

[#2346 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...
You flagged this item as spam.
Jon June 25th, 2009 @ 09:49 PM
I'm not so sure this bug is fixed. Product.my_named_scope works now, but I'm losing the order key on something like @store.products.my_named_scope. So a scoped query isn't respecting the :order key of a named_scope when a default_scope is present or an order clause of a parent model is present.

I made a gist of some of my app's output here: http://gist.github.com/136140

Probably related to ticket #2253
You flagged this item as spam.
jack dempsey (jackdempsey) July 29th, 2009 @ 04:16 PM
Not working for me as well. Following a couple of the links, starting with Jon's to ticket #2253, lead to what looks like a good solution. Will try the patch later if I can, but for now wanted to give another "ain't working here".
You flagged this item as spam.
Jon July 29th, 2009 @ 04:19 PM
Thanks Jack. In my experimentation, some of the other keys weren't working either. Primarily :select. This is a pretty dangerous bug.
You flagged this item as spam.
Jamie Hill December 5th, 2009 @ 02:25 PM
- Tag changed from “2.3, activerecord, bug, default_scope, named_scope, patch, verified” to “2.3, 2.3.5, activerecord, bug, default_scope, named_scope, patch, verified”
Sorry to resurrect an old ticket but this is definitely still an issue, default_scope becomes pretty much unusable without being able to override the :order etc. in named scopes.
You flagged this item as spam.
Jeroen June 10th, 2010 @ 03:26 PM
As far as I can see this ticket is still open.

I just ran against this problem and it seems I'm still encountering it (2.3.5 and 2.3.8).

Am I overlooking another ticket on this?
Santiago Pastorino June 13th, 2010 @ 02:08 AM
- State changed from “committed” to “open”
You flagged this item as spam.
Jan Xie June 23rd, 2010 @ 05:19 AM
- Tag changed from “2.3, 2.3.5, activerecord, bug, default_scope, named_scope, patch, verified” to “rails 3.0.0.beta4, 2.3, 2.3.5, activerecord, bug, default_scope, named_scope, patch, verified”
I just met the same problem today, I'm using edge rails (version string = Rails 3.0.0.beta4). Should I create a different ticket?
Santiago Pastorino June 23rd, 2010 @ 04:03 PM
Jan add the patch for 2-3-stable here
Neeraj Singh July 29th, 2010 @ 09:21 PM
- Milestone changed from “2.x” to “2.3.9”
- Importance changed from “” to “”
Santiago Pastorino July 30th, 2010 @ 04:20 AM
Can someone check if this is still an issue on 2-3-stable and provide a backport please.
Thanks.
You flagged this item as spam.
Lars Kuhnt August 3rd, 2010 @ 10:13 AM
I am still having this Problem on 2-3-stable with polymorphic has_many associations.

Here is the gist
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from “2.3.9” to “2.3.10”
[bulk edit]
Ryan Bigg October 9th, 2010 @ 10:12 PM
- Tag cleared.
Automatic cleanup of spam.
Ryan Bigg October 19th, 2010 @ 08:23 AM
Automatic cleanup of spam.
Ryan Bigg October 21st, 2010 @ 03:37 AM
Automatic cleanup of spam.
You flagged this item as spam.
Steve Purcell November 22nd, 2010 @ 02:50 PM
As of the 3.0.3 release, this bug is now present in the latest Rails. I can't comment on the proposed patch, but I will say that I haven't found a tidy way to work around this problem.
Neeraj Singh November 22nd, 2010 @ 10:37 PM
@Steve the behavior you are seeing in Rails 3.0.3 is not a bug.
```
class Car < ActiveRecord::Base
  default_scope :order => 'id desc'
  scope :recent, :order => 'name desc'
end

Car.recent #=>  SELECT "cars".* FROM "cars" ORDER BY id desc, name desc
```
In the above case because of default_scope order has 'id desc' at the first. That's okay.

What you might argue is a bug is that if you try User.unscoped.recent then alos you see 'id desc'. I have documented that. In this case you need to use block form of unscoped.

User.unscoped { recent } #=> SELECT "cars".* FROM "cars" ORDER BY name desc