Skip to content

Commit

Permalink
Updated rails#14544 to work with activerecord-deprecated_finders
Browse files Browse the repository at this point in the history
Used older style of lambda definitions within a test. Also explicitly check for
the case when `activerecord-deprecated_finders` is active in the context of scopes
and handles this case specially. This handling relies on and is meant to be used
in unison with rails/activerecord-deprecated_finders#23.

Fixes Issue rails#13466. See rails#14544 for complete description.
  • Loading branch information
jefflai2 committed Jun 10, 2014
1 parent 268304f commit b5aafd1
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 5 deletions.
11 changes: 11 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2061,6 +2061,17 @@

*Yves Senn*

* Changed scoped blocks to be executed with `instance_eval`

Named scopes (i.e. using STI) were previously cached according to
base class so scoped queries made by classes with a common ancestor would
leak. Changed the way scope blocks were called so inheritance rules are
followed during the call and scopes are cached correctly.

Fixes #13466.

*Jefferson Lai*

* Do not create useless database transaction when building `has_one` association.

Example:
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/scoping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ module Scoping

module ClassMethods
def current_scope #:nodoc:
ScopeRegistry.value_for(:current_scope, base_class.to_s)
ScopeRegistry.value_for(:current_scope, self.to_s)
end

def current_scope=(scope) #:nodoc:
ScopeRegistry.set_value_for(:current_scope, base_class.to_s, scope)
ScopeRegistry.set_value_for(:current_scope, self.to_s, scope)
end
end

Expand Down
10 changes: 8 additions & 2 deletions activerecord/lib/active_record/scoping/named.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,19 @@ def scope(name, body, &block)
end

singleton_class.send(:define_method, name) do |*args|
if body.respond_to?(:call)
if body.respond_to?(:exec_or_call)
# Need special handling when body is a ScopeWrapper from activerecord-deprecated_finders.
scope = all.scoping { body.exec_or_call(self, *args) }
elsif body.respond_to?(:to_proc)
scope = all.scoping { instance_exec(*args, &body) }
elsif body.respond_to?(:call)
scope = all.scoping { body.call(*args) }
scope = scope.extending(extension) if extension
else
scope = body
end

scope = scope.extending(extension) if extension && scope != body

scope || all
end
end
Expand Down
7 changes: 6 additions & 1 deletion activerecord/test/cases/scoping/named_scoping_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
require 'models/post'
require 'models/topic'
require 'models/comment'
require 'models/rating'
require 'models/reply'
require 'models/author'
require 'models/developer'

class NamedScopingTest < ActiveRecord::TestCase
fixtures :posts, :authors, :topics, :comments, :author_addresses
fixtures :posts, :authors, :topics, :comments, :author_addresses, :ratings

def test_implements_enumerable
assert !Topic.all.empty?
Expand Down Expand Up @@ -120,6 +121,10 @@ def test_scope_with_STI
assert_equal 1,SpecialPost.containing_the_letter_a.count
end

def test_scope_subquery_with_STI
assert_nothing_raised { VerySpecialComment.special_parent(SpecialRating.first).count }
end

def test_has_many_through_associations_have_access_to_scopes
assert_not_equal Comment.containing_the_letter_e, authors(:david).comments
assert !Comment.containing_the_letter_e.empty?
Expand Down
10 changes: 10 additions & 0 deletions activerecord/test/fixtures/ratings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,23 @@ normal_comment_rating:
id: 1
comment_id: 8
value: 1
type: Rating

special_comment_rating:
id: 2
comment_id: 6
value: 1
type: Rating

sub_special_comment_rating:
id: 3
comment_id: 12
value: 1
type: Rating

special_rating:
id: 4
comment_id: 10
value: 1
type: SpecialRating
special_comment_id: 3
1 change: 1 addition & 0 deletions activerecord/test/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class SubSpecialComment < SpecialComment
end

class VerySpecialComment < Comment
scope :special_parent, lambda { |special_rating| where parent_id: special_rating.special_comment.id }
end

class CommentThatAutomaticallyAltersPostBody < Comment
Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/models/rating.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@ class Rating < ActiveRecord::Base
belongs_to :comment
has_many :taggings, :as => :taggable
end

class SpecialRating < Rating
belongs_to :special_comment
end
2 changes: 2 additions & 0 deletions activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,9 @@ def create_table(*args, &block)

create_table :ratings, :force => true do |t|
t.integer :comment_id
t.integer :special_comment_id
t.integer :value
t.string :type
end

create_table :readers, :force => true do |t|
Expand Down

0 comments on commit b5aafd1

Please sign in to comment.