From 4d232025b7260834dc4a4403b2b9effd043215c4 Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Wed, 5 Apr 2006 15:36:02 +0000 Subject: Added descriptive error messages for invalid has_many :through associations: going through :has_one or :has_and_belongs_to_many [Rick] Added support for going through a polymorphic has_many association: (closes #4401) [Rick] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4169 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 14 +++++++++++++ activerecord/lib/active_record/associations.rb | 12 +++++++++++ .../associations/has_many_through_association.rb | 23 ++++++++++++---------- activerecord/lib/active_record/reflection.rb | 4 ++++ activerecord/test/associations_join_model_test.rb | 16 +++++++++++++++ activerecord/test/fixtures/author.rb | 5 +++++ 6 files changed, 64 insertions(+), 10 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 4ffef99db6..dc61ab3c31 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,19 @@ *SVN* +* Added descriptive error messages for invalid has_many :through associations: going through :has_one or :has_and_belongs_to_many [Rick] + +* Added support for going through a polymorphic has_many association: (closes #4401) [Rick] + + class PhotoCollection < ActiveRecord::Base + has_many :photos, :as => :photographic + belongs_to :firm + end + + class Firm < ActiveRecord::Base + has_many :photo_collections + has_many :photos, :through => :photo_collections + end + * Multiple fixes and optimizations in PostgreSQL adapter, allowing ruby-postgres gem to work properly. [ruben.nine@gmail.com] * Fixed that AssociationCollection#delete_all should work even if the records of the association are not loaded yet. [Florian Weber] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3ad0eeecc7..5319fa39e8 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -44,6 +44,18 @@ module ActiveRecord end end + class HasManyThroughSourceAssociationMacroError < ActiveRecordError #:nodoc + def initialize(reflection) + @reflection = reflection + @through_reflection = reflection.through_reflection + @source_reflection = reflection.source_reflection + end + + def message + "Invalid source reflection macro :#{@source_reflection.macro}#{" :through" if @source_reflection.options[:through]} for has_many #{@reflection.name.inspect}, :through => #{@through_reflection.name.inspect}. Use :source to specify the source reflection." + end + end + class EagerLoadPolymorphicError < ActiveRecordError #:nodoc: def initialize(reflection) @reflection = reflection 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 dafae89861..44054b42e1 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -63,17 +63,12 @@ module ActiveRecord ) end - def construct_conditions + def construct_conditions conditions = if @reflection.through_reflection.options[:as] "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_id = #{@owner.quoted_id} " + "AND #{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_type = #{@owner.class.quote @owner.class.base_class.name.to_s}" else - case @reflection.source_reflection.macro - when :belongs_to, :has_many - "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}" - else - raise ActiveRecordError, "Invalid source reflection macro :#{@reflection.source_reflection.macro} for has_many #{@reflection.name}, :through => #{@reflection.through_reflection.name}. Use :source to specify the source reflection." - end + "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}" end conditions << " AND (#{sql_conditions})" if sql_conditions @@ -88,19 +83,27 @@ module ActiveRecord selected = custom_select || @reflection.options[:select] || "#{@reflection.table_name}.*" end - def construct_joins(custom_joins = nil) + def construct_joins(custom_joins = nil) + polymorphic_join = nil if @reflection.through_reflection.options[:as] || @reflection.source_reflection.macro == :belongs_to reflection_primary_key = @reflection.klass.primary_key source_primary_key = @reflection.source_reflection.primary_key_name else reflection_primary_key = @reflection.source_reflection.primary_key_name source_primary_key = @reflection.klass.primary_key + if @reflection.source_reflection.options[:as] + polymorphic_join = "AND %s.%s = %s" % [ + @reflection.table_name, "#{@reflection.source_reflection.options[:as]}_type", + @owner.class.quote(@reflection.through_reflection.klass.name) + ] + end end - "INNER JOIN %s ON %s.%s = %s.%s #{@reflection.options[:joins]} #{custom_joins}" % [ + "INNER JOIN %s ON %s.%s = %s.%s %s #{@reflection.options[:joins]} #{custom_joins}" % [ @reflection.through_reflection.table_name, @reflection.table_name, reflection_primary_key, - @reflection.through_reflection.table_name, source_primary_key + @reflection.through_reflection.table_name, source_primary_key, + polymorphic_join ] end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 40a5e5f2d1..d2c5393b59 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -176,6 +176,10 @@ module ActiveRecord if source_reflection.options[:polymorphic] raise HasManyThroughAssociationPolymorphicError.new(class_name, self, source_reflection) end + + unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil? + raise HasManyThroughSourceAssociationMacroError.new(self) + end end end diff --git a/activerecord/test/associations_join_model_test.rb b/activerecord/test/associations_join_model_test.rb index f0ee2eb9e7..b7f0af173b 100644 --- a/activerecord/test/associations_join_model_test.rb +++ b/activerecord/test/associations_join_model_test.rb @@ -295,6 +295,22 @@ class AssociationsJoinModelTest < Test::Unit::TestCase assert_equal comments(:more_greetings), authors(:david).comments.find(2) end + def test_has_many_through_polymorphic_has_one + assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).tagging } + end + + def test_has_many_through_polymorphic_has_many + assert_equal [taggings(:welcome_general), taggings(:thinking_general)], authors(:david).taggings.uniq.sort_by { |t| t.id } + end + + def test_has_many_through_has_many_through + assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).tags } + end + + def test_has_many_through_habtm + assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).post_categories } + end + def test_eager_load_has_many_through_has_many author = Author.find :first, :conditions => ['name = ?', 'David'], :include => :comments, :order => 'comments.id' SpecialComment.new; VerySpecialComment.new diff --git a/activerecord/test/fixtures/author.rb b/activerecord/test/fixtures/author.rb index 1a9b6d788a..99142f6621 100644 --- a/activerecord/test/fixtures/author.rb +++ b/activerecord/test/fixtures/author.rb @@ -31,6 +31,11 @@ class Author < ActiveRecord::Base has_many :author_favorites has_many :favorite_authors, :through => :author_favorites, :order => 'name' + has_many :tagging, :through => :posts # through polymorphic has_one + has_many :taggings, :through => :posts, :source => :taggings # through polymorphic has_many + has_many :tags, :through => :posts # through has_many :through + has_many :post_categories, :through => :posts, :source => :categories + belongs_to :author_address attr_accessor :post_log -- cgit v1.2.3