Skip to content

Commit 8606a7f

Browse files
committed
Fix scope chaining + STI
See rails#9869 and rails#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.
1 parent f029fb0 commit 8606a7f

File tree

4 files changed

+5
-11
lines changed

4 files changed

+5
-11
lines changed

activerecord/lib/active_record/scoping.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ module Scoping
99

1010
module ClassMethods
1111
def current_scope #:nodoc:
12-
Thread.current["#{self}_current_scope"]
12+
Thread.current["#{base_class}_current_scope"]
1313
end
1414

1515
def current_scope=(scope) #:nodoc:
16-
Thread.current["#{self}_current_scope"] = scope
16+
Thread.current["#{base_class}_current_scope"] = scope
1717
end
1818
end
1919

activerecord/lib/active_record/scoping/named.rb

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,8 @@ def scope(name, body, &block)
160160

161161
singleton_class.send(:define_method, name) do |*args|
162162
if body.respond_to?(:call)
163-
scope = extension ? body.call(*args).extending(extension) : body.call(*args)
164-
165-
if scope
166-
default_scoped = scope.default_scoped
167-
scope = relation.merge(scope)
168-
scope.default_scoped = default_scoped
169-
end
163+
scope = all.scoping { body.call(*args) }
164+
scope = scope.extending(extension) if extension
170165
else
171166
scope = body
172167
end

activerecord/test/cases/scoping/named_scoping_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ def test_eager_default_scope_relations_are_deprecated
461461
end
462462

463463
def test_subclass_merges_scopes_properly
464-
assert_equal 1, SpecialComment.crazy_all.count
464+
assert_equal 1, SpecialComment.where(body: 'go crazy').created.count
465465
end
466466

467467
end

activerecord/test/models/comment.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ def self.all_as_method
2929
end
3030

3131
class SpecialComment < Comment
32-
scope :crazy_all, -> { where(body: 'go crazy').created }
3332
end
3433

3534
class SubSpecialComment < SpecialComment

0 commit comments

Comments
 (0)