Changes between Initial Version and Version 2 of Ticket #16101


Ignore:
Timestamp:
May 30, 2011, 2:05:46 AM (13 years ago)
Author:
Julien Phalip
Comment:

(Fixed the ticket description's formatting).

Accepting as this seems like a simple and reasonable way of increasing SingleObjectMixin's flexibility. Like for any new feature, the patch will need to include tests and documentation.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #16101

    • Property Needs documentation set
    • Property Needs tests set
    • Property Triage Stage Unreviewed → Accepted

  • Ticket #16101 – Description

    initial v2  
    1 Currently, in SingleObjectMixin, Django looks for a 'pk' or 'slug' keyword argument to use for looking up an instance of a model. You can override slug_field when the field on the model is something other than "slug". What you can't do is designate different URL keywords to use for these lookup, and to override this in a subclass the whole get_object method needs to be reimplemented.
     1Currently, in `SingleObjectMixin`, Django looks for a 'pk' or 'slug' keyword argument to use for looking up an instance of a model. You can override slug_field when the field on the model is something other than "slug". What you can't do is designate different URL keywords to use for these lookup, and to override this in a subclass the whole get_object method needs to be reimplemented.
    22
    3 My proposition is to have a 'pk_keyword' and 'slug_keyword' field on SingleObjectMixin which default to 'pk' and 'slug' respectively, but can easily be overridden.
     3My proposition is to have a 'pk_keyword' and 'slug_keyword' field on `SingleObjectMixin` which default to 'pk' and 'slug' respectively, but can easily be overridden.
    44
    55Use case:
    66
    77I have a product app with a URL like this:
    8 
    9 /(r'^(?P<item_class_slug>[\w-]+)/(?P<item_slug>[\w-]*)-(?P<pk>\d+)/$'
    10 
    11 The view uses a subclass of DetailView with the pk keyword being used for fetching the product. But we also have optional product review URLs which are only included if the reviews app is installed, and are included like this:
    12 
    13 url(r'^(?P<item_class_slug>[\w-]+)/(?P<item_slug>[\w-]*)-(?P<pk>\d+)/', include(self.reviews_app.urls)),
    14 
    15 Within the reviews URLs we have a review detail view which also uses DetailView, this means I end up with two pk keywords in the URLs. Now I could just use a different keyword for the product pk keyword for this include, but I prefer consistency in my URL patterns. I would rather use 'item_pk' and 'review_pk' in all cases, which I can't currently do.
     8{{{#!python
     9r'^(?P<item_class_slug>[\w-]+)/(?P<item_slug>[\w-]*)-(?P<pk>\d+)/$'
     10}}}
     11The view uses a subclass of `DetailView` with the pk keyword being used for fetching the product. But we also have optional product review URLs which are only included if the reviews app is installed, and are included like this:
     12{{{#!python
     13url(r'^(?P<item_class_slug>[\w-]+)/(?P<item_slug>[\w-]*)-(?P<pk>\d+)/reviews', include(self.reviews_app.urls)),
     14}}}
     15Within the reviews URLs we have a review detail view which also uses `DetailView`, this means I end up with two pk keywords in the URLs. Now I could just use a different keyword for the product pk keyword for this include, but I prefer consistency in my URL patterns. I would rather use 'item_pk' and 'review_pk' in all cases, which I can't currently do.
    1616I've had a look at the code and it seems like a straightforward change, I'm happy to submit the patch myself if this issue is accepted.
Back to Top