aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-07-18 10:28:24 -0400
committerSean Griffin <sean@thoughtbot.com>2015-07-18 10:30:58 -0400
commit7550f0a016ee6647aaa76c0c0ae30bebc3867288 (patch)
tree1a13b0e841e8e2aac71abed0d3192d25b408ac65
parent2a0a264b39eb99ddf444bbdacf3014868c8896cc (diff)
downloadrails-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.
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/autosave_association.rb7
-rw-r--r--activerecord/test/cases/autosave_association_test.rb8
-rw-r--r--activerecord/test/models/ship.rb11
-rw-r--r--activerecord/test/schema/schema.rb4
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