model_instance.association.delete does not call after_destroy callback if the association is a :through #1962

Closed
mdesantis opened this Issue Jul 4, 2011 · 13 comments

Comments

Projects
None yet
6 participants
Contributor

mdesantis commented Jul 4, 2011

Hello,

let's say we execute this code:

rails new rails_test
cd rails_test
rails g model User
rails g model Project
rails g model UsersProject user:references project:references
rake db:migrate

and we customize our models like these:

app/models/user.rb

class User < ActiveRecord::Base
  has_many :users_projects
  has_many :projects, :through => :users_projects
end

app/models/project.rb

class Project < ActiveRecord::Base
  has_many :users_projects
  has_many :users, :through => :users_projects
end

app/models/users_project.rb

class UsersProject < ActiveRecord::Base
  belongs_to :user
  belongs_to :project

  after_create { |record| puts "after create" }
  after_destroy { |record| puts "after destroy" }
end

And now:

rails c
User.create
Project.create
UsersProject.create :user >> User.first, :project >> Project.first
[...] after create [...]
UsersProject.first.destroy
[...] after destroy [...]

all right. Now:

User.first.projects << Project.first
[...] after create [...]
User.first.projects.delete Project.first
[...][...]

The after_destroy callback is not called. I think this is not the expected result, also beacuse the after_create callback works.

I tested it on rails 2.3.11, 3.0.9, 3.1.0.rc4.

Owner

pixeltrix commented Jul 4, 2011

From api.rubyonrails.org:

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). Returns the frozen instance.

The row is simply removed with an SQL DELETE statement on the record’s primary key, 
and no callbacks are executed.

To enforce the object’s before_destroy and after_destroy callbacks, Observer methods, 
or any :dependent association options, use destroy.

Are you saying that they are called for delete on other association types?

Contributor

mdesantis commented Jul 4, 2011

No, I mean they are not called. I read this, in the has_many documentation:

http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html

collection.delete(object, …)
Removes one or more objects from the collection by setting their foreign keys to NULL. Objects will be in addition destroyed if they’re associated with :dependent => :destroy, and deleted if they’re associated with :dependent => :delete_all.

Ok, let's try:

rails c
User.create; Project.create
UsersProject.all # => []
User.first.projects << Project.first
[...] after create [...]
UsersProject.all # => [#<UsersProject id: 1, user_id: 1, project_id: 1, created_at: "2011-07-04 17:27:22", updated_at: "2011-07-04 17:27:22">]
User.first.projects.delete Project.first
[...][...]
UsersProject.all # => []

So, it does not set the foreign keys to NULL, it deletes the object (in contrast with the documentation assertion), and without calling the destroy callback.

You are referring to ActiveRecord::Relation.delete, that is different from the association.delete.

Member

jonleighton commented Jul 4, 2011

Please have a look at the 'Deleting from associations' section in http://edgeapi.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html. This behaviour is documented now, and is being kept as it is mainly for historical reasons and in order to mirror the habtm behaviour.

@jonleighton jonleighton closed this Jul 4, 2011

Contributor

mdesantis commented Jul 5, 2011

Ok, so, I added the :dependent => destroy to the has_many :through, and after a User.first.delete Project.first the callbacks are called in Rails 3.1.0.rc4, but not in 3.0.9. Does it mean that I have to use Rails 3.1 if I would make use of this behaviour?

Thank you :)

Member

jonleighton commented Jul 5, 2011

Yeah, the :dependent option on through associations is unsupported in the 3.0 branch.

costa commented Jul 6, 2011

I'm sorry, I don't think documenting a bug makes it less a bug, so I wouldn't close this issue this early.

foo.bars << baz  # triggers FooBar's after_create
foo.bars.delete baz  # does not trigger FooBar's after_destroy with any options to has_many :through

is a bug. It cannot be connected to the HABTM behaviour because the whole point of has_many :through vs has_and_belongs_to_many is being able to add validations and callbacks.
Also, the documentation currently says “...except for has_many :through, where the default strategy is delete_all (delete the join records, without running their callbacks)” without mentioning :dependent => :destroy will not work with :through at all.

I am aware of the workaround of before_remove, but it's keeps neither concerns separted, nor code optimal — in my case of polymorphic has_many :through I have to hook in each associated model.

Member

jonleighton commented Jul 6, 2011

@costa

You can do

