From b763858ed5faeda720035dd2178e7c44aa34240a Mon Sep 17 00:00:00 2001 From: Gabe da Silveira Date: Sat, 8 Aug 2009 20:58:32 -0700 Subject: [PATCH] Enable has_many :through for going through a has_one association on the join model [#2719 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/associations.rb | 32 +++++++++++++++++-- .../has_many_through_association.rb | 2 +- activerecord/lib/active_record/reflection.rb | 2 +- .../has_many_associations_test.rb | 2 +- .../has_many_through_associations_test.rb | 12 +++++++ .../cases/associations/join_model_test.rb | 2 +- activerecord/test/models/author.rb | 1 + 7 files changed, 47 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7732bb535f972..ba603b021cb7f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -34,11 +34,13 @@ def initialize(reflection) end end - class HasManyThroughCantAssociateThroughHasManyReflection < ActiveRecordError #:nodoc: + class HasManyThroughCantAssociateThroughHasOneOrManyReflection < ActiveRecordError #:nodoc: def initialize(owner, reflection) super("Cannot modify association '#{owner.class.name}##{reflection.name}' because the source reflection class '#{reflection.source_reflection.class_name}' is associated to '#{reflection.through_reflection.class_name}' via :#{reflection.source_reflection.macro}.") end end + HasManyThroughCantAssociateThroughHasManyReflection = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection', 'ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection') + class HasManyThroughCantAssociateNewRecords < ActiveRecordError #:nodoc: def initialize(owner, reflection) super("Cannot associate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to create the has_many :through record associating them.") @@ -410,6 +412,32 @@ def association_instance_set(name, association) # @firm.clients.collect { |c| c.invoices }.flatten # select all invoices for all clients of the firm # @firm.invoices # selects all invoices by going through the Client join model. # + # Similarly you can go through a +has_one+ association on the join model: + # + # class Group < ActiveRecord::Base + # has_many :users + # has_many :avatars, :through => :users + # end + # + # class User < ActiveRecord::Base + # belongs_to :group + # has_one :avatar + # end + # + # class Avatar < ActiveRecord::Base + # belongs_to :user + # end + # + # @group = Group.first + # @group.users.collect { |u| u.avatar }.flatten # select all avatars for all users in the group + # @group.avatars # selects all avatars by going through the User join model. + # + # An important caveat with going through +has_one+ or +has_many+ associations on the join model is that these associations are + # *read-only*. For example, the following would not work following the previous example: + # + # @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 + # # === Polymorphic Associations # # Polymorphic associations on models are not restricted on what types of models they can be associated with. Rather, they @@ -759,7 +787,7 @@ module ClassMethods # [:through] # Specifies a Join Model through which to perform the query. Options for :class_name and :foreign_key # are ignored, as the association uses the source reflection. You can only use a :through query through a belongs_to - # or has_many association on the join model. + # has_one or has_many association on the join model. # [:source] # Specifies the source association name used by has_many :through queries. Only use it if the name cannot be # inferred from the association. has_many :subscribers, :through => :subscriptions will look for either :subscribers or 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 ac789e2e287d3..df2ef64fdcd04 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -96,7 +96,7 @@ def construct_owner_attributes(reflection) # Construct attributes for :through pointing to owner and associate. def construct_join_attributes(associate) # TODO: revist this to allow it for deletion, supposing dependent option is supported - raise ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection.new(@owner, @reflection) if @reflection.source_reflection.macro == :has_many + raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro) join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id) if @reflection.options[:source_type] join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2d4c1d5507c91..54b8c61245d5c 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -297,7 +297,7 @@ def check_validity! raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection) end - unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil? + unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil? raise HasManyThroughSourceAssociationMacroError.new(self) end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ee2e2e32efb96..9a5758f6eb20d 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -889,7 +889,7 @@ def test_modifying_a_through_a_has_many_should_raise lambda { authors(:mary).comments = [comments(:greetings), comments(:more_greetings)] }, lambda { authors(:mary).comments << Comment.create!(:body => "Yay", :post_id => 424242) }, lambda { authors(:mary).comments.delete(authors(:mary).comments.first) }, - ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection, &block) } + ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) } end def test_dynamic_find_should_respect_association_order_for_through 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 a8cdfc588e977..7453eb063a4a2 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -281,4 +281,16 @@ def test_find_on_has_many_association_collection_with_include_and_conditions post_with_no_comments = people(:michael).posts_with_no_comments.first assert_equal post_with_no_comments, posts(:authorless) end + + def test_has_many_through_has_one_reflection + assert_equal [comments(:eager_sti_on_associations_vs_comment)], authors(:david).very_special_comments + end + + def test_modifying_has_many_through_has_one_reflection_should_raise + [ + lambda { authors(:david).very_special_comments = [VerySpecialComment.create!(:body => "Gorp!", :post_id => 1011), VerySpecialComment.create!(:body => "Eep!", :post_id => 1012)] }, + lambda { authors(:david).very_special_comments << VerySpecialComment.create!(:body => "Hoohah!", :post_id => 1013) }, + lambda { authors(:david).very_special_comments.delete(authors(:david).very_special_comments.first) }, + ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) } + end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index b1060d01af7ed..d478180e666b6 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -377,7 +377,7 @@ def test_has_many_through_has_many_find_by_id end def test_has_many_through_polymorphic_has_one - assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).tagging } + assert_equal Tagging.find(1,2), authors(:david).tagging end def test_has_many_through_polymorphic_has_many diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index b844c7cce06ee..f264f980d6a6f 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -1,5 +1,6 @@ class Author < ActiveRecord::Base has_many :posts + has_many :very_special_comments, :through => :posts has_many :posts_with_comments, :include => :comments, :class_name => "Post" has_many :popular_grouped_posts, :include => :comments, :class_name => "Post", :group => "type", :having => "SUM(comments_count) > 1", :select => "type" has_many :posts_with_comments_sorted_by_comment_id, :include => :comments, :class_name => "Post", :order => 'comments.id'