Replace CBV lookup arguments with single `lookup_field` argument.
|Reported by:||tomchristie||Owned by:||tomchristie|
|Cc:||marc.tamlyn@…, tom@…||Triage Stage:||Accepted|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
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'
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:
- 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.
- If it's really something you need to do, then overriding get_object is still really simple, as well as being nice and explicit...
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?
Change History (12)
comment:1 Changed 2 years ago by mjtamlyn
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
comment:8 Changed 23 months ago by tomchristie
- Cc tom@… added
- Has patch set
- Version changed from 1.5 to master