This project is archived and is in readonly mode.
Make named_scopes remember the current scope when defined
Reported by Diego Algorta | February 13th, 2009 @ 07:40 AM | in 2.x
Given the following code:
class Post < ActiveRecord::Base
belongs_to :topic
belongs_to :author
named_scope :published, :conditions => {:published => true}
named_scope :by_author, lambda {|a| {:conditions => {:author_id => a.id}}}
named_scope :ranked, :order => "posts.rank DESC"
named_scope :limit, lambda {|limit| {:limit => limit}}
def self.top_by(an_author)
published.by_author(an_author).ranked.limit(10)
end
end
class Topic < ActiveRecord::Base
has_many :posts
end
class Author < ActiveRecord::Base
has_many :posts
end
Without this patch, @topic.posts.top_by(@author)
generates the exact same query than
Post.top_by(@author), totally ignoring the scope added
by the has_many :posts in Topic. That's
because that scope is being removed from the stacked scopes after
the top_by execution where the chained scopes where defined, but
before the chained scopes are really executing thus firing the
Scope#load_found call.
With this patch, the current scope is saved in the
Scope instance at instantiation time, so it can be
re-applied when executed.
Comments and changes to this ticket
-

Diego Algorta February 13th, 2009 @ 07:42 AM
Forgot to say that, as of now, this patch applies cleanly in both 2-2-stable and master branches.
-

-

-

Ryan Berdeen February 17th, 2009 @ 07:04 AM
+1
This is a dangerous bug I just spent hours tracking down myself. Scopes should never disappear without warning.
-

Diego Algorta February 17th, 2009 @ 05:08 PM
- Tag changed from 2.2-stable, activerecord, association_proxy, master, named_scope, patch to 2.2-stable, activerecord, association_proxy, master, named_scope, patch, verified
Adding the verified tag now that 3 people have voted positively.
-

Ryan Berdeen February 17th, 2009 @ 06:12 PM
I believe this was previously reported as #1770, which doesn't have a patch.
-

Ryan Berdeen February 18th, 2009 @ 01:36 AM
Whoops, looks like #1770 has somewhat similar symptoms, but its cause is probably in association_collection or elsewhere, not named_scope. This patch doesn't fix it.
-

Matt Jankowski February 20th, 2009 @ 01:11 PM
Does this address this issue too? - http://rails.lighthouseapp.com:8...
-

Diego Algorta February 20th, 2009 @ 01:45 PM
Yes. This patch should fix #1677 too. It's the same problem.
-

Repository February 25th, 2009 @ 05:13 PM
- State changed from new to resolved
(from [a9aa18fdcdf3146ccbdecff71e52015f26a0f0b7]) Fixed bug that makes named_scopes forgot current scope
Signed-off-by: rick technoweenie@gmail.com [#1960 #1677 state:resolved] http://github.com/rails/rails/co...
-

Rick February 25th, 2009 @ 05:17 PM
- State changed from resolved to open
Hmm I get this failure on master:
1) Failure: test_named_scope(DefaultScopingTest) [./test/cases/method_scoping_test.rb:602:in `test_named_scope' ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:57:in `__send__' ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:57:in `run']: <[9000, 150000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 80000]> expected but was <[150000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 100000, 80000, 9000]>. -

Rick February 25th, 2009 @ 05:29 PM
- State changed from open to resolved
Ah, the test was bad:
def test_named_scope - expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary } + expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.salary } assert_equal expected, received endIt was leaving out the DeveloperOrderedBySalary default scope
:order => 'salary DESC'. So, this patch actually fixed that failing test, and probably a bunch of other issues mixing default and named scopes. -

Alexander Podgorbunsky March 26th, 2009 @ 01:20 PM
Unfortunately, the test was failing right and then broken -- see #2346
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>
People watching this ticket
Attachments
Referenced by
-
1920
named_scope fails when accessed through a class method over an association
Resolved by #1960
-
4634
Duplicate association conditions are generated when using a named_scope on an association
#1960 introduced this behavior.
-
1770
Backtracking association proxy chained methods
Maybe you'd like to test if the patch in #1960 fixes
this...
-
1770
Backtracking association proxy chained methods
In fact, the same problem occurs with named scopes and cl...
-
1677
Association proxies using class methods with named scopes
I think my patch on #1960 fixes this.
-
1677
Association proxies using class methods with named scopes
Signed-off-by: rick technoweenie@gmail.com
[#1960 #1677 s...
-
1960
Make named_scopes remember the current scope when defined
Signed-off-by: rick technoweenie@gmail.com
[#1960 #1677 s...
-
2072
[PATCH] dynamic scope should reflect chained scopes
may be fixed by #1960.
-
1920
named_scope fails when accessed through a class method over an association
Ah actually this bug is already resolved in couple other
...
-
2346
named_scope doesn't override default_scope's :order key
This behavior appeared as a result of resolving #1960
-
2346
named_scope doesn't override default_scope's :order key
Here's the the patch to fix it Patch also fixes test that...
-
2257
default_scope with named_scope or scoped merge broken on 2.3
Manfred's commit has actually nothing to do with this bug...
-
2349
yet another default_scope bug
Not quite sure, but that seems to be a consequence of #19...
-
2923
named_scope through a has_many (or has_many through) generates redundant conditions
The reason this is happening is in part due to #1960 and
...