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