#36217 closed Bug (fixed)
post_save signal is not called when LogEntry has Deletion action
Reported by: | smiling-watermelon | Owned by: | Antoliny |
---|---|---|---|
Component: | contrib.admin | Version: | 5.1 |
Severity: | Release blocker | Keywords: | LogEntry Signals post_save |
Cc: | smiling-watermelon, Akash Kumar Sen, David Wobrock, Nick Pope, Mariusz Felisiak | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've added a post_save signal for my application to show LogEntry entities in my logs, but have ran into the following problem:
If I remove some object from admin, LogEntry with Deletion action is created and stored in the database, but my post_save signal is not called.
I've ensured that other actions are working fine:
- Addition
- Change
So, for some reason only "Deletion" action doesn't cause the post_save
signal to be called.
To help reproduce the issue I've created a small repository on GitHub (basically following Django's documentation on creating a project), and added my signal there.
If you need any more info from me, just let me know, hopefully the README is pretty easy to follow and understand.
The exact Django version I've been using is 5.1.6, in both - the demo (though there it's not pinned!) and my real project.
Change History (11)
follow-up: 2 comment:1 by , 10 days ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 9 days ago
Replying to Sarah Boyce:
Thank you for the ticket!
I think this is a consequence of 40b3975e7d3e1464a733c69171ad7d38f8814280 (#34462), as we now use
bulk_create
which doesn't send apost_save
signal (see bulk_create docs).
The
single_object
kwarg, which decides to use.save()
(and send signals), is used forlog_addition
etc but never forlog_deletions
(even if this is only one object). I believe this is an inconsistency of behavior we might want to clean up.
I also think that we perhaps should/could document that these signals are no longer sent as part of the backwards incompatible changes of Django 5.1 (with maybe an alternative suggestion).
Note that it wasn't clear to me why we can't remove
single_object
kwarg entirely (I'm likely missing something):
So, does that mean that from now on, bulk_create
will always be used regardless of the number of objects being added or changed, and signals will never be triggered?
comment:3 by , 9 days ago
So, does that mean that from now on, bulk_create will always be used regardless of the number of objects being added or changed, and signals will never be triggered?
Right now, save()
, which sends the signals, is used when single_object=True
.
This is used by log_addition
and log_change
. This is not used by log_deletions
(which makes sense as it can have multiple objects).
But log_deletions
is called with a single object in django.contrib.admin.options.ModelAdmin._delete_view
. To me, it is inconsistent that we use save
and send signals in the other cases, but not here.
comment:4 by , 9 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:5 by , 9 days ago
Has patch: | set |
---|
comment:6 by , 9 days ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:7 by , 9 days ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
comment:8 by , 6 days ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thank you for the ticket!
I think this is a consequence of 40b3975e7d3e1464a733c69171ad7d38f8814280 (#34462), as we now use
bulk_create
which doesn't send apost_save
signal (see bulk_create docs).The
single_object
kwarg, which decides to use.save()
(and send signals), is used forlog_addition
etc but never forlog_deletions
(even if this is only one object). I believe this is an inconsistency of behavior we might want to clean up.I also think that we perhaps should/could document that these signals are no longer sent as part of the backwards incompatible changes of Django 5.1 (with maybe an alternative suggestion).
Note that it wasn't clear to me why we can't remove
single_object
kwarg entirely (I'm likely missing something):django/contrib/admin/models.py
single_object and log_entry_list: