Skip to content

Commit

Permalink
[Fix rails#50897] Autosaving sets foreign key attribute when unchanged
Browse files Browse the repository at this point in the history
  • Loading branch information
joshuay03 committed Jan 28, 2024
1 parent 351a8d9 commit 05f9a4e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 4 deletions.
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
* Fix autosaving of `has_one` associations setting the foreign key attribute when it is unchanged.

This behaviour was inconsistent with autosaving `belongs_to` associations and can have unintended side effects like
raising `ActiveRecord::ReadOnlyAttributeError` when the foreign key attribute is marked as read-only.

*Joshua Young*

* Consistently raise an `ArgumentError` when passing an invalid argument to a nested attributes association writer.

Previously, this would only raise on collection associations and produce a generic error on singular associations.
Expand Down
3 changes: 2 additions & 1 deletion activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -458,7 +458,8 @@ def save_has_one_association(reflection)
primary_key_foreign_key_pairs = primary_key.zip(foreign_key)

primary_key_foreign_key_pairs.each do |primary_key, foreign_key|
record[foreign_key] = _read_attribute(primary_key)
association_id = _read_attribute(primary_key)
record[foreign_key] = association_id unless record[foreign_key] == association_id
end
association.set_inverse_instance(record)
end
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Expand Up @@ -302,6 +302,13 @@ def test_callbacks_firing_order_on_save
eye.update(iris_attributes: { color: "blue" })
assert_equal [false, false, false, false], eye.after_save_callbacks_stack
end

def test_foreign_key_attribute_is_not_set_unless_changed
eye = Eye.create!(iris_with_read_only_foreign_key_attributes: { color: "honey" })
assert_nothing_raised do
eye.update!(override_iris_with_read_only_foreign_key_color: true)
end
end
end

class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
Expand Down
20 changes: 17 additions & 3 deletions activerecord/test/models/eye.rb
Expand Up @@ -4,6 +4,7 @@ class Eye < ActiveRecord::Base
attr_reader :after_create_callbacks_stack
attr_reader :after_update_callbacks_stack
attr_reader :after_save_callbacks_stack
attr_accessor :override_iris_with_read_only_foreign_key_color

# Callbacks configured before the ones has_one sets up.
after_create :trace_after_create
Expand All @@ -13,27 +14,40 @@ class Eye < ActiveRecord::Base
has_one :iris
accepts_nested_attributes_for :iris

has_one :iris_with_read_only_foreign_key, class_name: "IrisWithReadOnlyForeignKey", foreign_key: :eye_id
accepts_nested_attributes_for :iris_with_read_only_foreign_key

before_save do |eye|
if eye.override_iris_with_read_only_foreign_key_color
eye.iris_with_read_only_foreign_key&.color = "blue"
end
end

# Callbacks configured after the ones has_one sets up.
after_create :trace_after_create2
after_update :trace_after_update2
after_save :trace_after_save2

def trace_after_create
(@after_create_callbacks_stack ||= []) << !iris.persisted?
(@after_create_callbacks_stack ||= []) << !iris&.persisted?
end
alias trace_after_create2 trace_after_create

def trace_after_update
(@after_update_callbacks_stack ||= []) << iris.has_changes_to_save?
(@after_update_callbacks_stack ||= []) << iris&.has_changes_to_save?
end
alias trace_after_update2 trace_after_update

def trace_after_save
(@after_save_callbacks_stack ||= []) << iris.has_changes_to_save?
(@after_save_callbacks_stack ||= []) << iris&.has_changes_to_save?
end
alias trace_after_save2 trace_after_save
end

class Iris < ActiveRecord::Base
belongs_to :eye
end

class IrisWithReadOnlyForeignKey < Iris
attr_readonly :eye_id
end

0 comments on commit 05f9a4e

Please sign in to comment.