Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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.
  • Loading branch information...
commit 8606a7fbe9367e9ae37ad058dd07f0dd38daf015 1 parent f029fb0
Jon Leighton jonleighton authored
4 activerecord/lib/active_record/scoping.rb
View
@@ -9,11 +9,11 @@ module Scoping
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
9 activerecord/lib/active_record/scoping/named.rb
View
@@ -160,13 +160,8 @@ def scope(name, body, &block)
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
2  activerecord/test/cases/scoping/named_scoping_test.rb
View
@@ -461,7 +461,7 @@ def test_eager_default_scope_relations_are_deprecated
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
1  activerecord/test/models/comment.rb
View
@@ -29,7 +29,6 @@ def self.all_as_method
end
class SpecialComment < Comment
- scope :crazy_all, -> { where(body: 'go crazy').created }
end
class SubSpecialComment < SpecialComment

1 comment on commit 8606a7f

Aaron Patterson
Owner

This change caused bug #13466

Please sign in to comment.
Something went wrong with that request. Please try again.