Ticket #6466: fix_add_and_delete_for_has_many_through.diff

test/associations/join_model_test.rb

@@ -6 +6 @@
-require 'fixtures/author'
-require 'fixtures/category'
-require 'fixtures/categorization'
+require 'fixtures/book'
+require 'fixtures/citation'
-
-class AssociationsJoinModelTest < Test::Unit::TestCase
-  self.use_transactional_fixtures = false
-
+, :books
-
-  def test_has_many
-    assert authors(:david).categories.include?(categories(:general))
@@ -424 +426 @@
-    tags = posts(:thinking).tags
-    assert_equal tags, posts(:thinking).tags.push(tags(:general))
-  end
+  
+  def test_delete_associate_when_deleting_from_has_many_through_with_non_standard_id
+    count = books(:awdr).references.count
+    references_before = books(:awdr).references
+    book = Book.create!(:name => 'Getting Real')
+    book_awdr = books(:awdr)
+    book_awdr.references << book
+    assert_equal(count + 1, book_awdr.references(true).size)
-
+    assert_nothing_raised { book_awdr.references.delete(book) }
+    assert_equal(count, book_awdr.references.size)
+    assert_equal(count, book_awdr.references(true).size)
+    assert_equal(references_before.sort, book_awdr.references.sort)
+  end
+  
-  def test_delete_associate_when_deleting_from_has_many_through
-    count = posts(:thinking).tags.count
-    tags_before = posts(:thinking).tags

test/fixtures/citation.rb

@@ +1 @@
+class Citation < ActiveRecord::Base
+  belongs_to :reference_of, :class_name => "Book", :foreign_key => :book2_id 
+  belongs_to :book1, :class_name => "Book", :foreign_key => :book1_id 
+  belongs_to :book2, :class_name => "Book", :foreign_key => :book2_id 
+
+end

test/fixtures/db_definitions/schema.rb

@@ -57 +57 @@
-  create_table :lock_with_custom_column_without_defaults, :force => true do |t|
-    t.column :custom_lock_version, :integer
-  end
+  
+  create_table :books, :force => true do |t|
+    t.column :name, :string
+  end
+  
+  create_table :citations, :id => false, :force => true do |t|
+    t.column :book1_id, :integer
+    t.column :book2_id, :integer
+  end
+  
-end

test/fixtures/book.rb

@@ +1 @@
+class Book < ActiveRecord::Base
+  has_many :citations, :foreign_key => 'book1_id'  
+  has_many :references, :through => :citations, :source => :reference_of, :uniq => true
+
+end

test/fixtures/books.yml

@@ +1 @@
+awdr:
+  id: 1
+  name: "Agile Web Development with Rails"
+
+rfr:
+  id: 2
+  name: "Ruby for Rails"

lib/active_record/associations/has_many_through_association.rb

@@ -71 +71 @@
-
-      # Remove +records+ from this association.  Does not destroy +records+.
-      def delete(*records)
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
-
+         load_target
+
+         klass = through.klass
+         klass.transaction do
+           flatten_deeper(records).each do |associate|
+             raise_on_type_mismatch(associate)
+             raise ActiveRecord::HasManyThroughCantDisassociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
+
+             @owner.send(@reflection.through_reflection.name).proxy_target.delete(klass.delete_all(construct_join_attributes(associate)))
+             @target.delete(associate)
+           end
+         end
+
+         self  
+       end
+
-      def build(attrs = nil)
-        raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, @reflection.through_reflection)
-      end
@@ -138 +143 @@
-
-        # Construct attributes for :through pointing to owner and associate.
-        def construct_join_attributes(associate)
-association_foreign_key
+primary_key_name
-        end
-
-        # Associate attributes pointing to owner, quoted.

lib/active_record/associations.rb

@@ -44 +44 @@
-    end
-  end
-
+  class HasManyThroughCantDisassociateNewRecords < ActiveRecordError #:nodoc:
+    def initialize(owner, reflection)
+      super("Cannot disassociate 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}")