Code

Opened 5 years ago

Closed 3 years ago

#10754 closed Cleanup/optimization (fixed)

post and pre_delete signal documentation imprecise

Reported by: runeh Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords: signals
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The documentation claims that the pre and post_delete signals is "Sent before or after a model’s delete() method is called." ( http://docs.djangoproject.com/en/dev/topics/signals/ ). This is not the case when QuerySet.delete() is used to delete multiple objects. The signals are sent, but the object.delete() method is not called. One case where this happens is in the admin, when selecting multiple objects to delete.

Unsure if this is a documentation bug or if the docs are correct and the behaviour when using QuerySet.delete() is wrong.

Attachments (0)

Change History (7)

comment:1 Changed 5 years ago by kmtracey

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

The fact that the individual model delete() methods are not called is documented:

http://docs.djangoproject.com/en/dev/topics/db/queries/#deleting-objects

"Keep in mind that this will, whenever possible, be executed purely in SQL, and so the delete() methods of individual object instances will not necessarily be called during the process. If you've provided a custom delete() method on a model class and want to ensure that it is called, you will need to "manually" delete instances of that model (e.g., by iterating over a QuerySet and calling delete() on each object individually) rather than using the bulk delete() method of a QuerySet."

If the individual detele() methods are not called, it follows that the associated signal handlers won't be either.

#10751 is already open for the issue of bulk delete in admin bypassing individual instance methods. I think where doc is missing is for admin actions: a note about how, by default, bulk delete in admin will use queryset delete() and therefore bypass instance method customizations, and an indication of how to change this behavior (provide your own customized delete action), if you always want your individual methods called.

comment:2 Changed 5 years ago by runeh

You say "If the individual detele() methods are not called, it follows that the associated signal handlers won't be either.". That's not what the code does. When doing bulk deletes the signal handlers _are_ called. http://code.djangoproject.com/browser/django/trunk/django/db/models/query.py#L981

The signals docs says that pre and post delete signal handlers are called before and after calling delete(). In the case of bulk deletes, the signal handlers are called, but delete() is not called.

comment:3 Changed 5 years ago by kmtracey

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Ah, sorry I misread the original description (I thought it was saying that signal handlers were NOT being called) and didn't check the code. Agreed the signals doc here is misleading, it reads like the sending of the signal is tied to the instance's delete() method. If signals are sent even for bulk delete operations then that should probably be noted in the signals doc, and in the brief scan I have time for at the moment I don't see it.

comment:4 Changed 5 years ago by SmileyChris

  • Needs documentation set
  • Triage Stage changed from Unreviewed to Accepted

If kmtracey agrees, I'm calling it accepted ;)

comment:5 Changed 4 years ago by SashaN

I'm not sure whether I'm hitting bug or it is just lack of documentation, therefore I'm adding my note here.
I'm using django 1.1.1. I have two models, Event and EventBooking with ForeignKey to Event.

I'm using post_delete() signal for EventBooking. the signal handler will just sent email
to Event owner particular booking has been canceled.

I've hit a problem when Event owner canceled (deleted) Event instance, which had non-empty
eventbooking_set(). Once Event instance was being deleted, the EventBooking instances were
deleted sending Event owner email the particular booking is being canceled. To suppress
this odd behaviour I've decided to introduce no_email BooleanField into EventBooking.

The 'no_email' is False on default. It is set to True in pre_delete() event bound
to Event. Once Event instance is being deleted the pre_delete() signal is emitted.
My signal handler will set no_email field in all EventBooking instances in evenbooking_set
to True. It will also save each EventBooking instance in eventbooking_set.

The post_delete() signal handler bound to EventBooking checks no_email flag. if no_email is
False it will notify Event owner the booking is being canceled, if no_email is True it
will send no email.

Unfortunately my cludgy solution does not work. It looks like EventBooking instances
are not updated pre_remove() hook.

Is it expectable behaviour?

Also it is good idea to send emails in signal processing?
or is rather misusing signals?

comment:6 Changed 3 years ago by SmileyChris

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:7 Changed 3 years ago by SmileyChris

  • Resolution set to fixed
  • Status changed from reopened to closed

In [16019]:

Fixes #10754 - minor clarification to the post/pre_delete signal documentation

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.