From 7aad851c2e0d4579aa33a54a069a767b53cca406 Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Fri, 30 Apr 2010 14:59:41 -0700 Subject: Allow pre-casted values (other than nil) to pass through from calculations un-touched [#4514 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/relation/calculations.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'activerecord/lib/active_record/relation') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index a5ea6e7e3a..8ab5eaa724 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -239,11 +239,15 @@ module ActiveRecord end def type_cast_calculated_value(value, column, operation = nil) - case operation - when 'count' then value.to_i - when 'sum' then type_cast_using_column(value || '0', column) - when 'average' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d - else type_cast_using_column(value, column) + if value.is_a?(String) || value.nil? + case operation + when 'count' then value.to_i + when 'sum' then type_cast_using_column(value || '0', column) + when 'average' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d + else type_cast_using_column(value, column) + end + else + value end end -- cgit v1.2.3 From 902861a43ae90032063f4a14a3e8b4b9b9c3ca2f Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Thu, 6 May 2010 16:14:09 -0400 Subject: Fix unintuitive behavior with multiple order and group clauses [#4545 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/relation/calculations.rb | 2 +- activerecord/lib/active_record/relation/query_methods.rb | 8 ++------ activerecord/lib/active_record/relation/spawn_methods.rb | 7 ++++++- 3 files changed, 9 insertions(+), 8 deletions(-) (limited to 'activerecord/lib/active_record/relation') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 8ab5eaa724..858d298470 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -195,7 +195,7 @@ module ActiveRecord select_statement << ", #{group_field} AS #{group_alias}" - relation = select(select_statement).group(group) + relation = except(:group).select(select_statement).group(group) calculated_data = @klass.connection.select_all(relation.to_sql) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 7bca12d85e..8d8bb659e1 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -162,13 +162,9 @@ module ActiveRecord arel = arel.take(@limit_value) if @limit_value.present? arel = arel.skip(@offset_value) if @offset_value.present? - @group_values.uniq.each do |g| - arel = arel.group(g) if g.present? - end + arel = arel.group(*@group_values.uniq.select{|g| g.present?}) - @order_values.uniq.each do |o| - arel = arel.order(Arel::SqlLiteral.new(o.to_s)) if o.present? - end + arel = arel.order(*@order_values.uniq.select{|o| o.present?}.map(&:to_s)) selects = @select_values.uniq diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 8fdd64afcc..bb1f138f5b 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -80,10 +80,15 @@ module ActiveRecord options.assert_valid_keys(VALID_FIND_OPTIONS) - [:joins, :select, :group, :having, :order, :limit, :offset, :from, :lock, :readonly].each do |finder| + [:joins, :select, :group, :having, :limit, :offset, :from, :lock, :readonly].each do |finder| relation = relation.send(finder, options[finder]) if options.has_key?(finder) end + # Give precedence to newly-applied orders and groups to play nicely with with_scope + [:group, :order].each do |finder| + relation.send("#{finder}_values=", Array.wrap(options[finder]) + relation.send("#{finder}_values")) if options.has_key?(finder) + end + relation = relation.where(options[:conditions]) if options.has_key?(:conditions) relation = relation.includes(options[:include]) if options.has_key?(:include) relation = relation.extending(options[:extend]) if options.has_key?(:extend) -- cgit v1.2.3 From 9aaef5935660ba13531512fb7def4b8bdf14511d Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 6 May 2010 23:47:23 -0300 Subject: Make find_or_create and find_or_initialize work mixing explicit parameters and a hash [#4457 state:committed] Signed-off-by: Jeremy Kemper --- .../lib/active_record/relation/finder_methods.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'activerecord/lib/active_record/relation') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index d6144dc206..f69acb6877 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -234,20 +234,24 @@ module ActiveRecord end def find_or_instantiator_by_attributes(match, attributes, *args) - guard_protected_attributes = false - - if args[0].is_a?(Hash) - guard_protected_attributes = true - attributes_for_create = args[0].with_indifferent_access - conditions = attributes_for_create.slice(*attributes).symbolize_keys - else - attributes_for_create = conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h} + protected_attributes_for_create = unprotected_attributes_for_create = {} + args.each_with_index do |arg, i| + if arg.is_a?(Hash) + protected_attributes_for_create = args[i].with_indifferent_access + else + unprotected_attributes_for_create[attributes[i]] = args[i] + end end + conditions = (protected_attributes_for_create.merge(unprotected_attributes_for_create)).slice(*attributes).symbolize_keys + record = where(conditions).first unless record - record = @klass.new { |r| r.send(:attributes=, attributes_for_create, guard_protected_attributes) } + record = @klass.new do |r| + r.send(:attributes=, protected_attributes_for_create, true) unless protected_attributes_for_create.empty? + r.send(:attributes=, unprotected_attributes_for_create, false) unless unprotected_attributes_for_create.empty? + end yield(record) if block_given? record.save if match.instantiator == :create end -- cgit v1.2.3 From 0b4211c88b97fcac7f98170d73c4e264ce253700 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 8 May 2010 17:17:11 -0300 Subject: Here the intention was to assign to different objects --- activerecord/lib/active_record/relation/finder_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record/relation') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index f69acb6877..7a0c9dc612 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -234,7 +234,7 @@ module ActiveRecord end def find_or_instantiator_by_attributes(match, attributes, *args) - protected_attributes_for_create = unprotected_attributes_for_create = {} + protected_attributes_for_create, unprotected_attributes_for_create = {}, {} args.each_with_index do |arg, i| if arg.is_a?(Hash) protected_attributes_for_create = args[i].with_indifferent_access -- cgit v1.2.3