aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record
diff options
context:
space:
mode:
authorSean Griffin <sean@seantheprogrammer.com>2016-09-29 14:57:31 -0400
committerSean Griffin <sean@seantheprogrammer.com>2016-09-29 14:57:31 -0400
commit268a5bb010ef91880d9b03b60e5e86ae5521e73c (patch)
tree4a0802c7cd40df60f312745f5695b5e9124a668b /activerecord/lib/active_record
parent09487dc86c134f99d2d385f090ea3cfc9e69dc40 (diff)
downloadrails-268a5bb010ef91880d9b03b60e5e86ae5521e73c.tar.gz
rails-268a5bb010ef91880d9b03b60e5e86ae5521e73c.tar.bz2
rails-268a5bb010ef91880d9b03b60e5e86ae5521e73c.zip
Don't skip in-memory insertion of associations when loaded in validate
This was caused by 6d0d83a33f59d9415685852cf77818c41e2e2700. While the bug it's trying to fix is handled if the association is loaded in an after_(create|save) callback, it doesn't handle any cases that load the association before the persistence takes place (validation, or before_* filters). Instead of caring about the timing of persistence, we can just ensure that we're not double adding the record instead. The test from that commit actually broke, but it was not because the bug has been re-introduced. It was because `Bulb` in our test suite is doing funky things that look like STI but isn't STI, so equality comparison didn't happen as the loaded model was of a different class. Fixes #26661.
Diffstat (limited to 'activerecord/lib/active_record')
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb15
-rw-r--r--activerecord/lib/active_record/associations/has_many_through_association.rb4
2 files changed, 12 insertions, 7 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index e5b3af8252..278c95e27b 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -309,15 +309,12 @@ module ActiveRecord
def replace_on_target(record, index, skip_callbacks)
callback(:before_add, record) unless skip_callbacks
- was_loaded = loaded?
yield(record) if block_given?
- unless !was_loaded && loaded?
- if index
- @target[index] = record
- else
- @target << record
- end
+ if index
+ @target[index] = record
+ else
+ append_record(record)
end
callback(:after_add, record) unless skip_callbacks
@@ -514,6 +511,10 @@ module ActiveRecord
load_target.select { |r| ids.include?(r.id.to_s) }
end
end
+
+ def append_record(record)
+ @target << record unless @target.include?(record)
+ end
end
end
end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index d258eac0ed..1f264d325a 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -203,6 +203,10 @@ module ActiveRecord
def invertible_for?(record)
false
end
+
+ def append_record(record)
+ @target << record
+ end
end
end
end