Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactored previous changes to nested attributes.

  • Loading branch information...
commit 90f001ba391fa9021d179b4a3d5d95c1308c04b4 1 parent 7074c5a
Eloy Durán alloy authored
50 activerecord/lib/active_record/nested_attributes.rb
View
@@ -209,9 +209,9 @@ module ClassMethods
# exception is raised. If omitted, any number associations can be processed.
# Note that the :limit option is only applicable to one-to-many associations.
# [:update_only]
- # Allows to specify that the an existing record can only be updated.
- # A new record in only created when there is no existing record. This
- # option only works for on-to-one associations and is ignored for
+ # Allows you to specify that an existing record may only be updated.
+ # A new record may only be created when there is no existing record.
+ # This option only works for one-to-one associations and is ignored for
# collection associations. This option is off by default.
#
# Examples:
@@ -238,7 +238,7 @@ def accepts_nested_attributes_for(*attr_names)
self.nested_attributes_options[association_name.to_sym] = options
# def pirate_attributes=(attributes)
- # assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false)
+ # assign_nested_attributes_for_one_to_one_association(:pirate, attributes)
# end
class_eval %{
def #{association_name}_attributes=(attributes)
@@ -279,43 +279,29 @@ def _delete #:nodoc:
# Assigns the given attributes to the association.
#
- # If the given attributes include an <tt>:id</tt> that matches the existing
- # record’s id, then the existing record will be modified. Otherwise a new
- # record will be built.
- #
- # If update_only is true, a new record is only created when no object exists,
- # otherwise it will be updated
- #
# If update_only is false and the given attributes include an <tt>:id</tt>
# that matches the existing record’s id, then the existing record will be
- # modified. Otherwise a new record will be built.
+ # modified. If update_only is true, a new record is only created when no
+ # object exists. Otherwise a new record will be built.
#
- # If the given attributes include a matching <tt>:id</tt> attribute _and_ a
- # <tt>:_destroy</tt> key set to a truthy value, then the existing record
- # will be marked for destruction.
+ # If the given attributes include a matching <tt>:id</tt> attribute, or
+ # update_only is true, and a <tt>:_destroy</tt> 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]
attributes = attributes.with_indifferent_access
+ check_existing_record = (options[:update_only] || !attributes['id'].blank?)
- if options[:update_only]
- if existing_record = send(association_name)
- assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
+ 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 !reject_new_record?(association_name, attributes)
+ method = "build_#{association_name}"
+ if respond_to?(method)
+ send(method, attributes.except(*UNASSIGNABLE_KEYS))
else
- unless reject_new_record?(association_name, attributes)
- send("build_#{association_name}", attributes.except(*UNASSIGNABLE_KEYS))
- end
- end
- elsif attributes['id'].blank?
- unless reject_new_record?(association_name, attributes)
- method = "build_#{association_name}"
- if respond_to?(method)
- send(method, attributes.except(*UNASSIGNABLE_KEYS))
- else
- raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?"
- end
+ raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?"
end
- elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s
- assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
end
end
41 activerecord/test/cases/nested_attributes_test.rb
View
@@ -230,34 +230,24 @@ def test_should_automatically_enable_autosave_on_the_association
end
def test_should_accept_update_only_option
- Pirate.accepts_nested_attributes_for :ship, :update_only => true
- @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' })
-
- Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+ @pirate.update_attribute(:update_only_ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' })
end
def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
- Pirate.accepts_nested_attributes_for :ship, :update_only => true
@ship.delete
-
assert_difference('Ship.count', 1) do
- @pirate.reload.update_attribute(:ship_attributes, { :name => 'Mayflower' })
+ @pirate.reload.update_attribute(:update_only_ship_attributes, { :name => 'Mayflower' })
end
-
- Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
end
-
def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
- Pirate.accepts_nested_attributes_for :ship, :update_only => true
+ @ship.delete
+ @ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning')
assert_no_difference('Ship.count') do
- @pirate.reload.update_attributes(:ship_attributes => { :name => 'Mayflower' })
+ @pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
end
-
assert_equal 'Mayflower', @ship.reload.name
-
- Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
end
end
@@ -376,6 +366,27 @@ def test_should_not_destroy_the_associated_model_until_the_parent_is_saved
def test_should_automatically_enable_autosave_on_the_association
assert Ship.reflect_on_association(:pirate).options[:autosave]
end
+
+ def test_should_accept_update_only_option
+ @ship.update_attribute(:update_only_pirate_attributes, { :id => @pirate.ship.id, :catchphrase => 'Arr' })
+ end
+
+ def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
+ @pirate.delete
+ assert_difference('Pirate.count', 1) do
+ @ship.reload.update_attribute(:update_only_pirate_attributes, { :catchphrase => 'Arr' })
+ end
+ end
+
+ def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
+ @pirate.delete
+ @pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye')
+
+ assert_no_difference('Pirate.count') do
+ @ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' })
+ end
+ assert_equal 'Arr', @pirate.reload.catchphrase
+ end
end
module NestedAttributesOnACollectionAssociationTests
2  activerecord/test/models/pirate.rb
View
@@ -19,6 +19,7 @@ class Pirate < ActiveRecord::Base
# These both have :autosave enabled because accepts_nested_attributes_for is used on them.
has_one :ship
+ has_one :update_only_ship, :class_name => 'Ship'
has_one :non_validated_ship, :class_name => 'Ship'
has_many :birds
has_many :birds_with_method_callbacks, :class_name => "Bird",
@@ -34,6 +35,7 @@ class Pirate < ActiveRecord::Base
accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+ accepts_nested_attributes_for :update_only_ship, :update_only => true
accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks,
:birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true
2  activerecord/test/models/ship.rb
View
@@ -2,9 +2,11 @@ class Ship < ActiveRecord::Base
self.record_timestamps = false
belongs_to :pirate
+ belongs_to :update_only_pirate, :class_name => 'Pirate'
has_many :parts, :class_name => 'ShipPart', :autosave => true
accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+ accepts_nested_attributes_for :update_only_pirate, :update_only => true
validates_presence_of :name
end

1 comment on commit 90f001b

Claudio Poli

finally, thanks alloy

Please sign in to comment.
Something went wrong with that request. Please try again.