Permalink
Browse files

Ensure nested with_scope merges conditions inside out [#2193 state:re…

…solved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information...
1 parent 8272630 commit c3aa2bcdcffb42f578b0e89fe08e1c4e234ccf3b @Manfred Manfred committed with lifo Mar 10, 2009
View
19 activerecord/lib/active_record/base.rb
@@ -416,7 +416,7 @@ def self.reset_subclasses #:nodoc:
end
@@subclasses = {}
-
+
##
# :singleton-method:
# Contains the database configuration - as is typically stored in config/database.yml -
@@ -661,7 +661,6 @@ def find_by_sql(sql)
connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
end
-
# Returns true if a record exists in the table that matches the +id+ or
# conditions given, or false otherwise. The argument can take five forms:
#
@@ -1003,7 +1002,6 @@ def decrement_counter(counter_name, id)
update_counters(id, counter_name => -1)
end
-
# Attributes named in this macro are protected from mass-assignment,
# such as <tt>new(attributes)</tt>,
# <tt>update_attributes(attributes)</tt>, or
@@ -1104,7 +1102,6 @@ def serialized_attributes
read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {})
end
-
# Guesses the table name (in forced lower-case) based on the name of the class in the inheritance hierarchy descending
# directly from ActiveRecord::Base. So if the hierarchy looks like: Reply < Message < ActiveRecord::Base, then Message is used
# to guess the table name even when called on Reply. The rules used to do the guess are handled by the Inflector class
@@ -1417,7 +1414,6 @@ def inspect
end
end
-
def quote_value(value, column = nil) #:nodoc:
connection.quote(value,column)
end
@@ -1486,7 +1482,7 @@ def respond_to?(method_id, include_private = false)
elsif match = DynamicScopeMatch.match(method_id)
return true if all_attributes_exists?(match.attribute_names)
end
-
+
super
end
@@ -2014,7 +2010,6 @@ def expand_id_conditions(id_or_conditions)
end
end
-
# Defines an "attribute" method (like +inheritance_column+ or
# +table_name+). A new (class) method will be created with the
# given name. If a value is specified, the new method will
@@ -2111,7 +2106,7 @@ def with_scope(method_scoping = {}, action = :merge, &block)
end
# Merge scopings
- if action == :merge && current_scoped_methods
+ if [:merge, :reverse_merge].include?(action) && current_scoped_methods
method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)|
case hash[method]
when Hash
@@ -2133,7 +2128,11 @@ def with_scope(method_scoping = {}, action = :merge, &block)
end
end
else
- hash[method] = hash[method].merge(params)
+ if action == :reverse_merge
+ hash[method] = hash[method].merge(params)
+ else
+ hash[method] = params.merge(hash[method])
+ end
end
else
hash[method] = params
@@ -2143,7 +2142,6 @@ def with_scope(method_scoping = {}, action = :merge, &block)
end
self.scoped_methods << method_scoping
-
begin
yield
ensure
@@ -2749,7 +2747,6 @@ def attributes=(new_attributes, guard_protected_attributes = true)
assign_multiparameter_attributes(multi_parameter_attributes)
end
-
# Returns a hash of all the attributes with their names as keys and the values of the attributes as values.
def attributes
self.attribute_names.inject({}) do |attrs, name|
View
4 activerecord/lib/active_record/named_scope.rb
@@ -104,7 +104,7 @@ def named_scope(name, options = {}, &block)
end
end
end
-
+
class Scope
attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined
NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
@@ -175,7 +175,7 @@ def method_missing(method, *args, &block)
if scopes.include?(method)
scopes[method].call(self, *args)
else
- with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do
+ with_scope({:find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {}}, :reverse_merge) do
method = :new if method == :build
if current_scoped_methods_when_defined
with_scope current_scoped_methods_when_defined do
View
38 activerecord/test/cases/method_scoping_test.rb
@@ -262,6 +262,15 @@ def test_merge_options
end
end
+ def test_merge_inner_scope_has_priority
+ Developer.with_scope(:find => { :limit => 5 }) do
+ Developer.with_scope(:find => { :limit => 10 }) do
+ merged_option = Developer.instance_eval('current_scoped_methods')[:find]
+ assert_equal({ :limit => 10 }, merged_option)
+ end
+ end
+ end
+
def test_replace_options
Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do
@@ -400,6 +409,29 @@ def test_merged_scoped_find_combines_and_sanitizes_conditions
end
end
+ def test_nested_scoped_create
+ comment = nil
+ Comment.with_scope(:create => { :post_id => 1}) do
+ Comment.with_scope(:create => { :post_id => 2}) do
+ assert_equal({ :post_id => 2 }, Comment.send(:current_scoped_methods)[:create])
+ comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!"
+ end
+ end
+ assert_equal 2, comment.post_id
+ end
+
+ def test_nested_exclusive_scope_for_create
+ comment = nil
+ Comment.with_scope(:create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do
+ Comment.with_exclusive_scope(:create => { :post_id => 1 }) do
+ assert_equal({ :post_id => 1 }, Comment.send(:current_scoped_methods)[:create])
+ comment = Comment.create :body => "Hey guys"
+ end
+ end
+ assert_equal 1, comment.post_id
+ assert_equal 'Hey guys', comment.body
+ end
+
def test_merged_scoped_find_on_blank_conditions
[nil, " ", [], {}].each do |blank|
Developer.with_scope(:find => {:conditions => blank}) do
@@ -523,7 +555,6 @@ def test_nested_scope
end
end
-
class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase
fixtures :posts, :categories, :categories_posts
@@ -549,7 +580,6 @@ def test_nested_scope
end
end
-
class DefaultScopingTest < ActiveRecord::TestCase
fixtures :developers
@@ -577,7 +607,7 @@ def test_default_scoping_with_inheritance
# Scopes added on children should append to parent scope
expected_klass_scope = [{ :create => {}, :find => { :order => 'salary DESC' }}, { :create => {}, :find => {} }]
assert_equal expected_klass_scope, klass.send(:scoped_methods)
-
+
# Parent should still have the original scope
assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods)
end
@@ -620,7 +650,6 @@ def test_overwriting_default_scope
=begin
# We disabled the scoping for has_one and belongs_to as we can't think of a proper use case
-
class BelongsToScopingTest< ActiveRecord::TestCase
fixtures :comments, :posts
@@ -640,7 +669,6 @@ def test_forwarding_to_dynamic_finders
end
-
class HasOneScopingTest< ActiveRecord::TestCase
fixtures :comments, :posts
View
22 activerecord/test/cases/named_scope_test.rb
@@ -247,7 +247,7 @@ def test_should_create_with_bang_with_proxy_options
topic = Topic.approved.create!({})
assert topic.approved
end
-
+
def test_should_build_with_proxy_options_chained
topic = Topic.approved.by_lifo.build({})
assert topic.approved
@@ -287,15 +287,21 @@ def test_chaining_with_duplicate_joins
assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size
end
- def test_chanining_should_use_latest_conditions_when_creating
- post1 = Topic.rejected.approved.new
- assert post1.approved?
+ def test_chaining_should_use_latest_conditions_when_creating
+ post = Topic.rejected.new
+ assert !post.approved?
+
+ post = Topic.rejected.approved.new
+ assert post.approved?
- post2 = Topic.approved.rejected.new
- assert ! post2.approved?
+ post = Topic.approved.rejected.new
+ assert !post.approved?
+
+ post = Topic.approved.rejected.approved.new
+ assert post.approved?
end
- def test_chanining_should_use_latest_conditions_when_searching
+ def test_chaining_should_use_latest_conditions_when_searching
# Normal hash conditions
assert_equal Topic.all(:conditions => {:approved => true}), Topic.rejected.approved.all
assert_equal Topic.all(:conditions => {:approved => false}), Topic.approved.rejected.all
@@ -306,7 +312,7 @@ def test_chanining_should_use_latest_conditions_when_searching
# Nested hash conditions with different keys
assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq
end
-
+
def test_methods_invoked_within_scopes_should_respect_scope
assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id]
end

0 comments on commit c3aa2bc

Please sign in to comment.