Permalink
Browse files

Change relation merging to always append select, group and order values

  • Loading branch information...
1 parent 67a2b5e commit c07f0ae52ecf9a45116e1b6ab422e0af9f2c1ada @lifo lifo committed Aug 31, 2010
View
17 activerecord/lib/active_record/relation/spawn_methods.rb
@@ -6,8 +6,9 @@ def merge(r)
merged_relation = clone
return merged_relation unless r
- ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) - [:joins, :where]).each do |method|
+ Relation::ASSOCIATION_METHODS.each do |method|
value = r.send(:"#{method}_values")
+
unless value.empty?
if method == :includes
merged_relation = merged_relation.includes(value)
@@ -17,15 +18,18 @@ def merge(r)
end
end
+ (Relation::MULTI_VALUE_METHODS - [:joins, :where]).each do |method|
+ value = r.send(:"#{method}_values")
+ merged_relation.send(:"#{method}_values=", merged_relation.send(:"#{method}_values") + value) if value.present?
+ end
+
merged_relation = merged_relation.joins(r.joins_values)
merged_wheres = @where_values
r.where_values.each do |w|
if w.respond_to?(:operator) && w.operator == :==
- merged_wheres = merged_wheres.reject { |p|
- p.respond_to?(:operator) && p.operator == :== && p.operand1.name == w.operand1.name
- }
+ merged_wheres = merged_wheres.reject {|p| p.respond_to?(:operator) && p.operator == :== && p.operand1.name == w.operand1.name }
end
merged_wheres += [w]
@@ -34,9 +38,8 @@ def merge(r)
merged_relation.where_values = merged_wheres
Relation::SINGLE_VALUE_METHODS.reject {|m| m == :lock}.each do |method|
- unless (value = r.send(:"#{method}_value")).nil?
- merged_relation.send(:"#{method}_value=", value)
- end
+ value = r.send(:"#{method}_value")
+ merged_relation.send(:"#{method}_value=", value) unless value.nil?
end
merged_relation.lock_value = r.lock_value unless merged_relation.lock_value
View
26 activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -567,16 +567,6 @@ def test_dynamic_find_should_respect_association_order
assert_equal high_id_jamis, projects(:active_record).developers.find_by_name('Jamis')
end
- def test_dynamic_find_order_should_override_association_order
- # Developers are ordered 'name DESC, id DESC'
- low_id_jamis = developers(:jamis)
- middle_id_jamis = developers(:poor_jamis)
- high_id_jamis = projects(:active_record).developers.create(:name => 'Jamis')
-
- assert_equal low_id_jamis, projects(:active_record).developers.find(:first, :conditions => "name = 'Jamis'", :order => 'id')
- assert_equal low_id_jamis, projects(:active_record).developers.find_by_name('Jamis', :order => 'id')
- end
-
def test_dynamic_find_all_should_respect_association_order
# Developers are ordered 'name DESC, id DESC'
low_id_jamis = developers(:jamis)
@@ -587,14 +577,9 @@ def test_dynamic_find_all_should_respect_association_order
assert_equal [high_id_jamis, middle_id_jamis, low_id_jamis], projects(:active_record).developers.find_all_by_name('Jamis')
end
- def test_dynamic_find_all_order_should_override_association_order
- # Developers are ordered 'name DESC, id DESC'
- low_id_jamis = developers(:jamis)
- middle_id_jamis = developers(:poor_jamis)
- high_id_jamis = projects(:active_record).developers.create(:name => 'Jamis')
-
- assert_equal [low_id_jamis, middle_id_jamis, high_id_jamis], projects(:active_record).developers.find(:all, :conditions => "name = 'Jamis'", :order => 'id')
- assert_equal [low_id_jamis, middle_id_jamis, high_id_jamis], projects(:active_record).developers.find_all_by_name('Jamis', :order => 'id')
+ def test_find_should_append_to_association_order
+ ordered_developers = projects(:active_record).developers.order('projects.id')
+ assert_equal ['developers.name desc, developers.id desc', 'projects.id'], ordered_developers.order_values
end
def test_dynamic_find_all_should_respect_association_limit
@@ -625,11 +610,10 @@ def test_new_with_values_in_collection
end
def test_find_in_association_with_options
- developers = projects(:active_record).developers.find(:all)
+ developers = projects(:active_record).developers.all
assert_equal 3, developers.size
- assert_equal developers(:poor_jamis), projects(:active_record).developers.find(:first, :conditions => "salary < 10000")
- assert_equal developers(:jamis), projects(:active_record).developers.find(:first, :order => "salary DESC")
+ assert_equal developers(:poor_jamis), projects(:active_record).developers.where("salary < 10000").first
end
def test_replace_with_less
View
25 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -147,6 +147,11 @@ def test_find_many_with_merged_options
assert_equal 2, companies(:first_firm).limited_clients.find(:all, :limit => nil).size
end
+ def test_find_should_append_to_association_order
+ ordered_clients = companies(:first_firm).clients_sorted_desc.order('companies.id')
+ assert_equal ['id DESC', 'companies.id'], ordered_clients.order_values
+ end
+
def test_dynamic_find_last_without_specified_order
assert_equal companies(:second_client), companies(:first_firm).unsorted_clients.find_last_by_type('Client')
end
@@ -156,21 +161,11 @@ def test_dynamic_find_should_respect_association_order
assert_equal companies(:second_client), companies(:first_firm).clients_sorted_desc.find_by_type('Client')
end
- def test_dynamic_find_order_should_override_association_order
- assert_equal companies(:first_client), companies(:first_firm).clients_sorted_desc.find(:first, :conditions => "type = 'Client'", :order => 'id')
- assert_equal companies(:first_client), companies(:first_firm).clients_sorted_desc.find_by_type('Client', :order => 'id')
- end
-
def test_dynamic_find_all_should_respect_association_order
assert_equal [companies(:second_client), companies(:first_client)], companies(:first_firm).clients_sorted_desc.find(:all, :conditions => "type = 'Client'")
assert_equal [companies(:second_client), companies(:first_client)], companies(:first_firm).clients_sorted_desc.find_all_by_type('Client')
end
- def test_dynamic_find_all_order_should_override_association_order
- assert_equal [companies(:first_client), companies(:second_client)], companies(:first_firm).clients_sorted_desc.find(:all, :conditions => "type = 'Client'", :order => 'id')
- assert_equal [companies(:first_client), companies(:second_client)], companies(:first_firm).clients_sorted_desc.find_all_by_type('Client', :order => 'id')
- end
-
def test_dynamic_find_all_should_respect_association_limit
assert_equal 1, companies(:first_firm).limited_clients.find(:all, :conditions => "type = 'Client'").length
assert_equal 1, companies(:first_firm).limited_clients.find_all_by_type('Client').length
@@ -1018,21 +1013,11 @@ def test_dynamic_find_should_respect_association_order_for_through
assert_equal Comment.find(10), authors(:david).comments_desc.find_by_type('SpecialComment')
end
- def test_dynamic_find_order_should_override_association_order_for_through
- assert_equal Comment.find(3), authors(:david).comments_desc.find(:first, :conditions => "comments.type = 'SpecialComment'", :order => 'comments.id')
- assert_equal Comment.find(3), authors(:david).comments_desc.find_by_type('SpecialComment', :order => 'comments.id')
- end
-
def test_dynamic_find_all_should_respect_association_order_for_through
assert_equal [Comment.find(10), Comment.find(7), Comment.find(6), Comment.find(3)], authors(:david).comments_desc.find(:all, :conditions => "comments.type = 'SpecialComment'")
assert_equal [Comment.find(10), Comment.find(7), Comment.find(6), Comment.find(3)], authors(:david).comments_desc.find_all_by_type('SpecialComment')
end
- def test_dynamic_find_all_order_should_override_association_order_for_through
- assert_equal [Comment.find(3), Comment.find(6), Comment.find(7), Comment.find(10)], authors(:david).comments_desc.find(:all, :conditions => "comments.type = 'SpecialComment'", :order => 'comments.id')
- assert_equal [Comment.find(3), Comment.find(6), Comment.find(7), Comment.find(10)], authors(:david).comments_desc.find_all_by_type('SpecialComment', :order => 'comments.id')
- end
-
def test_dynamic_find_all_should_respect_association_limit_for_through
assert_equal 1, authors(:david).limited_comments.find(:all, :conditions => "comments.type = 'SpecialComment'").length
assert_equal 1, authors(:david).limited_comments.find_all_by_type('SpecialComment').length
View
13 activerecord/test/cases/relation_scoping_test.rb
@@ -363,29 +363,30 @@ def test_default_scope_called_twice_merges_conditions
assert_equal "David", klass.first.name
assert_equal 100000, klass.first.salary
end
+
def test_method_scope
- expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary }
+ expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary }
received = DeveloperOrderedBySalary.all_ordered_by_name.collect { |dev| dev.salary }
assert_equal expected, received
end
def test_nested_scope
- expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary }
+ expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary }
received = DeveloperOrderedBySalary.send(:with_scope, :find => { :order => 'name DESC'}) do
DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary }
end
assert_equal expected, received
end
def test_named_scope_overwrites_default
- expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.name }
+ expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.name }
received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.name }
assert_equal expected, received
end
- def test_named_scope_reorders_default
- expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.name }
- received = DeveloperOrderedBySalary.reordered_by_name.find(:all).collect { |dev| dev.name }
+ def test_reorder_overrides_default_scope_order
+ expected = Developer.order('name DESC').collect { |dev| dev.name }
+ received = DeveloperOrderedBySalary.reorder('name DESC').collect { |dev| dev.name }
assert_equal expected, received
end
View
2 activerecord/test/models/developer.rb
@@ -87,7 +87,7 @@ def raise_if_projects_empty!
class DeveloperOrderedBySalary < ActiveRecord::Base
self.table_name = 'developers'
default_scope :order => 'salary DESC'
- scope :by_name, :order => 'name DESC'
+ scope :by_name, order('name DESC')
scope :reordered_by_name, reorder('name DESC')
def self.all_ordered_by_name

0 comments on commit c07f0ae

Please sign in to comment.