diff options
| -rw-r--r-- | activerecord/CHANGELOG | 24 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations.rb | 38 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_many_through_association.rb | 50 | ||||
| -rw-r--r-- | activerecord/test/cases/associations/has_many_through_associations_test.rb | 18 | ||||
| -rw-r--r-- | activerecord/test/models/person.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/models/post.rb | 1 | ||||
| -rw-r--r-- | activerecord/test/models/reader.rb | 3 |
7 files changed, 124 insertions, 12 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 72bbeeec61..e434372fb7 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,29 @@ *Rails 3.1.0 (unreleased)* +* Make has_many :through associations work correctly when you build a record and then save it. This + requires you to set the :inverse_of option on the source reflection on the join model, like so: + + class Post < ActiveRecord::Base + has_many :taggings + has_many :tags, :through => :taggings + end + + class Tagging < ActiveRecord::Base + belongs_to :post + belongs_to :tag, :inverse_of => :tagging # :inverse_of must be set! + end + + class Tag < ActiveRecord::Base + has_many :taggings + has_many :posts, :through => :taggings + end + + post = Post.first + tag = post.tags.build :name => "ruby" + tag.save # will save a Taggable linking to the post + + [Jon Leighton] + * Support the :dependent option on has_many :through associations. For historical and practical reasons, :delete_all is the default deletion strategy employed by association.delete(*records), despite the fact that the default strategy is :nullify for regular has_many. Also, this only diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 529d8ca2d3..bf0f6dc542 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -523,6 +523,22 @@ module ActiveRecord # @group.avatars << Avatar.new # this would work if User belonged_to Avatar rather than the other way around # @group.avatars.delete(@group.avatars.last) # so would this # + # If you are using a +belongs_to+ on the join model, it is a good idea to set the + # <tt>:inverse_of</tt> option on the +belongs_to+, which will mean that the following example + # works correctly (where <tt>tags</tt> is a +has_many+ <tt>:through</tt> association): + # + # @post = Post.first + # @tag = @post.tags.build :name => "ruby" + # @tag.save + # + # The last line ought to save the through record (a <tt>Taggable</tt>). This will only work if the + # <tt>:inverse_of</tt> is set: + # + # class Taggable < ActiveRecord::Base + # belongs_to :post + # belongs_to :tag, :inverse_of => :taggings + # end + # # === Polymorphic Associations # # Polymorphic associations on models are not restricted on what types of models they @@ -1043,13 +1059,21 @@ module ActiveRecord # [:as] # Specifies a polymorphic interface (See <tt>belongs_to</tt>). # [:through] - # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt> - # and <tt>:foreign_key</tt> are ignored, as the association uses the source reflection. You - # can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, <tt>has_one</tt> - # or <tt>has_many</tt> association on the join model. The collection of join models - # can be managed via the collection API. For example, new join models are created for - # newly associated objects, and if some are gone their rows are deleted (directly, - # no destroy callbacks are triggered). + # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt>, + # <tt>:primary_key</tt> and <tt>:foreign_key</tt> are ignored, as the association uses the + # source reflection. You can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, + # <tt>has_one</tt> or <tt>has_many</tt> association on the join model. + # + # If the association on the join model is a +belongs_to+, the collection can be modified + # and the records on the <tt>:through</tt> model will be automatically created and removed + # as appropriate. Otherwise, the collection is read-only, so you should manipulate the + # <tt>:through</tt> association directly. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option on the source association on the + # join model. This allows associated records to be built which will automatically create + # the appropriate join model records when they are saved. (See the 'Association Join Models' + # section above.) # [:source] # Specifies the source association name used by <tt>has_many :through</tt> queries. # Only use it if the name cannot be inferred from the association. 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 9f74d57c4d..c4d3ef8fef 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -37,16 +37,44 @@ module ActiveRecord def insert_record(record, validate = true) return if record.new_record? && !record.save(:validate => validate) - - through_association = @owner.send(@reflection.through_reflection.name) - through_association.create!(construct_join_attributes(record)) - + through_record(record).save! update_counter(1) record end private + def through_record(record) + through_association = @owner.send(:association_proxy, @reflection.through_reflection.name) + attributes = construct_join_attributes(record) + + through_record = Array.wrap(through_association.target).find { |candidate| + candidate.attributes.slice(*attributes.keys) == attributes + } + + unless through_record + through_record = through_association.build(attributes) + through_record.send("#{@reflection.source_reflection.name}=", record) + end + + through_record + end + + def build_record(attributes) + record = super(attributes) + + inverse = @reflection.source_reflection.inverse_of + if inverse + if inverse.macro == :has_many + record.send(inverse.name) << through_record(record) + elsif inverse.macro == :has_one + record.send("#{inverse.name}=", through_record(record)) + end + end + + record + end + def target_reflection_has_associated_record? if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank? false @@ -79,6 +107,8 @@ module ActiveRecord count = scope.delete_all end + delete_through_records(through, records) + if @reflection.through_reflection.macro == :has_many && update_through_counter?(method) update_counter(-count, @reflection.through_reflection) end @@ -86,6 +116,18 @@ module ActiveRecord update_counter(-count) end + def delete_through_records(through, records) + if @reflection.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) + end + end + end + def find_target return [] unless target_reflection_has_associated_record? scoped.all diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 7e77539fe7..efdecd4b09 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -113,6 +113,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted") end + def test_build_then_save_with_has_many_inverse + post = posts(:thinking) + person = post.people.build(:first_name => "Bob") + person.save + post.reload + + assert post.people.include?(person) + end + + def test_build_then_save_with_has_one_inverse + post = posts(:thinking) + person = post.single_people.build(:first_name => "Bob") + person.save + post.reload + + assert post.single_people.include?(person) + end + def test_delete_association assert_queries(2){posts(:welcome);people(:michael); } diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index a18b9e44df..cc3a4f5f9d 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -1,5 +1,7 @@ class Person < ActiveRecord::Base has_many :readers + has_one :reader + has_many :posts, :through => :readers has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null' diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 1bbcab3a39..a342aaf60b 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -95,6 +95,7 @@ class Post < ActiveRecord::Base has_many :readers has_many :readers_with_person, :include => :person, :class_name => "Reader" has_many :people, :through => :readers + has_many :single_people, :through => :readers has_many :people_with_callbacks, :source=>:person, :through => :readers, :before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) }, :after_add => lambda {|owner, reader| log(:added, :after, reader.first_name) }, diff --git a/activerecord/test/models/reader.rb b/activerecord/test/models/reader.rb index 27527bf566..0207a2bd92 100644 --- a/activerecord/test/models/reader.rb +++ b/activerecord/test/models/reader.rb @@ -1,4 +1,5 @@ class Reader < ActiveRecord::Base belongs_to :post - belongs_to :person + belongs_to :person, :inverse_of => :readers + belongs_to :single_person, :class_name => 'Person', :foreign_key => :person_id, :inverse_of => :reader end |