Destroy association habtm record before destroying the record itself.…

… Fixes issue rails#402.
tardate · Jul 8, 2011 · 28f057c · 28f057c
1 parent e9f2c67
commit 28f057c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 10 deletions.
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
@@ -1411,7 +1411,7 @@ def has_and_belongs_to_many(association_id, options = {}, &extension)
         reflection = create_has_and_belongs_to_many_reflection(association_id, options, &extension)
         collection_accessor_methods(reflection, HasAndBelongsToManyAssociation)
 
-        configure_after_destroy_method_for_has_and_belongs_to_many(reflection)
+        configure_destroy_hook_for_has_and_belongs_to_many(reflection)
 
         add_association_callbacks(reflection.name, options)
       end
@@ -1705,15 +1705,16 @@ def #{method_name}
           end
         end
 
-        def configure_after_destroy_method_for_has_and_belongs_to_many(reflection)
-          method_name = :"has_and_belongs_to_many_after_destroy_for_#{reflection.name}"
-          class_eval <<-eoruby, __FILE__, __LINE__ + 1
-            def #{method_name}
-              association = #{reflection.name}
-              association.delete_all if association
-            end
-          eoruby
-          after_destroy method_name
+        def configure_destroy_hook_for_has_and_belongs_to_many(reflection)
+          include(Module.new {
+            class_eval <<-RUBY, __FILE__, __LINE__ + 1
+              def destroy_associations
+                association = #{reflection.name}
+                association.delete_all if association
+                super
+              end
+            RUBY
+          })
         end
 
         def delete_all_has_many_dependencies(record, reflection_name, association_class, dependent_conditions)

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
@@ -79,6 +79,8 @@ def delete
     # Deletes the record in the database and freezes this instance to reflect
     # that no changes should be made (since they can't be persisted).
     def destroy
+      destroy_associations
+
       if persisted?
         self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).delete_all
       end
@@ -245,6 +247,11 @@ def touch(name = nil)
     end
 
   private
+
+    # A hook to be overriden by association modules.
+    def destroy_associations
+    end
+
     def create_or_update
       raise ReadOnlyRecord if readonly?
       result = new_record? ? create : update

diff --git a/activerecord/test/cases/habtm_destroy_order_test.rb b/activerecord/test/cases/habtm_destroy_order_test.rb
@@ -16,6 +16,16 @@ class HabtmDestroyOrderTest < ActiveRecord::TestCase
     assert !sicp.destroyed?
   end
 
+  test 'should not raise error if have foreign key in the join table' do
+    student = Student.new(:name => "Ben Bitdiddle")
+    lesson = Lesson.new(:name => "SICP")
+    lesson.students << student
+    lesson.save!
+    assert_nothing_raised do
+      student.destroy
+    end
+  end
+
   test "not destroying a student with lessons leaves student<=>lesson association intact" do
     # test a normal before_destroy doesn't destroy the habtm joins
     begin

diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
@@ -666,6 +666,8 @@ def create_table(*args, &block)
     end
 
     execute "ALTER TABLE fk_test_has_fk ADD CONSTRAINT fk_name FOREIGN KEY (#{quote_column_name 'fk_id'}) REFERENCES #{quote_table_name 'fk_test_has_pk'} (#{quote_column_name 'id'})"
+
+    execute "ALTER TABLE lessons_students ADD CONSTRAINT student_id_fk FOREIGN KEY (#{quote_column_name 'student_id'}) REFERENCES #{quote_table_name 'students'} (#{quote_column_name 'id'})"
   end
 end