This project is archived and is in readonly mode.

#2346 open
Alexander Podgorbunsky

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

Comments and changes to this ticket

  • Max Lapshin
  • Yaroslav Markin

    Yaroslav Markin March 26th, 2009 @ 04:55 PM

    +1 since 2.3 named_scope merging is a problem for all folks using default_scope

  • Jérôme
  • Anton Ageev
  • Samsonov Ivan
  • igonef
  • Akira Matsuda
  • Eloy Duran

    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.

  • Yaroslav Markin

    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

    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

    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.

  • Yaroslav Markin

    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

    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

    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...

  • Jon

    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

  • jack dempsey (jackdempsey)

    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".

  • Jon

    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.

  • Jamie Hill

    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.

  • Jeroen

    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

    Santiago Pastorino June 13th, 2010 @ 02:08 AM

    • State changed from “committed” to “open”
  • Jan Xie

    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

    Santiago Pastorino June 23rd, 2010 @ 04:03 PM

    Jan add the patch for 2-3-stable here

  • Neeraj Singh

    Neeraj Singh July 29th, 2010 @ 09:21 PM

    • Milestone changed from 2.x to 2.3.9
    • Importance changed from “” to “”
  • Santiago Pastorino

    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.

  • Lars Kuhnt

    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

    Jeremy Kemper August 30th, 2010 @ 02:28 AM

    • Milestone changed from 2.3.9 to 2.3.10
  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 10:12 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Ryan Bigg

    Ryan Bigg October 19th, 2010 @ 08:23 AM

    Automatic cleanup of spam.

  • Ryan Bigg

    Ryan Bigg October 21st, 2010 @ 03:37 AM

    Automatic cleanup of spam.

  • Steve Purcell

    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

    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

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages