diff options
author | Sean Griffin <sean@thoughtbot.com> | 2015-07-18 10:28:24 -0400 |
---|---|---|
committer | Sean Griffin <sean@thoughtbot.com> | 2015-07-18 10:30:58 -0400 |
commit | 7550f0a016ee6647aaa76c0c0ae30bebc3867288 (patch) | |
tree | 1a13b0e841e8e2aac71abed0d3192d25b408ac65 /activerecord | |
parent | 2a0a264b39eb99ddf444bbdacf3014868c8896cc (diff) | |
download | rails-7550f0a016ee6647aaa76c0c0ae30bebc3867288.tar.gz rails-7550f0a016ee6647aaa76c0c0ae30bebc3867288.tar.bz2 rails-7550f0a016ee6647aaa76c0c0ae30bebc3867288.zip |
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 #20874.
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 7 | ||||
-rw-r--r-- | activerecord/lib/active_record/autosave_association.rb | 7 | ||||
-rw-r--r-- | activerecord/test/cases/autosave_association_test.rb | 8 | ||||
-rw-r--r-- | activerecord/test/models/ship.rb | 11 | ||||
-rw-r--r-- | activerecord/test/schema/schema.rb | 4 |
5 files changed, 37 insertions, 0 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 47364325ea..997abcb48d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Ensure that cyclic associations with autosave don't cause duplicate errors + to be added to the parent record. + + Fixes #20874. + + *Sean Griffin* + * Ensure that `ActionController::Parameters` can still be passed to nested attributes. diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 0792d19c3e..5f38bd51f6 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -222,6 +222,7 @@ module ActiveRecord true end validate validation_method + after_validation :_ensure_no_duplicate_errors end end end @@ -456,5 +457,11 @@ module ActiveRecord end end end + + def _ensure_no_duplicate_errors + errors.messages.each_key do |attribute| + errors[attribute].uniq! + end + end end end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 676b29ed7c..6b69b96e21 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -67,6 +67,14 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase assert_no_difference_when_adding_callbacks_twice_for Pirate, :parrots end + def test_cyclic_autosaves_do_not_add_multiple_validations + ship = ShipWithoutNestedAttributes.new + ship.prisoners.build + + assert_not ship.valid? + assert_equal 1, ship.errors[:name].length + end + private def assert_no_difference_when_adding_callbacks_twice_for(model, association_name) diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 312caef604..fcc533380b 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -19,6 +19,17 @@ class Ship < ActiveRecord::Base end end +class ShipWithoutNestedAttributes < ActiveRecord::Base + self.table_name = "ships" + has_many :prisoners, inverse_of: :ship, foreign_key: :ship_id + + validates :name, presence: true +end + +class Prisoner < ActiveRecord::Base + belongs_to :ship, autosave: true, class_name: "ShipWithoutNestedAttributes", inverse_of: :prisoners +end + class FamousShip < ActiveRecord::Base self.table_name = 'ships' belongs_to :famous_pirate diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6872b49ad9..2fc806f181 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -678,6 +678,10 @@ ActiveRecord::Schema.define do t.datetime :updated_at end + create_table :prisoners, force: true do |t| + t.belongs_to :ship + end + create_table :speedometers, force: true, id: false do |t| t.string :speedometer_id t.string :name |