Replace CBV lookup arguments with single `lookup_field` argument.

[Tom Christie]

As discussed on this thread: ​https://groups.google.com/forum/?fromgroups=#!topic/django-developers/ht6Xq2ytPe4.

SingleObjectMixin currently exposes these three attributes and one method all dealing with queryset filter arguments...

• pk_url_kwarg = 'pk'
• slug_field = 'slug'
• slug_url_kwarg = 'slug'
• get_slug_field()

I was wondering if it would be worth considering simplifying the API somewhat, by moving those three attributes, and that method, into a PendingDeprecation state, and replacing their usage with a single attribute instead, that is used both as the URL kwarg, and as the queryset filter.

• lookup_field = 'pk'

That'd (eventually) lead to a simpler get_object implementation internally, and (immediately) present a slightly nicer, simpler API.

It'd be marginally different in that slug based lookups would require an explicit lookup_field = 'slug' to be added to the view,
and also in that it wouldn't handle the case of slug_field being set to a different value from slug_url_kwarg.

Personally I don't see the later being an issue as:

1. It's not clear to me why you'd ever *need* to use a URL kwarg that's not the same as the filter arg.
2. If it's really something you need to do, then overriding get_object is still really simple, as well as being nice and explicit...

get_object(self, queryset):

if queryset is None:

queryset = self.get_queryset()

return get_object_or_404(queryset, ...) # whatever custom lookup behavior you need.

To me, the main trade-off seems to be - Is the resulting API enough of an improvement to be worth the change?

I'm more than happy to take on the work, but I guess this ticket might need to go through the design decision needed stage?

[Marc Tamlyn]

Gonna put in a +0 on this - it's a nicer API but I'm not sure it's worth changing things for. I've never had much problem just using something else as "slug", and it's nice that both "slug" and "pk" work automatically without any more configuration.

[HM]

I usually wind up with long, complex urls and looking up more than one thing actually, so would prefer something like:

# key: kwarg, value: model field

lookups = {
    'pk': 'pk', 
    'slug': 'slug', 
    'otherkey': 'othermodel__serialno', 
    'owner': 'owner__username', 
    'year': 'year_published'
}

and then either a separate function to set them, run during dispatch(), or one dynamic function per item.

The default lookups would then be {'pk': 'pk', 'slug': 'slug'}

[Tom Christie]

If this gets some enthusiasm, then i think it'd be worthwhile, otherwise I'm also happy to WONTFIX this, on the assumption that the current lookup API is good enough.

Not keen on the dict based approach, if we're going to refactor the API I think it's only worthwhile if we're refactoring for simplicity.

[Aymeric Augustin]

Tom: after discussing with mjtamlyn at the DjangoCon sprints, as well as reading Andrew Ingram's comments on the mailing list, I'm rather in favor of simplifying the API.

The solution proposed in comment 2, as well as many others, can be achieved through subclassing.

[Aymeric Augustin]

Whoops, accidental change.

[Tom Christie]

Submitted Pull request, with fix, tests and docs.

​https://github.com/django/django/pull/1205

[anonymous]

I like simplifying this, but I don't think it's appropriate to force that the urlpattern name matches the model field name. We need two parameters here -- one is the urlconf name, the other is the model field. lookup_url_field and lookup_model_field, maybe. Assuming that the urlconf matches the model is inappropriate coupling.

[Tom Christie]

I don't agree that the loose coupling between URL conf and models is an issue. Specifying url kwargs that correspond correctly to the relevant model fields is a good thing. The arguments then make more sense when used in the corresponding reverse lookups. Not doing so creates confusion, eg if you have username based user lookup, but refer in the URL conf to the field as slug, then it's not obvious if the URLconf is in error, or if the disparity is intended.

[Marc Tamlyn]

Here's an example where it would be more confusing to *not* use a different name:

/<publisher>/
/<publisher>/<author>/
/<publisher>/<author>/<book>/

I think that is clearer in the urls.py than

/<slug>/
/<publisher>/<slug>/
/<publisher>/<author>/<slug>/

[Tom Christie]

Closing this as wontfix given no consensus