Fix has_many :through delete with custom foreign keys. Closes #6466.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8043 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 20 insertions, 8 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b64b339781..e542e8d85a 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -49,6 +49,12 @@ module ActiveRecord
     end
   end
 
+  class HasManyThroughCantDissociateNewRecords < ActiveRecordError #:nodoc:
+    def initialize(owner, reflection)
+      super("Cannot dissociate 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 delete 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 5bb306c060..5ff0e3d0d7 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -71,18 +71,24 @@ module ActiveRecord
       def delete(*records)
         records = flatten_deeper(records)
         records.each { |associate| raise_on_type_mismatch(associate) }
-        records.reject! { |associate| @target.delete(associate) if associate.new_record? }
-        return if records.empty?
-        
-        @delete_join_finder ||= "find_all_by_#{@reflection.source_reflection.association_foreign_key}"
+
         through = @reflection.through_reflection
-        through.klass.transaction do
-          records.each do |associate|
-            joins = @owner.send(through.name).send(@delete_join_finder, associate.id)
-            @owner.send(through.name).delete(joins)
+        raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) if @owner.new_record?
+
+        load_target
+
+        klass = through.klass
+        klass.transaction do
+          flatten_deeper(records).each do |associate|
+            raise_on_type_mismatch(associate)
+            raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
+
+            @owner.send(through.name).proxy_target.delete(klass.delete_all(construct_join_attributes(associate)))
             @target.delete(associate)
           end
         end
+
+        self
       end
 
       def build(attrs = nil)