From 8606a7fbe9367e9ae37ad058dd07f0dd38daf015 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 5 Apr 2013 12:46:56 +0100 Subject: [PATCH] Fix scope chaining + STI See #9869 and #9929. The problem arises from the following example: class Project < ActiveRecord::Base scope :completed, -> { where completed: true } end class MajorProject < Project end When calling: MajorProject.where(tasks_count: 10).completed This expands to: MajorProject.where(tasks_count: 10).scoping { MajorProject.completed } However the lambda for the `completed` scope is defined on Project. This means that when it is called, `self` is Project rather than MajorProject. So it expands to: MajorProject.where(tasks_count: 10).scoping { Project.where(completed: true) } Since the scoping was applied on MajorProject, and not Project, this fails to apply the tasks_count condition. The solution is to make scoping apply across STI classes. I am slightly concerned about the possible side-effects of this, but no tests fail and it seems ok. I guess we'll see. --- activerecord/lib/active_record/scoping.rb | 4 ++-- activerecord/lib/active_record/scoping/named.rb | 9 ++------- activerecord/test/cases/scoping/named_scoping_test.rb | 2 +- activerecord/test/models/comment.rb | 1 - 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb index 9746b1c3c2..886182f534 100644 --- a/activerecord/lib/active_record/scoping.rb +++ b/activerecord/lib/active_record/scoping.rb @@ -9,11 +9,11 @@ module ActiveRecord module ClassMethods def current_scope #:nodoc: - Thread.current["#{self}_current_scope"] + Thread.current["#{base_class}_current_scope"] end def current_scope=(scope) #:nodoc: - Thread.current["#{self}_current_scope"] = scope + Thread.current["#{base_class}_current_scope"] = scope end end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 12317601b6..da73bead32 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -160,13 +160,8 @@ module ActiveRecord singleton_class.send(:define_method, name) do |*args| if body.respond_to?(:call) - scope = extension ? body.call(*args).extending(extension) : body.call(*args) - - if scope - default_scoped = scope.default_scoped - scope = relation.merge(scope) - scope.default_scoped = default_scoped - end + scope = all.scoping { body.call(*args) } + scope = scope.extending(extension) if extension else scope = body end diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 1ac68746de..afe32af1d1 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -461,7 +461,7 @@ class NamedScopingTest < ActiveRecord::TestCase end def test_subclass_merges_scopes_properly - assert_equal 1, SpecialComment.crazy_all.count + assert_equal 1, SpecialComment.where(body: 'go crazy').created.count end end diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 9d594bd7d6..ede5fbd0c6 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -29,7 +29,6 @@ class Comment < ActiveRecord::Base end class SpecialComment < Comment - scope :crazy_all, -> { where(body: 'go crazy').created } end class SubSpecialComment < SpecialComment