Improve SuccessMessageMixin working with BaseDeleteView

[Chris Chapman]

Currently a developer is blocked from easily using values from the object before deleting it. This is because BaseDeleteView.form_valid is where the object is deleted, but this is called before constructing the success message. This could be easily improved if SuccessMessageMixin.form_valid method was slightly modified. The following suggestion just switches the order of the first two lines in the method:

    def form_valid(self, form):
        success_message = self.get_success_message(form.cleaned_data)
        response = super().form_valid(form)
        if success_message:
            messages.success(self.request, success_message)
        return response

This change would allow the following to work:

MyDeleteView(SuccessMessageMixin, DeleteView):
    """Delete object and give user feedback on success."""
    
    success_message = "Successfully deleted %(name)s"
    ...

    def get_success_message(self, cleaned_data):
        data = {**cleaned_data, "name": str(self.object)}
        return super().get_success_message(data)

Just as before, the message will only be applied if the call to super().form_valid(form) is successful.
This change would allow access to the object as typically expected with any class inheriting from SingleObjectMixin.

[Carlton Gibson]

Hi Chris.

I'm going to say wontfix here initially.

SuccessMessageMixin is ​essentially unchanged since it was introduced 9 years ago. In get_success_message() you still have self.object available, with everything but the pk (that was nulled during the delete() call):

>>> from django.contrib.auth.models import Group
>>> g = Group(name="Hello")
>>> g.save()
>>> g
<Group: Hello>
>>> g.id
1
>>> g.delete()
(1, {'auth.Group': 1})
>>> g.id
>>> g.name
'Hello'

Even the pk available from self.kwargs in the usual case, so you already have everything available.

The example you gave — data = {**cleaned_data, "name": str(self.object)} — I guess would be relying on the default __str__ implementation using the pk but you're already overriding get_success_message so there's no reason not to do something better, like using a .name for the name key, or implementing a __str__ that doesn't depend in pk, or using self.kwargs directly if that's what you really want.

As such, I can't see that it's worth the code change adding other ways to achieve the same thing.
I hope that makes sense.
Happy to consider if there's a use-case that can't be addressed in this way.