Opened 10 days ago

Closed 5 days ago

Last modified 5 days ago

#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)

comment:1 by Sarah Boyce, 10 days ago

Cc: Akash Kumar Sen David Wobrock Nick Pope Mariusz Felisiak added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thank you for the ticket!

I think this is a consequence of 40b3975e7d3e1464a733c69171ad7d38f8814280 (#34462), as we now use bulk_create which doesn't send a post_save signal (see bulk_create docs).

The single_object kwarg, which decides to use .save() (and send signals), is used for log_addition etc but never for log_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

    a b ACTION_FLAG_CHOICES = [  
    2424class LogEntryManager(models.Manager):
    2525    use_in_migrations = True
    2626
    27     def log_actions(
    28         self, user_id, queryset, action_flag, change_message="", *, single_object=False
    29     ):
     27    def log_actions(self, user_id, queryset, action_flag, change_message=""):
    3028        if isinstance(change_message, list):
    3129            change_message = json.dumps(change_message)
    3230
    class LogEntryManager(models.Manager):  
    4442            for obj in queryset
    4543        ]
    4644
    47         if single_object and log_entry_list:
     45        if len(log_entry_list) == 1:
    4846            instance = log_entry_list[0]
    4947            instance.save()
    5048            return instance

in reply to:  1 comment:2 by Antoliny, 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 a post_save signal (see bulk_create docs).

The single_object kwarg, which decides to use .save() (and send signals), is used for log_addition etc but never for log_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 Sarah Boyce, 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 Antoliny, 9 days ago

Owner: set to Antoliny
Status: newassigned

comment:5 by Antoliny, 9 days ago

Has patch: set

comment:6 by Antoliny, 9 days ago

Needs documentation: set
Needs tests: set

comment:7 by Antoliny, 9 days ago

Needs documentation: unset
Needs tests: unset

comment:8 by Sarah Boyce, 6 days ago

Triage Stage: AcceptedReady for checkin

comment:9 by Sarah Boyce <42296566+sarahboyce@…>, 5 days ago

Resolution: fixed
Status: assignedclosed

In c09bceef:

Fixed #36217 -- Restored pre_save/post_save signal emission via LogEntry.save() for single-object deletion in the admin.

Regression in 40b3975e7d3e1464a733c69171ad7d38f8814280.

Thanks smiling-watermelon for the report.

Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

comment:10 by Sarah Boyce <42296566+sarahboyce@…>, 5 days ago

In 5997fdc9:

[5.2.x] Fixed #36217 -- Restored pre_save/post_save signal emission via LogEntry.save() for single-object deletion in the admin.

Regression in 40b3975e7d3e1464a733c69171ad7d38f8814280.

Thanks smiling-watermelon for the report.

Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

Backport of c09bceef68e5abb79accedd12dade16aa6577a09 from main.

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 5 days ago

In 03ace75:

[5.1.x] Fixed #36217 -- Restored pre_save/post_save signal emission via LogEntry.save() for single-object deletion in the admin.

Regression in 40b3975e7d3e1464a733c69171ad7d38f8814280.

Thanks smiling-watermelon for the report.

Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

Backport of c09bceef68e5abb79accedd12dade16aa6577a09 from main.

Note: See TracTickets for help on using tickets.
Back to Top