foo.bars.destroy baz

which should fire the callbacks. Please let me know if it does not (on 3.1) and we will fix it.

Your assertion that :dependent => :destroy will not work is wrong for 3.1. It does not work in 3.0, but support has been added in 3.1.

As to changing the default :dependent behaviour to :destroy, personally I would have liked it to be like this in the first place, but to change it now would break backwards compatibility, which is why I have not done so.

costa commented Jul 6, 2011

Hmm, looks like a misunderstending.

foo.bars.destroy baz would destroy baz, not the FooBar join object. Is that also what you've meant with :dependent => :destroy, that it will destroy the associated object, not the association object?

I'm talking about callbacks of the join models, of course, like in the original sample code above.

Owner

pixeltrix commented Jul 6, 2011

This code should act as a workaround for 3.0:

foo.foo_bars.where(:bar_id => baz.id).destroy

A little bit ugly but it should only delete the FooBar objects and run the callbacks

Member

jonleighton commented Jul 6, 2011

@costa

I am going to use a more 'real world' example because I think it's less confusing. Also please note that I am describing the behaviour in 3.1 and not making any assertions about how 3.0 does or should act.

Suppose you have Post, Taggable and Tag. Post has many tags through taggable and vice versa.

If you do post.tags.destroy(tag), it will destroy the join record, and not the tag. If you want to destroy the tag (and not the join record), you can do tag.destroy. If you want to destroy both, you could add a dependency in Taggable, e.g.

class Taggable < AR::Base
  belongs_to :post
  belongs_to :tag, :dependent => :destroy
end

Then when you do post.tags.destroy(tag) it will destroy the join record, with the knock-on effect of destroying the tag.

I encourage you to download 3.1 and play around with how these things work. And also to read the documentation I linked to above - if you find issues or ambiguities with it please submit patches to docrails.

heaven commented Mar 15, 2012

I am having another situation. My models looks like in first example here:

class Porject < ActiveRecord::Base
  has_many :project_candidates, :dependent => :destroy, :class_name => "ProjectCandidate"
  has_many :candidates, :through => :project_candidates, :source => :profile
end

class Profile < ActiveRecord::Base
  has_many :candidacies, :dependent => :destroy, :class_name => "ProjectCandidate"
  has_many :projects, :through => :candidacies
end

class ProjectCandidate < ActiveRecord::Base
  belongs_to :project
  belongs_to :profile

  after_save :update_profile_index
  after_destroy :update_profile_index

  private

  def update_profile_index
    self.profile.index!
  end
end

In Project#edit html form I have f.association :candidates (model Profile). The problem is in that the update_profile_index method is called only on after_save and after_destroy is ignored. So all is fine when I am adding new candidates to the project, but how to be with deleting? I think for now delete method is used here, but probably should be destroy?

I got the same problem. I have a join class as so

class AssetableAsset < ActiveRecord::Base
  belongs_to :assetable, :polymorphic => true
  belongs_to :asset

  # Clean up orphans
  after_destroy do
    if asset.assetable_assets.count == 0
      asset.background_destroy
    end
  end
end

I want to clean up orphaned asset instances when the join count reaches zero. If I have an assetable
instance and an asset instance

assetable.assets.delete asset

causes the following SQL

DELETE FROM "assetable_assets" WHERE "assetable_assets"."assetable_id" = 11 
AND "assetable_assets"."assetable_type" = 'Event' AND "assetable_assets"."asset_id" = 268

or
assetable.assets.destroy asset

causes the following SQL

SELECT "assetable_assets".* FROM "assetable_assets" WHERE "assetable_assets"."assetable_id" = 11
AND "assetable_assets"."assetable_type" = 'Event' AND "assetable_assets"."asset_id" = 269

DELETE FROM "assetable_assets" WHERE "assetable_assets"."id" = ?  [["id", 3]]

but neither forces the after_remove callback on the AssetableAsset class to be invoked but the second form suggests it should be invoking the callbacks because it is first instantiating the object instances before destroying them, a two step process.

I got the following to work by changing my code to use a before_destroy method

class AssetableAsset < ActiveRecord::Base
  belongs_to :assetable, :polymorphic => true
  belongs_to :asset

  # Clean up orphans
  before_destroy do
    if asset.assetable_assets.count == 1
      asset.background_destroy
    end
  end
end

Doesn't this indicate a bug in after_destroy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment