Opened 3 years ago

Closed 3 years ago

#19044 closed Cleanup/optimization (fixed)

Allow get_success_url to access self. object in DeletionMixin

Reported by: anonymous Owned by: charettes
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 charettes)

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 nxvl 3 years ago.
0001-Fixed-19044-Made-DeletionMixin-interpolate-it-s-suce.patch (4.6 KB) - added by charettes 3 years ago.
19044.2.diff (4.8 KB) - added by timo 3 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by ptone

  • Easy pickings set
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement unset
  • Summary changed from DeletionMixin: allow substitution in success_url to Allow get_success_url to access self. object in DeletionMixin
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization
  • Version changed from 1.4 to master

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 3 years ago by nxvl

comment:2 Changed 3 years ago by nxvl

  • Owner changed from nobody to nxvl
  • Status changed from new to assigned

comment:3 Changed 3 years ago by nxvl

  • Needs tests unset

comment:4 Changed 3 years ago by nxvl

  • Needs documentation unset

comment:5 Changed 3 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 3 years ago by ptone

  • 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 3 years ago by slurms

  • Needs tests unset
  • Patch needs improvement unset

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

comment:8 Changed 3 years ago by mjtamlyn

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 3 years ago by charettes

  • Description modified (diff)
  • Owner changed from nxvl to charettes

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

comment:10 Changed 3 years ago by charettes

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 3 years ago by charettes

  • Triage Stage changed from Ready for checkin to Accepted

Someone else should review the wording of the release note.

Changed 3 years ago by timo

comment:12 Changed 3 years ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

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

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

  • Resolution set to fixed
  • Status changed from assigned to closed

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