Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #2065 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

after_destroy and before_destroy broken if using has_and_belongs_to_many

Reported by: larrywilliams1@gmail.com Assigned to: David
Priority: normal Milestone:
Component: ActiveRecord Version: 0.13.1
Severity: major Keywords: fd
Cc:

Description

I have the following code:

has_and_belongs_to_many :categories

    def after_destroy
    	categories.each do |category| 
			Category.dosomething(category)
    	end
    end
    
    def after_update
    	categories.each do |category| 
			Category.dosomething(category)
    	end
    end

Everything works fine in after_update, the size of the categories array is correct if there are categories assigned to the object. But in after_destroy the size of the categories array is always 0. It's the same with before_destroy.

Change History

08/29/05 14:45:36 changed by sam.kirchmeier@gmail.com

In your example, the link between categories and the thing being destroyed no longer exists. The join table records are automatically deleted when the thing being destroyed is destroyed. This is correct behavior! You should move your code to the before_destroy callback to act on the join table before the object is destroyed.

This fix for your code will not work either, however, because the join table is being deleted as the first step in the before_destroy chain.

The code that causes this is in habtm on lines 560-561 of active_record/associations.rb:

before_destroy_sql = "DELETE FROM #{options[:join_table]} WHERE #{association_class_primary_key_name} = \\\#{self.quoted_id}"
module_eval(%{before_destroy "self.connection.delete(%{#{before_destroy_sql}})"}) # "

I believe that this behavior by default is incorrect. This code will delete your join table records before any before_destroy code you try to add. This code leaves you out of luck if you need to process any joined items at destroy-time.

I was able to solve this in my app by adding the following hack:

# force destroy_any_update_photos to be called first in the before_destroy queue
write_inheritable_attribute :before_destroy, read_inheritable_attribute(:before_destroy).insert(0, :destroy_and_update_photos)

def destroy_and_update_photos
  # destroy photos that are only in this album
  # update photos that are in multiple albums
  if self.photos_count > 0
    photo_ids = self.photos.map{ |photo| photo.id }.join(',')
    Photo.destroy_all "id IN (#{photo_ids}) AND albums_count = 1"
    Photo.update_all 'albums_count = albums_count - 1', "id IN (#{photo_ids}) AND albums_count > 1"
  end
end

08/29/05 14:54:30 changed by sam.kirchmeier@gmail.com

I forgot to mention that my suggestion for a fix in Rails is to change the habtm code on lines 560-561 of active_record/associations.rb to use after_destroy instead of before_destroy. Is there any problem with this?

It makes more sense that the records in the join table be deleted last. That way you can do whatever you need to with the join table in before_destroy, and the join table records pertaining to the destroyed object will still be gone after all is said and done.

08/30/05 05:52:48 changed by larrywilliams1@gmail.com

I noticed the same when looking at the log. The records in the join table are deleted first so there's no way of getting hold of the related records in after_destroy and before_destroy.

Anyway in my opinion nothing should be deleted before the before_destroy method is called. It shouldn't be called before_destroy then.

I fixed this in my code by moving the code outside of the before_destroy to the controller. It's a hack but works.

08/30/05 05:54:39 changed by anonymous

  • severity changed from critical to major.

09/04/05 15:28:28 changed by larrywilliams1@gmail.com

I tried to use write_inheritable_attribute but my code is never called.

10/12/05 18:10:15 changed by elliot@townx.org

I also ran across this problem. I fixed it in the model for my application like this:

class Quotation < ActiveRecord::Base  
  has_and_belongs_to_many :categories
  
  protected
  
  # after initialising, save a copy of the categories associated with the quotation;
  # this is used to clear categories after a quotation is deleted;
  # because after_destroy for quotation occurs after links to categories have
  # been removed
  def after_initialize
    @categories_copy = categories.clone
  end
  
  # purge the copy of the categories for a quotation
  def after_destroy
    @categories_copy.each do |cc|
      cc.purge
    end
  end
end

So I clone the categories array after the model object is initialised, which remains intact even after the records in the join table have been removed. Then I can run the purge method for each category before the quotation is destroyed; the purge method just automatically removes any categories which no longer have associated quotations (e.g. if the just-deleted quotation was the only quotation in the category, it will be deleted once the quotation has been deleted). However, when I tried to do a similar trick for the Category model (to ensure that a category cannot be removed if it has associated quotations), I got all sorts of errors, so I'm guessing there's some weird feedback thing going on here.

Anyway, I agree with previous posts: records in the join table should not be deleted until the primary record has been deleted. Presently, associations are lost before the before_destroy method runs, so you don't get to do any work on the associated records.

11/05/05 21:22:47 changed by david

  • keywords set to fd.

11/08/05 08:49:24 changed by bitsweat

This is a touchy one. The associated records must be destroyed *before* the parent record to maintain referential integrity. But your before_destroy callbacks should still work, so we need the critical before_destroy to run after all other before_destroy callbacks. Hmm..

11/08/05 10:19:13 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.

(In [2940]) Destroy associated has_and_belongs_to_many records after all before_destroy callbacks but before destroy. This allows you to act on the habtm association as you please while preserving referential integrity. Closes #2065.