#34802: django.contrib.admin.actions.delete_selected(): return number of rows
deleted via ModelAdmin.delete_queryset()
-------------------------------------------+------------------------
Reporter: Wicher Minnaard | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
== Status quo
Currently, the `django.contrib.admin.actions.delete_selected()` Admin
action
[https://github.com/django/django/blob/4.2.4/django/contrib/admin/actions.py#L52-L57
will display a feedback message] via the Messages framework reporting on
how many objects have been deleted. But in reality the number displayed is
an estimate based on an earlier query (for the confirmation dialog) rather
than the actual number of objects eventually deleted.
Usually this is not a problem; it's a matter of the user's flavour of
semantics: do they expect to see '''(a)''' the count of objects that are
no more, regardless of whether ''they'' have been the one to delete them
(perhaps some got deleted concurrently)? Or do they expect to see
'''(b)''' the count of objects deleted by cause of their exact action and
by their exact action only?
There is no universally correct answer here, but I think we should assume
that the current behaviour of '''(a)''' should not be changed.
== Problem statement
However, when overriding `ModelAdmin.delete_queryset()` exactly to, well,
customize which objects will actually be deleted, it can result in
confusing inconsistencies for the user using the Admin. For instance, a
developer might override `delete_queryset()` to remove some of the
candidate objects to be deleted, and issue warning messages about which
objects have been skipped and why. An example flow of events:
1. User is on an admin listing and checkboxes 5 out of 5 objects to
delete. Then uses the "''Delete selected [...]''" admin action on them.
2. Django shows the interstitial "''Are you sure you want to delete
[...]''" page.
3. User confirms.
4. Overridden `ModelAdmin.delete_queryset()` is called. '''2''' out of the
'''5''' objects are excluded from deletion, and a warning message is added
using the Messages framework informing the user of such. Eventually
'''3''' objects get deleted in the overridden `delete_queryset()` method.
5. User is returned to the admin listing, which now shows the '''2'''
objects exempted from deletion from the original selection of all 5
objects. Above it, the Messages framework messages are displayed: the
warning message raised by the developer, but also a message
"''Successfully deleted 5 [...]''" which at that point is evidently
fiction to the user.
== Discussion
=== Developer should not override `delete_queryset()` to meddle with which
objects get deleted?
Per the documentation:
Override this method to customize the deletion process for the “delete
selected objects” action.
Whether "''not doing some of that deletion process''" falls under
"''customization''" is up for debate.
Also, what are the alternatives? Ideally the queryset for the interstitial
confirmation dialog would be easy to override, so that the developer could
raise the warning about excluding the two objects there and then, and as a
side effect the interstitial will display the correct candidate objects
right there, '''''and''''' in the subsequent "''Successfully deleted
[...]''" message.
== Proposed solutions
1. Make queryset for interstitial overridable as suggested above in
'''Discussion'''.
or
2. Let `delete_queryset()` optionally return a count of actually deleted
objects. Currently the method doesn't return anything, thus it is
straightforward to provide backward compatibility: if nothing (`None`) is
returned, the current behaviour persists, but if a count is returned, then
that count will go into the message raised. As a side effect, this will
flip the count semantics discussed under '''Status quo''' from '''(b)'''
to '''(a)''', which could be held against this solution.
I regard solution '''1''' as the superior, neatest solution, yet solution
'''2''' is just a quick patch away (patch attached as an example, I'm
happy to make a PR for it).
I can make a PR for solution '''1''' too if there is up-front concensus
that the goal is desirable.
--
Ticket URL: <https://code.djangoproject.com/ticket/34802>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018a41384cfc-f74c4b38-dd23-49da-a595-4d3e38695ab5-000000%40eu-central-1.amazonses.com.
