From 0da426be96d9e02b1b7a34e364dff984cd4218f3 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 18 Aug 2006 07:35:07 +0000 Subject: [PATCH] Add records to has_many :through using <<, push, and concat by creating the association record. Raise if base or associate are new records since both ids are required to create the association. #build raises since you can't associate an unsaved record. #create! takes an attributes hash and creates the associated record and its association in a transaction. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4786 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 12 +++ .../lib/active_record/associations.rb | 6 ++ .../has_many_through_association.rb | 98 +++++++++++++------ .../test/associations_join_model_test.rb | 24 +++-- 4 files changed, 104 insertions(+), 36 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 47e7531c68..dbe7d81623 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,17 @@ *SVN* +* Add records to has_many :through using <<, push, and concat by creating the association record. Raise if base or associate are new records since both ids are required to create the association. #build raises since you can't associate an unsaved record. #create! takes an attributes hash and creates the associated record and its association in a transaction. [Jeremy Kemper] + + # Create a tagging to associate the post and tag. + post.tags << Tag.find_by_name('old') + post.tags.create! :name => 'general' + + # Would have been: + post.taggings.create!(:tag => Tag.find_by_name('finally') + transaction do + post.taggings.create!(:tag => Tag.create!(:name => 'general')) + end + * Cache nil results for :included has_one associations also. #5787 [Michael Schoen] * Fixed a bug which would cause .save to fail after trying to access a empty has_one association on a unsaved record. [Tobias Luetke] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 222bf6ca6a..8f45ce5428 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -38,6 +38,12 @@ module ActiveRecord end end + 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.") + end + end + class EagerLoadPolymorphicError < ActiveRecordError #:nodoc: def initialize(reflection) super("Can not eagerly load the polymorphic association #{reflection.name.inspect}") 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 8f4ac104ad..7662e31914 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -22,12 +22,12 @@ module ActiveRecord elsif @reflection.options[:order] options[:order] = @reflection.options[:order] end - + options[:select] = construct_select(options[:select]) options[:from] ||= construct_from options[:joins] = construct_joins(options[:joins]) options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? - + merge_options_from_reflection!(options) # Pass through args exactly as we received them. @@ -40,19 +40,41 @@ module ActiveRecord @loaded = false end + # Adds records to the association. The source record and its associates + # must have ids in order to create records associating them, so this + # will raise ActiveRecord::HasManyThroughCantAssociateNewRecords if + # either is a new record. Calls create! so you can rescue errors. def <<(*args) - raise ActiveRecord::ReadOnlyAssociation.new(@reflection) + return if args.empty? + through = @reflection.through_reflection + raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) if @owner.new_record? + + klass = through.klass + klass.transaction do + args.each do |associate| + raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record? + klass.with_scope(:create => construct_association_attributes(through)) { klass.create! } + end + end end - [:push, :concat, :create, :build].each do |method| - alias_method method, :<< + [:push, :concat].each { |method| alias_method method, :<< } + + def build(attrs = nil) + raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, @reflection.through_reflection) end - + + def create!(attrs = nil) + @reflection.klass.transaction do + self << @reflection.klass.with_scope(:create => attrs) { @reflection.klass.create! } + end + end + # Calculate sum using SQL, not Enumerable def sum(*args, &block) calculate(:sum, *args, &block) end - + protected def method_missing(method, *args, &block) if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) @@ -63,12 +85,12 @@ module ActiveRecord end def find_target - records = @reflection.klass.find(:all, + records = @reflection.klass.find(:all, :select => construct_select, :conditions => construct_conditions, :from => construct_from, :joins => construct_joins, - :order => @reflection.options[:order], + :order => @reflection.options[:order], :limit => @reflection.options[:limit], :group => @reflection.options[:group], :include => @reflection.options[:include] || @reflection.source_reflection.options[:include] @@ -77,27 +99,44 @@ module ActiveRecord @reflection.options[:uniq] ? records.to_set.to_a : records end - 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}" + def construct_association_attributes(reflection) + if as = reflection.options[:as] + { "#{as}_id" => @owner.id, + "#{as}_type" => @owner.class.base_class.name.to_s } else - "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}" + { reflection.primary_key_name => @owner.id } end - conditions << " AND (#{sql_conditions})" if sql_conditions - - return conditions + end + + def construct_quoted_association_attributes(reflection) + if as = reflection.options[:as] + { "#{as}_id" => @owner.quoted_id, + "#{as}_type" => reflection.klass.quote( + @owner.class.base_class.name.to_s, + reflection.klass.columns_hash["#{as}_type"]) } + else + { reflection.primary_key_name => @owner.quoted_id } + end + end + + def construct_conditions + table_name = @reflection.through_reflection.table_name + conditions = construct_quoted_association_attributes(@reflection.through_reflection).map do |attr, value| + "#{table_name}.#{attr} = #{value}" + end + conditions << sql_conditions if sql_conditions + "(" + conditions.join(') AND (') + ")" end def construct_from @reflection.table_name end - + def construct_select(custom_select = nil) - selected = custom_select || @reflection.options[:select] || "#{@reflection.table_name}.*" + 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 @@ -120,14 +159,15 @@ module ActiveRecord polymorphic_join ] end - + def construct_scope - { - :find => { :from => construct_from, :conditions => construct_conditions, :joins => construct_joins, :select => construct_select }, - :create => { @reflection.primary_key_name => @owner.id } - } + { :create => construct_association_attributes(@reflection), + :find => { :from => construct_from, + :conditions => construct_conditions, + :joins => construct_joins, + :select => construct_select } } end - + def construct_sql case when @reflection.options[:finder_sql] @@ -147,14 +187,14 @@ module ActiveRecord @counter_sql = @finder_sql end end - + def conditions @conditions ||= [ (interpolate_sql(@reflection.active_record.send(:sanitize_sql, @reflection.options[:conditions])) if @reflection.options[:conditions]), (interpolate_sql(@reflection.active_record.send(:sanitize_sql, @reflection.through_reflection.options[:conditions])) if @reflection.through_reflection.options[:conditions]) ].compact.collect { |condition| "(#{condition})" }.join(' AND ') unless (!@reflection.options[:conditions] && !@reflection.through_reflection.options[:conditions]) end - + alias_method :sql_conditions, :conditions end end diff --git a/activerecord/test/associations_join_model_test.rb b/activerecord/test/associations_join_model_test.rb index b5d48db8bc..508628f37a 100644 --- a/activerecord/test/associations_join_model_test.rb +++ b/activerecord/test/associations_join_model_test.rb @@ -363,14 +363,24 @@ class AssociationsJoinModelTest < Test::Unit::TestCase assert_nil posts(:thinking).tags.find_by_name("General").attributes["tag_id"] end - def test_raise_error_when_adding_to_has_many_through - assert_raise(ActiveRecord::ReadOnlyAssociation) { posts(:thinking).tags << tags(:general) } - assert_raise(ActiveRecord::ReadOnlyAssociation) { posts(:thinking).tags.push tags(:general) } - assert_raise(ActiveRecord::ReadOnlyAssociation) { posts(:thinking).tags.concat tags(:general) } - assert_raise(ActiveRecord::ReadOnlyAssociation) { posts(:thinking).tags.build(:name => 'foo') } - assert_raise(ActiveRecord::ReadOnlyAssociation) { posts(:thinking).tags.create(:name => 'foo') } + def test_raise_error_when_adding_new_record_to_has_many_through + assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags << tags(:general).clone } + assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).clone.tags << tags(:general) } + assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags.build } end - + + def test_create_associate_when_adding_to_has_many_through + count = Tagging.count + assert_nothing_raised { posts(:thinking).tags << tags(:general) } + assert_equal(count + 1, Tagging.count) + + assert_nothing_raised { posts(:thinking).tags.create!(:name => 'foo') } + assert_equal(count + 2, Tagging.count) + + assert_nothing_raised { posts(:thinking).tags.concat(Tag.create!(:name => 'abc'), Tag.create!(:name => 'def')) } + assert_equal(count + 4, Tagging.count) + end + def test_has_many_through_sum_uses_calculations assert_nothing_raised { authors(:david).comments.sum(:post_id) } end