Ensure nested with_scope merges conditions inside out [#2193 state:resolved]
Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
This commit is contained in:
parent
8272630ce8
commit
c3aa2bcdcf
@ -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
|
||||
@ -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|
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
|
@ -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?
|
||||
|
||||
post2 = Topic.approved.rejected.new
|
||||
assert ! post2.approved?
|
||||
post = Topic.rejected.approved.new
|
||||
assert post.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
|
||||
|
Loading…
Reference in New Issue
Block a user