Opened 4 years ago

Closed 4 years ago

#19044 closed Cleanup/optimization (fixed)

Allow get_success_url to access self. object in DeletionMixin

Reported by: anonymous Owned by: Simon Charette
Component: Generic views Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Simon Charette)

Let's say I have object with parent set and once object is deleted, browser should be redirected to parent page.

Expected way to work is:

url(r'^object/(?P<pk>\d+)/delete/$', DeleteView.as_view(model=Object, success_url='/parent/%(parent_id)d/'), name='object_delete')

This can be achieved by evaluating success_url first and then deleting object:

class DeletionMixin(object):
    """
    A mixin providing the ability to delete objects
    """
    success_url = None

    def delete(self, request, *args, **kwargs):
        self.object = self.get_object()
        success_url = self.get_success_url()
        self.object.delete()
        return HttpResponseRedirect(success_url)

    # Add support for browsers which only accept GET and POST for now.
    def post(self, *args, **kwargs):
        return self.delete(*args, **kwargs)

    def get_success_url(self):
        if self.success_url:
            return self.success_url % self.object.__dict__
        else:
            raise ImproperlyConfigured(
                "No URL to redirect to. Provide a success_url.")

Attachments (3)

19044.diff (777 bytes) - added by Nicolas Valcárcel 4 years ago.
0001-Fixed-19044-Made-DeletionMixin-interpolate-it-s-suce.patch (4.6 KB) - added by Simon Charette 4 years ago.
19044.2.diff (4.8 KB) - added by Tim Graham 4 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by Preston Holmes

Easy pickings: set
Needs documentation: set
Needs tests: set
Patch needs improvement: unset
Summary: DeletionMixin: allow substitution in success_urlAllow get_success_url to access self. object in DeletionMixin
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization
Version: 1.4master

To simplify and clarify a bit:

the get_success_url should be able to see self.object when it is called - and so should happen before the object is deleted.

Seems completely reasonable, and should be backwards compatible.

see https://docs.djangoproject.com/en/dev/internals/contributing/ regarding patch format requirements

Changed 4 years ago by Nicolas Valcárcel

Attachment: 19044.diff added

comment:2 Changed 4 years ago by Nicolas Valcárcel

Owner: changed from nobody to Nicolas Valcárcel
Status: newassigned

comment:3 Changed 4 years ago by Nicolas Valcárcel

Needs tests: unset

comment:4 Changed 4 years ago by Nicolas Valcárcel

Needs documentation: unset

comment:5 Changed 4 years ago by anonymous

Attached patch doesn't address an issue properly.
Please take a look at pull request - https://github.com/django/django/pull/417

comment:6 Changed 4 years ago by Preston Holmes

Needs tests: set
Patch needs improvement: set

I have no idea why the % dict pattern was chosen - I'd prefer to make the code consistent for now - but not overtly document it.

pull is more complete than the attached patch - not sure if it is the same author - but if not please note the convention of letting one person work on a feature at a time.

Tests are needed - committer could take care of a brief release note


comment:7 Changed 4 years ago by Nick Sandford

Needs tests: unset
Patch needs improvement: unset

Added tests, pull request here: https://github.com/django/django/pull/663/.

comment:8 Changed 4 years ago by Marc Tamlyn

Triage Stage: AcceptedReady for checkin

comment:9 Changed 4 years ago by Simon Charette

Description: modified (diff)
Owner: changed from Nicolas Valcárcel to Simon Charette

Assigning to myself since I'm planning to commit this.

comment:10 Changed 4 years ago by Simon Charette

Added a new patch based on nxvl and slurms work with a release note.

Tests were rewritten because they were not failing without the DeletionMixin.delete fix since self.object.slug is still accessible after deletion. It's not the case with self.object.id.

Also fixed some badly indented code while I was there.

comment:11 Changed 4 years ago by Simon Charette

Triage Stage: Ready for checkinAccepted

Someone else should review the wording of the release note.

Changed 4 years ago by Tim Graham

Attachment: 19044.2.diff added

comment:12 Changed 4 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

I've tweaked the release note a bit and also added documentation of this in the docs.

comment:13 Changed 4 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: assignedclosed

In a10f3908042a71ec5ef81bf76f0f278ca5e7a596:

Fixed #19044 -- Made DeletionMixin interpolate its success_url.

Thanks to nxvl and slurms for the initial patch, ptone for the review
and timo for the documentation tweaks.

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