Skip to content

Commit 7550f0a

Browse files
committed
Ensure cyclic associations w/ autosave don't cause duplicate errors
This code is so fucked. Things that cause this bug not to replicate: - Defining the validation before the association (we end up calling `uniq!` on the errors in the autosave validation) - Adding `accepts_nested_attributes_for` (I have no clue why. The only thing it does that should affect this is adds `autosave: true` to the inverse reflection, and doing that manually doesn't fix this). This solution is a hack, and I'm almost certain there's a better way to go about it, but this shouldn't cause a huge hit on validation times, and is the simplest way to get it done. Fixes rails#20874.
1 parent 2a0a264 commit 7550f0a

File tree

5 files changed

+37
-0
lines changed

5 files changed

+37
-0
lines changed

activerecord/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
* Ensure that cyclic associations with autosave don't cause duplicate errors
2+
to be added to the parent record.
3+
4+
Fixes #20874.
5+
6+
*Sean Griffin*
7+
18
* Ensure that `ActionController::Parameters` can still be passed to nested
29
attributes.
310

activerecord/lib/active_record/autosave_association.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ def define_autosave_validation_callbacks(reflection)
222222
true
223223
end
224224
validate validation_method
225+
after_validation :_ensure_no_duplicate_errors
225226
end
226227
end
227228
end
@@ -456,5 +457,11 @@ def save_belongs_to_association(reflection)
456457
end
457458
end
458459
end
460+
461+
def _ensure_no_duplicate_errors
462+
errors.messages.each_key do |attribute|
463+
errors[attribute].uniq!
464+
end
465+
end
459466
end
460467
end

activerecord/test/cases/autosave_association_test.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ def test_should_not_add_the_same_callbacks_multiple_times_for_has_and_belongs_to
6767
assert_no_difference_when_adding_callbacks_twice_for Pirate, :parrots
6868
end
6969

70+
def test_cyclic_autosaves_do_not_add_multiple_validations
71+
ship = ShipWithoutNestedAttributes.new
72+
ship.prisoners.build
73+
74+
assert_not ship.valid?
75+
assert_equal 1, ship.errors[:name].length
76+
end
77+
7078
private
7179

7280
def assert_no_difference_when_adding_callbacks_twice_for(model, association_name)

activerecord/test/models/ship.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ def cancel_save_callback_method
1919
end
2020
end
2121

22+
class ShipWithoutNestedAttributes < ActiveRecord::Base
23+
self.table_name = "ships"
24+
has_many :prisoners, inverse_of: :ship, foreign_key: :ship_id
25+
26+
validates :name, presence: true
27+
end
28+
29+
class Prisoner < ActiveRecord::Base
30+
belongs_to :ship, autosave: true, class_name: "ShipWithoutNestedAttributes", inverse_of: :prisoners
31+
end
32+
2233
class FamousShip < ActiveRecord::Base
2334
self.table_name = 'ships'
2435
belongs_to :famous_pirate

activerecord/test/schema/schema.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,10 @@ def except(adapter_names_to_exclude)
678678
t.datetime :updated_at
679679
end
680680

681+
create_table :prisoners, force: true do |t|
682+
t.belongs_to :ship
683+
end
684+
681685
create_table :speedometers, force: true, id: false do |t|
682686
t.string :speedometer_id
683687
t.string :name

0 commit comments

Comments
 (0)