[Django] #37327: Improve IncorrectLookupParameters admin error message

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------+-----------------------------------------
Reporter: Tim Schilling | Type: Uncategorized
Status: new | Component: contrib.admin
Version: dev | Severity: Normal
Keywords: messaging | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+-----------------------------------------
This is based on leunga1000's [https://github.com/django/django/pull/15464
PR]. The `ModelAdmin.changelist_view`'s handling of
`IncorrectLookupParameters` presents a page title of "Database error" for
the particular exception of `IncorrectLookupParameters`. Since the user is
already past authentication and authorization, we should give them more
information that the querystring had an error. I would suggest "Incorrect
lookup" or "Incorrect lookup in query string" depending on how technical
we want to get.

I agree with leunga1000's assessment that this message is a bit cryptic
and think we should make a change.
--
Ticket URL: <https://code.djangoproject.com/ticket/37327>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37327: Improve IncorrectLookupParameters admin error message

-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:

Keywords: messaging | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Tim Schilling):

* type: Uncategorized => Cleanup/optimization

--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:1>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------

Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned

Component: contrib.admin | Version: dev
Severity: Normal | Resolution:

Keywords: messaging | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Md. Saikat Islam):

* owner: (none) => Md. Saikat Islam
* stage: Unreviewed => Accepted
* status: new => assigned

Comment:

Seems valid issue. The error message should be improved. I am taking it.
--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:2>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Md. Saikat Islam):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:3>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Md. Saikat Islam):

PR is ready for this ticket: https://github.com/django/django/pull/21899
--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:4>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by blighj):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:5>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

I'm having a a [https://en.wiktionary.org/wiki/Chesterton%27s_fence
Chesterton's Fence] doubt here. The code comment says "the 'invalid=1'
parameter was already in the query string", so does anyone know why this
was regarded as a database error before?
--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:6>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Md. Saikat Islam):

the `changelist_view` uses creates a `get_changelist_instance` and that
method calls many methods inside ChangeList class in `view/main.py`. I
inspected that, inside these two files most of the times we have raised
`IncorrectLookupParameters` error, for e.g `FieldDoesNotExist`,
`ValueError`.

Heres my anaylisis on this, when we first catch the
IncorrectLookupParameters we consider it the users fault, and pass a flag
(value is actually 'e=' in code, the comment is stale to have 'invalid='.
And after a fresh redirect, when we again has IncorrectLookupParameters
error, we are sure that, its not problem in request params, its maybe our
raised errors.

So, I am now confidence that the "database error" is not correct. Best, we
use "Server error" in when we have error after the flag set.
--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:7>

[Django]

#37327: Improve IncorrectLookupParameters admin error message
-------------------------------------+-------------------------------------
Reporter: Tim Schilling | Owner: Md.
Type: | Saikat Islam
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: messaging | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* easy: 1 => 0
* needs_tests: 0 => 1

Comment:

Saikat has done a great job being responsive to feedback. On their PR, I
just requested a test case that shows the error condition that would
actually render this 20-year old template. If we can't find one, we should
close as `needsinfo` or possibly repurpose this ticket to remove the
template (which is [https://djangoci.com/view/%C2%ADCoverage/job/django-
coverage/HTML_20Coverage_20Report/z_7966ed225253a4ca_options_py.html#t2342
untested]).

Without knowing if the error condition is realistic we can't take a
decision about how careful to be about invalidating translations.
--
Ticket URL: <https://code.djangoproject.com/ticket/37327#comment:8>