Fix adding multiple instances of the same record to a has_many :through.
Fixes #3425.
This commit is contained in:
parent
b4b178f7e9
commit
71bc921ec8
@ -39,6 +39,10 @@
|
||||
|
||||
*Rails 3.1.2 (unreleased)*
|
||||
|
||||
* Fix adding multiple instances of the same record to a has_many :through. [GH #3425]
|
||||
|
||||
[Jon Leighton]
|
||||
|
||||
* Fix creating records in a through association with a polymorphic source type. [GH #3247]
|
||||
|
||||
[Jon Leighton]
|
||||
|
@ -6,6 +6,11 @@ module Associations
|
||||
class HasManyThroughAssociation < HasManyAssociation #:nodoc:
|
||||
include ThroughAssociation
|
||||
|
||||
def initialize(owner, reflection)
|
||||
super
|
||||
@through_records = {}
|
||||
end
|
||||
|
||||
# Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been
|
||||
# loaded and calling collection.size if it has. If it's more likely than not that the collection does
|
||||
# have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer
|
||||
@ -42,27 +47,42 @@ def insert_record(record, validate = true, raise = false)
|
||||
end
|
||||
end
|
||||
|
||||
through_record(record).save!
|
||||
save_through_record(record)
|
||||
update_counter(1)
|
||||
record
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def through_record(record)
|
||||
through_association = owner.association(through_reflection.name)
|
||||
attributes = construct_join_attributes(record)
|
||||
def through_association
|
||||
owner.association(through_reflection.name)
|
||||
end
|
||||
|
||||
through_record = Array.wrap(through_association.target).find { |candidate|
|
||||
candidate.attributes.slice(*attributes.keys) == attributes
|
||||
}
|
||||
|
||||
unless through_record
|
||||
through_record = through_association.build(attributes)
|
||||
# We temporarily cache through record that has been build, because if we build a
|
||||
# through record in build_record and then subsequently call insert_record, then we
|
||||
# want to use the exact same object.
|
||||
#
|
||||
# However, after insert_record has been called, we clear the cache entry because
|
||||
# we want it to be possible to have multiple instances of the same record in an
|
||||
# association
|
||||
def build_through_record(record)
|
||||
@through_records[record.object_id] ||= begin
|
||||
through_record = through_association.build(construct_join_attributes(record))
|
||||
through_record.send("#{source_reflection.name}=", record)
|
||||
through_record
|
||||
end
|
||||
end
|
||||
|
||||
through_record
|
||||
def save_through_record(record)
|
||||
build_through_record(record).save!
|
||||
ensure
|
||||
@through_records.delete(record.object_id)
|
||||
end
|
||||
|
||||
def through_record(record)
|
||||
attributes = construct_join_attributes(record)
|
||||
candidates = Array.wrap(through_association.target)
|
||||
candidates.find { |c| c.attributes.slice(*attributes.keys) == attributes }
|
||||
end
|
||||
|
||||
def build_record(attributes, options = {})
|
||||
@ -73,9 +93,9 @@ def build_record(attributes, options = {})
|
||||
inverse = source_reflection.inverse_of
|
||||
if inverse
|
||||
if inverse.macro == :has_many
|
||||
record.send(inverse.name) << through_record(record)
|
||||
record.send(inverse.name) << build_through_record(record)
|
||||
elsif inverse.macro == :has_one
|
||||
record.send("#{inverse.name}=", through_record(record))
|
||||
record.send("#{inverse.name}=", build_through_record(record))
|
||||
end
|
||||
end
|
||||
|
||||
@ -104,7 +124,7 @@ def update_through_counter?(method)
|
||||
def delete_records(records, method)
|
||||
ensure_not_nested
|
||||
|
||||
through = owner.association(through_reflection.name)
|
||||
through = through_association
|
||||
scope = through.scoped.where(construct_join_attributes(*records))
|
||||
|
||||
case method
|
||||
@ -126,14 +146,16 @@ def delete_records(records, method)
|
||||
end
|
||||
|
||||
def delete_through_records(through, records)
|
||||
if through_reflection.macro == :has_many
|
||||
records.each do |record|
|
||||
through.target.delete(through_record(record))
|
||||
end
|
||||
else
|
||||
records.each do |record|
|
||||
through.target = nil if through.target == through_record(record)
|
||||
records.each do |record|
|
||||
through_record = through_record(record)
|
||||
|
||||
if through_reflection.macro == :has_many
|
||||
through.target.delete(through_record)
|
||||
else
|
||||
through.target = nil if through.target == through_record
|
||||
end
|
||||
|
||||
@through_records.delete(record.object_id)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -67,6 +67,16 @@ def test_associate_existing_record_twice_should_add_to_target_twice
|
||||
end
|
||||
end
|
||||
|
||||
def test_associate_existing_record_twice_should_add_records_twice
|
||||
post = posts(:thinking)
|
||||
person = people(:david)
|
||||
|
||||
assert_difference 'post.people.count', 2 do
|
||||
post.people << person
|
||||
post.people << person
|
||||
end
|
||||
end
|
||||
|
||||
def test_associating_new
|
||||
assert_queries(1) { posts(:thinking) }
|
||||
new_person = nil # so block binding catches it
|
||||
|
Loading…
Reference in New Issue
Block a user