django.contrib.admin.actions.delete_selected() should return number of rows deleted via ModelAdmin.delete_queryset()

[Wicher Minnaard]

Status quo

Currently, the django.contrib.admin.actions.delete_selected() Admin action ​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 display a correct count (conforming to the status quo semantics flavour (a)) 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 (a) to (b), 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.

[Mariusz Felisiak]

Thanks for the ticket, however delete_queryset() was added to change a deletion process not to change a list of objects to be deleted (as documented), e.g. use Model.delete() instead of a bulk deletion. IMO, any implicit queryset modification can be confusing for users and will brake other things as log_deletion(). I have doubts the described use case should be supported.

Please first start a discussion on the DevelopersMailingList, where you'll reach a wider audience and see what other think, and ​follow the guidelines with regards to requesting features.

[Wicher Minnaard]

Replying to Mariusz Felisiak:

Thanks for the ticket, however delete_queryset() was added to change a deletion process not to change a list of objects to be deleted (as documented), e.g. use Model.delete() instead of a bulk deletion. IMO, any implicit queryset modification can be confusing for users and will brake other things as log_deletion(). I have doubts the described use case should be supported.

Yes. I recognize that objection, it's discussed in the discussion section.
So solution 2 (the quick patch) is out the window.
Did you have any considerations for solution 1, which does not involve delete_queryset()?

Please first start a discussion on the DevelopersMailingList, where you'll reach a wider audience and see what other think, and ​follow the guidelines with regards to requesting features.

I'll consider, thanks!