#33263: DeleteView in Django4.0 does not call .delete() method
-------------------------------------+------------------------------------
Reporter: Eugene Prikazchikov | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Carlton Gibson):
OK, so the change here was intended, and doc'd but likely needs calling
out better in the release notes.
This was a deliberate change in #21936, finally resolved in
[https://github.com/django/django/pull/14634 PR 14634] after 8 years and
(I think) 4 previous PRs.
The mainline there was:
[https://github.com/django/django/pull/2585 PR 2585] to
[https://github.com/django/django/pull/13362 PR 13362] to
[https://github.com/django/django/pull/14634 PR 14634].
The PR adjusted the inheritance structure to use `FormMixin` for `post`,
allowing the confirmation step and success message. This is mentioned in
the generic view docs and the
[https://docs.djangoproject.com/en/4.0/releases/4.0/#generic-views Django
4.0 release notes].
The interplay with `DeletionMixin` complicated the issue significantly.
This was first picked up by
[https://code.djangoproject.com/ticket/21936#comment:9 Caroline Simpson
working on the original PR].
Happy to take input but after staring at it multiple times, for a long
time, the only stable conclusion was to separate the `post()` HTTP handler
from the `delete()` HTTP handler — proxying one to the other doesn't leave
space for the form and messaging behaviour. (See the historical PRs for
attempts in this direction.) Someone sending an API-like HTTP DELETE
method request gets the old behaviour here. It's only browser based POST
requests that are affected.
Continuing to proxy `post()` to `delete()`, as the PR here suggests, ends
up duplicating the `get_object()` and `get_success_url()` calls
(essentially from the two mixing, but inlined in this case because of the
tangled inheritance structure,
[https://github.com/django/django/pull/14634/files#diff-
bf5815bb9e60d6b9f1a261957863a70cc9ad03efdbd7941c0e1659b7ceb2895fR237-R240
see comment].) Doing that is less than ideal: with the added `FormMixin`
behaviour, `post()` is a much more complex handler than `delete()` —
they're no longer equivalent.
On the final PR
[https://github.com/django/django/pull/14634#discussion_r669325533 Mariusz
and I discussed adding an extra `_delete` hook, to be called by both
`delete()` and `form_valid()` but agreed is was too much API].
The correct approach is to implement your custom logic in `form_valid`,
and per any `FormMixin` subclass. Or **if** the view is genuinely handling
DELETE HTTP requests as well as POSTs, move the shared logic into a method
called by **both** `form_valid` and `delete` (which would be the hook we
decided not to add to the built-in CBV API).
I'd suggest a small addition to the release notes here, along the lines of
`"To accommodate this change custom logic in `delete()` methods should be
moved to `form_valid()` or a shared helper method as needed."`
I hope that makes sense. As I say, open to further input, but this seemed
the minimum change that satisfied the desiderata.
--
Ticket URL: <https://code.djangoproject.com/ticket/33263#comment:2>
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/066.1f8f01e91bf6639d236753226f30477b%40djangoproject.com.
