Permalink
Browse files

Destroy association habtm record before destroying the record itself.…

… Fixes issue #402.
  • Loading branch information...
1 parent dfec373 commit ea4b94a7bf707d27af44701fa44ef5f916b84e01 Tomas D'Stefano committed with jonleighton May 31, 2011
@@ -7,24 +7,22 @@ class HasAndBelongsToMany < CollectionAssociation #:nodoc:
def build
reflection = super
check_validity(reflection)
- define_after_destroy_method
+ define_destroy_hook
reflection
end
private
- def define_after_destroy_method
+ def define_destroy_hook
name = self.name
- model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
- def #{after_destroy_method_name}
- association(#{name.to_sym.inspect}).delete_all
- end
- eoruby
- model.after_destroy after_destroy_method_name
- end
-
- def after_destroy_method_name
- "has_and_belongs_to_many_after_destroy_for_#{name}"
+ model.send(:include, Module.new {
+ class_eval <<-RUBY, __FILE__, __LINE__ + 1
@sobrinho
sobrinho Jul 9, 2011 Contributor

Can be refactored to something like:

model.class_eval <<-RUBY, __FILE__, __LINE__ + 1
  # method
end
@sobrinho
sobrinho Jul 9, 2011 Contributor

GitHub goes crazy, duplicates the code but everybody will understand :)

@jonleighton
jonleighton Jul 9, 2011 Member

No, it's necessary to create an anonymous module so that super works correctly and hooks from other associations are therefore executed.

+ def destroy_associations
+ association(#{name.to_sym.inspect}).delete_all
+ super
+ end
+ RUBY
+ })
end
# TODO: These checks should probably be moved into the Reflection, and we should not be
@@ -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?
IdentityMap.remove(self) if IdentityMap.enabled?
pk = self.class.primary_key
@@ -284,6 +286,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
@@ -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
@@ -719,6 +719,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

0 comments on commit ea4b94a

Please sign in to comment.