Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #9426 from exviva/nested_attributes_reuse_existing…

…_new_record

Do not overwrite manually built records during one-to-one nested attribute assignment
Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
commit ae8f2c6ed79af1774f74f84f70a4cee249ce769f 1 parent e1fbf7f
@jonleighton jonleighton authored
View
27 activerecord/CHANGELOG.md
@@ -1,6 +1,33 @@
## unreleased ##
+
* Fix `add_column` with `array` option when using PostgreSQL. Fixes #10432
+* Do not overwrite manually built records during one-to-one nested attribute assignment
+
+ For one-to-one nested associations, if you build the new (in-memory)
+ child object yourself before assignment, then the NestedAttributes
+ module will not overwrite it, e.g.:
+
+ class Member < ActiveRecord::Base
+ has_one :avatar
+ accepts_nested_attributes_for :avatar
+
+ def avatar
+ super || build_avatar(width: 200)
+ end
+ end
+
+ member = Member.new
+ member.avatar_attributes = {icon: 'sad'}
+ member.avatar.width # => 200
+
+ *Olek Janiszewski*
+
+* fixes bug introduced by #3329. Now, when autosaving associations,
+ deletions happen before inserts and saves. This prevents a 'duplicate
+ unique value' database error that would occur if a record being created had
+ the same value on a unique indexed field as that of a record being destroyed.
+
*Adam Anderson*
View
12 activerecord/lib/active_record/associations/association.rb
@@ -164,6 +164,13 @@ def marshal_load(data)
@reflection = @owner.class.reflect_on_association(reflection_name)
end
+ def initialize_attributes(record) #:nodoc:
+ skip_assign = [reflection.foreign_key, reflection.type].compact
+ attributes = create_scope.except(*(record.changed - skip_assign))
+ record.assign_attributes(attributes)
+ set_inverse_instance(record)
+ end
+
private
def find_target?
@@ -233,10 +240,7 @@ def stale_state
def build_record(attributes)
reflection.build_association(attributes) do |record|
- skip_assign = [reflection.foreign_key, reflection.type].compact
- attributes = create_scope.except(*(record.changed - skip_assign))
- record.assign_attributes(attributes)
- set_inverse_instance(record)
+ initialize_attributes(record)
end
end
end
View
39 activerecord/lib/active_record/nested_attributes.rb
@@ -229,6 +229,23 @@ class TooManyRecords < ActiveRecordError
# belongs_to :member, inverse_of: :posts
# validates_presence_of :member
# end
+ #
+ # For one-to-one nested associations, if you build the new (in-memory)
+ # child object yourself before assignment, then this module will not
+ # overwrite it, e.g.:
+ #
+ # class Member < ActiveRecord::Base
+ # has_one :avatar
+ # accepts_nested_attributes_for :avatar
+ #
+ # def avatar
+ # super || build_avatar(width: 200)
+ # end
+ # end
+ #
+ # member = Member.new
+ # member.avatar_attributes = {icon: 'sad'}
+ # member.avatar.width # => 200
module ClassMethods
REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |key, value| key == '_destroy' || value.blank? } }
@@ -356,20 +373,28 @@ def _destroy
def assign_nested_attributes_for_one_to_one_association(association_name, attributes)
options = self.nested_attributes_options[association_name]
attributes = attributes.with_indifferent_access
+ existing_record = send(association_name)
- if (options[:update_only] || !attributes['id'].blank?) && (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]) unless call_reject_if(association_name, attributes)
+ if (options[:update_only] || !attributes['id'].blank?) && existing_record &&
+ (options[:update_only] || existing_record.id.to_s == attributes['id'].to_s)
+ assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes)
elsif attributes['id'].present?
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)
- send(method, attributes.except(*UNASSIGNABLE_KEYS))
+ assignable_attributes = attributes.except(*UNASSIGNABLE_KEYS)
+
+ if existing_record && existing_record.new_record?
+ existing_record.assign_attributes(assignable_attributes)
+ association(association_name).initialize_attributes(existing_record)
else
- raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?"
+ method = "build_#{association_name}"
+ if respond_to?(method)
+ send(method, assignable_attributes)
+ else
+ raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?"
+ end
end
end
end
View
14 activerecord/test/cases/nested_attributes_test.rb
@@ -131,6 +131,20 @@ def test_reject_if_with_a_proc_which_returns_true_always_for_has_one
assert_equal 's1', ship.reload.name
end
+ def test_reuse_already_built_new_record
+ pirate = Pirate.new
+ ship_built_first = pirate.build_ship
+ pirate.ship_attributes = { name: 'Ship 1' }
+ assert_equal ship_built_first.object_id, pirate.ship.object_id
+ end
+
+ def test_do_not_allow_assigning_foreign_key_when_reusing_existing_new_record
+ pirate = Pirate.create!(catchphrase: "Don' botharrr talkin' like one, savvy?")
+ pirate.build_ship
+ pirate.ship_attributes = { name: 'Ship 1', pirate_id: pirate.id + 1 }
+ assert_equal pirate.id, pirate.ship.pirate_id
+ end
+
def test_reject_if_with_a_proc_which_returns_true_always_for_has_many
Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true }
man = Man.create(name: "John")
Please sign in to comment.
Something went wrong with that request. Please try again.