From 1afa9fa5a973734852bd7a9d20a534355c221098 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Wed, 30 Dec 2009 22:10:18 +0100 Subject: Refactored nested attributes a bit around :reject_if => :all_blank. --- activerecord/lib/active_record/nested_attributes.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index ff3a51d5c0..3e6f84f31d 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -188,6 +188,8 @@ module ActiveRecord # the parent model is saved. This happens inside the transaction initiated # by the parents save method. See ActiveRecord::AutosaveAssociation. module ClassMethods + REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |_, value| value.blank? } } + # Defines an attributes writer for the specified association(s). If you # are using attr_protected or attr_accessible, then you # will need to add the attribute writer to the allowed list. @@ -229,6 +231,7 @@ module ActiveRecord options = { :allow_destroy => false, :update_only => false } options.update(attr_names.extract_options!) options.assert_valid_keys(:allow_destroy, :reject_if, :limit, :update_only) + options[:reject_if] = REJECT_ALL_BLANK_PROC if options[:reject_if] == :all_blank attr_names.each do |association_name| if reflection = reflect_on_association(association_name) @@ -241,11 +244,7 @@ module ActiveRecord reflection.options[:autosave] = true add_autosave_association_callbacks(reflection) - self.nested_attributes_options[association_name.to_sym] = options - - if options[:reject_if] == :all_blank - self.nested_attributes_options[association_name.to_sym][:reject_if] = proc { |attributes| attributes.all? {|k,v| v.blank?} } - end + nested_attributes_options[association_name.to_sym] = options # def pirate_attributes=(attributes) # assign_nested_attributes_for_one_to_one_association(:pirate, attributes) -- cgit v1.2.3 From f82adc7c5addb4f9bf6b9cb3f1fcf3fb47505e53 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 31 Dec 2009 14:03:04 +0100 Subject: Add AssociationReflection#collection_association? which returns true if it's for a has_many or has_and_belongs_to_many association. --- activerecord/lib/active_record/reflection.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index b751c9ad68..69772bf9bb 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -252,10 +252,17 @@ module ActiveRecord end end + # Returns whether or not this association reflection is for a collection + # association. Returns +true+ if the +macro+ is one of +has_many+ or + # +has_and_belongs_to_many+, +false+ otherwise. + def collection_association? + [:has_many, :has_and_belongs_to_many].include?(macro) + end + private def derive_class_name class_name = name.to_s.camelize - class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro) + class_name = class_name.singularize if collection_association? class_name end -- cgit v1.2.3 From 2171e0a1d13a176db1e9e9dd55242fdb319b526c Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 31 Dec 2009 14:07:56 +0100 Subject: Cleanup some code in nested_attributes.rb, autosave_association.rb, and associations.rb with AssociationReflection#collection_association? Also cache the result value. --- activerecord/lib/active_record/associations.rb | 7 +++---- activerecord/lib/active_record/autosave_association.rb | 3 +-- activerecord/lib/active_record/nested_attributes.rb | 8 +------- activerecord/lib/active_record/reflection.rb | 5 ++++- 4 files changed, 9 insertions(+), 14 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b2ae447d5e..1320f5b624 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1770,7 +1770,7 @@ module ActiveRecord end def using_limitable_reflections?(reflections) - reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero? + reflections.collect(&:collection_association?).length.zero? end def column_aliases(join_dependency) @@ -1843,7 +1843,7 @@ module ActiveRecord case associations when Symbol, String reflection = base.reflections[associations] - if reflection && [:has_many, :has_and_belongs_to_many].include?(reflection.macro) + if reflection && reflection.collection_association? records.each { |record| record.send(reflection.name).target.uniq! } end when Array @@ -1853,12 +1853,11 @@ module ActiveRecord when Hash associations.keys.each do |name| reflection = base.reflections[name] - is_collection = [:has_many, :has_and_belongs_to_many].include?(reflection.macro) parent_records = [] records.each do |record| if descendant = record.send(reflection.name) - if is_collection + if reflection.collection_association? parent_records.concat descendant.target.uniq else parent_records << descendant diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 98ab64537e..62cb996b5d 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -167,8 +167,7 @@ module ActiveRecord validation_method = "validate_associated_records_for_#{reflection.name}" force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true) - case reflection.macro - when :has_many, :has_and_belongs_to_many + if reflection.collection_association? unless method_defined?(save_method) before_save :before_save_collection_association diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 3e6f84f31d..e05e46916c 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -235,16 +235,10 @@ module ActiveRecord attr_names.each do |association_name| if reflection = reflect_on_association(association_name) - type = case reflection.macro - when :has_one, :belongs_to - :one_to_one - when :has_many, :has_and_belongs_to_many - :collection - end - reflection.options[:autosave] = true add_autosave_association_callbacks(reflection) nested_attributes_options[association_name.to_sym] = options + type = (reflection.collection_association? ? :collection : :one_to_one) # def pirate_attributes=(attributes) # assign_nested_attributes_for_one_to_one_association(:pirate, attributes) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 69772bf9bb..96aac96cda 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -256,7 +256,10 @@ module ActiveRecord # association. Returns +true+ if the +macro+ is one of +has_many+ or # +has_and_belongs_to_many+, +false+ otherwise. def collection_association? - [:has_many, :has_and_belongs_to_many].include?(macro) + if @collection_association.nil? + @collection_association = [:has_many, :has_and_belongs_to_many].include?(macro) + end + @collection_association end private -- cgit v1.2.3 From f866ced24ac9cdece7801f50ec79cdbe3ff5ad59 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Sat, 2 Jan 2010 00:49:49 +0100 Subject: Don't use strings for callbacks, as these will be evaled. Rather use symbols, which uses a direct method dispatch. Patch by Comron Sattari. [#3429 state:resolved] --- activerecord/lib/active_record/autosave_association.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 62cb996b5d..b3e28b9373 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -163,8 +163,8 @@ module ActiveRecord # guards that check if the save or validation methods have already been # defined before actually defining them. def add_autosave_association_callbacks(reflection) - save_method = "autosave_associated_records_for_#{reflection.name}" - validation_method = "validate_associated_records_for_#{reflection.name}" + save_method = :"autosave_associated_records_for_#{reflection.name}" + validation_method = :"validate_associated_records_for_#{reflection.name}" force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true) if reflection.collection_association? -- cgit v1.2.3 From a559260e41170397ec940321f12853ccdb440993 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Sat, 2 Jan 2010 00:57:04 +0100 Subject: Removed unnecessary call to #try and cleaned up a bit more. --- activerecord/lib/active_record/nested_attributes.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index e05e46916c..c86c1a9d18 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -291,7 +291,7 @@ module ActiveRecord # update_only is true, and a :_destroy key set to a truthy value, # then the existing record will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes) - options = self.nested_attributes_options[association_name] + options = nested_attributes_options[association_name] attributes = attributes.with_indifferent_access check_existing_record = (options[:update_only] || !attributes['id'].blank?) @@ -336,7 +336,7 @@ module ActiveRecord # { :id => '2', :_destroy => true } # ]) def assign_nested_attributes_for_collection_association(association_name, attributes_collection) - options = self.nested_attributes_options[association_name] + options = nested_attributes_options[association_name] unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" @@ -387,13 +387,11 @@ module ActiveRecord end def call_reject_if(association_name, attributes) - callback = self.nested_attributes_options[association_name][:reject_if] - - case callback + case callback = nested_attributes_options[association_name][:reject_if] when Symbol method(callback).arity == 0 ? send(callback) : send(callback, attributes) when Proc - callback.try(:call, attributes) + callback.call(attributes) end end end -- cgit v1.2.3 From fc6aae34597bbecaf441571b20ab861b021ea8a5 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Sat, 2 Jan 2010 14:59:29 +0100 Subject: Remove deprecated '_delete' option from NestedAttributes. --- activerecord/lib/active_record/nested_attributes.rb | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index c86c1a9d18..e1e80355c0 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -264,21 +264,11 @@ module ActiveRecord marked_for_destruction? end - # Deal with deprecated _delete. - # - def _delete #:nodoc: - ActiveSupport::Deprecation.warn "_delete is deprecated in nested attributes. Use _destroy instead." - _destroy - end - private # Attribute hash keys that should not be assigned as normal attributes. # These hash keys are nested attributes implementation details. - # - # TODO Remove _delete from UNASSIGNABLE_KEYS when deprecation warning are - # removed. - UNASSIGNABLE_KEYS = %w( id _destroy _delete ) + UNASSIGNABLE_KEYS = %w( id _destroy ) # Assigns the given attributes to the association. # @@ -375,8 +365,7 @@ module ActiveRecord # Determines if a hash contains a truthy _destroy key. def has_destroy_flag?(hash) - ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) || - ConnectionAdapters::Column.value_to_boolean(hash['_delete']) # TODO Remove after deprecation. + ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) end # Determines if a new record should be build by checking for -- cgit v1.2.3 From b6264c43f414f323656ed135d46213466cbe00fb Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Sun, 3 Jan 2010 21:45:08 +0100 Subject: Moved the validation logic to the association reflection and refactored autosave_association.rb a bit. --- .../lib/active_record/autosave_association.rb | 39 +++++++++------------- activerecord/lib/active_record/reflection.rb | 13 ++++++++ 2 files changed, 29 insertions(+), 23 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index b3e28b9373..f01d0903cd 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -158,46 +158,39 @@ module ActiveRecord # # For performance reasons, we don't check whether to validate at runtime, # but instead only define the method and callback when needed. However, - # this can change, for instance, when using nested attributes. Since we - # don't want the callbacks to get defined multiple times, there are - # guards that check if the save or validation methods have already been - # defined before actually defining them. + # this can change, for instance, when using nested attributes, which is + # called _after_ the association has been defined. Since we don't want + # the callbacks to get defined multiple times, there are guards that + # check if the save or validation methods have already been defined + # before actually defining them. def add_autosave_association_callbacks(reflection) save_method = :"autosave_associated_records_for_#{reflection.name}" validation_method = :"validate_associated_records_for_#{reflection.name}" - force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true) + collection = reflection.collection_association? - if reflection.collection_association? - unless method_defined?(save_method) + unless method_defined?(save_method) + if collection before_save :before_save_collection_association define_method(save_method) { save_collection_association(reflection) } # Doesn't use after_save as that would save associations added in after_create/after_update twice after_create save_method after_update save_method - end - - if !method_defined?(validation_method) && - (force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false)) - define_method(validation_method) { validate_collection_association(reflection) } - validate validation_method - end - else - unless method_defined?(save_method) - case reflection.macro - when :has_one + else + if reflection.macro == :has_one define_method(save_method) { save_has_one_association(reflection) } after_save save_method - when :belongs_to + else define_method(save_method) { save_belongs_to_association(reflection) } before_save save_method end end + end - if !method_defined?(validation_method) && force_validation - define_method(validation_method) { validate_single_association(reflection) } - validate validation_method - end + if reflection.validate? && !method_defined?(validation_method) + method = (collection ? :validate_collection_association : :validate_single_association) + define_method(validation_method) { send(method, reflection) } + validate validation_method end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 96aac96cda..d6fad5cf29 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -262,6 +262,19 @@ module ActiveRecord @collection_association end + # Returns whether or not the association should be validated as part of + # the parent's validation. + # + # Unless you explicitely disable validation with + # :validate => false, it will take place when: + # + # * you explicitely enable validation; :validate => true + # * you use autosave; :autosave => true + # * the association is a +has_many+ association + def validate? + !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many) + end + private def derive_class_name class_name = name.to_s.camelize -- cgit v1.2.3 From 9550916903c931161f04a98091fba712d7fa5c1d Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Sun, 3 Jan 2010 22:48:25 +0100 Subject: Raise a RecordNotFound if an ID in nested attributes is given but doesn't return a record. [#2415 state:resolved] --- activerecord/lib/active_record/nested_attributes.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index e1e80355c0..9038888d22 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -288,6 +288,10 @@ module ActiveRecord if check_existing_record && (record = send(association_name)) && (options[:update_only] || record.id.to_s == attributes['id'].to_s) assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) + + elsif attributes['id'] + raise_nested_attributes_record_not_found(association_name, attributes['id']) + elsif !reject_new_record?(association_name, attributes) method = "build_#{association_name}" if respond_to?(method) @@ -349,6 +353,8 @@ module ActiveRecord end elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s } assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + else + raise_nested_attributes_record_not_found(association_name, attributes['id']) end end end @@ -383,5 +389,10 @@ module ActiveRecord callback.call(attributes) end end + + def raise_nested_attributes_record_not_found(association_name, record_id) + reflection = self.class.reflect_on_association(association_name) + raise RecordNotFound, "Couldn't find #{reflection.klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}" + end end end -- cgit v1.2.3