This project is archived and is in readonly mode.
Make named_scopes remember the current scope when defined
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.
-
Diego AlgortaForgot to say that, as of now, this patch applies cleanly in both 2-2-stable and master branches.
-
-
-
Ryan Berdeen+1
This is a dangerous bug I just spent hours tracking down myself. Scopes should never disappear without warning.
-
Diego Algorta
- 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
I believe this was previously reported as #1770, which doesn't have a patch.
-
Ryan Berdeen
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 JankowskiDoes this address this issue too? - http://rails.lighthouseapp.com:8...
-
Diego Algorta
Yes. This patch should fix #1677 too. It's the same problem.
-
Repository
- 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- 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
- 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
Unfortunately, the test was failing right and then broken -- see #2346