#33298: get_object_or_404 / get_list_or_404 don't document/test *args
------------------------------------------------+------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
During code review I came across the following (slightly redacted)
snippet:
{{{
doc = get_object_or_404(
Document.objects.all(),
Q(is_public=True) | Q(owner=request.user.id),
id=data['document_id']
)
}}}
This looked odd to me, but the accompanying tests prove that it works. I
wanted to comment that the first argument could just be `Document` but
just to make sure I headed to the Django documentation of
[https://docs.djangoproject.com/en/3.2/topics/http/shortcuts/#get-object-
or-404 get_object_or_404].
To my surprise the usage of `*args` is not documented. While the function
signature is shown as `(klass, *args, **kwargs)`, only the `klass` and
`**kwargs` arguments are documented and explained (same goes for
[https://docs.djangoproject.com/en/3.2/topics/http/shortcuts/#get-list-
or-404 get_list_or_404]).
I then dug a bit deeper and looked at the
[https://github.com/django/django/blob/69b0736fad1d1f0197409ca025b7bcdf5666ae62/tests/get_object_or_404/tests.py
tests]. There I can only see tests that exercise the `klass` and
`**kwargs` arguments, there are no tests that pass in `*args` or a
combination `*args` and `**kwargs`.
This made me look at the actual
[https://github.com/django/django/blob/69b0736fad1d1f0197409ca025b7bcdf5666ae62/django/shortcuts.py
implementation]. There the docstring state:
klass may be a Model, Manager, or QuerySet object. All other passed
arguments and keyword arguments are used in the get() query.
I then dug deeper, and found that in the end the initial code snippet is
converted to something like
`Document.objects.all().filter(Q(Q(is_public=True) |
Q(owner=request.user.id), id=data['document_id'])).get()`.
So, in conclusion, is passing `*args` to `get_object_or_404` and
`get_list_or_404` an expected use-case that should be tested/documented?
--
Ticket URL: <https://code.djangoproject.com/ticket/33298>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/048.c66828751ef0a7a6dbafd680e8377b75%40djangoproject.com.
