[Django] #32480: Standardization of the exception message in permission_denied() + fixing the comments in views.defaults

[Django]

#32480: Standardization of the exception message in permission_denied() + fixing
the comments in views.defaults
------------------------------------------------+------------------------
Reporter: berycz | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Hello,
I [https://github.com/django/django/pull/14024 pulled a request on github]
to fix some things in django/views/defaults.py .
I was writing custom views for 400/403/404/500 because I needed them to
return TemplateResponse, so I looked into the default ones for inspiration
and I found some confusing things there:

**1)** the permission_denied() returns
{{{#!python
context={'exception': str(exception)}
}}}
while the page_not_found() uses a nicer
{{{#!python
exception_repr = exception.__class__.__name__
# Try to get an "interesting" exception message, if any (and not the
ugly
# Resolver404 dictionary)
try:
message = exception.args[0]
except (AttributeError, IndexError):
pass
else:
if isinstance(message, str):
exception_repr = message
context = {
'request_path': quote(request.path),
'exception': exception_repr,
}
}}}
I suppose someone just forgot to read the whole code when making some
changes, so it might be better to use the same code to get the exception
message...?

**2)** permission_denied() has in the function description:

{{{
Context: None
}}}
while it should be
{{{
Context:
exception
The message from the exception which triggered the 403 (if one
was
supplied), or the exception class name
}}}

**3)** There is twice this comment about csrf_token, before
page_not_found() and permission_denied()
{{{
# This can be called when CsrfViewMiddleware.process_view has not run,
# therefore need @requires_csrf_token in case the template needs
# {% csrf_token %}.
}}}
But you actually have the decorator ''@requires_csrf_token'' in front of
all the views, so it might kinda confuse people who read that.

Thanks for all the work,
with regards,
Bery

--
Ticket URL: <https://code.djangoproject.com/ticket/32480>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#32480: Outdated docstring in permission_denied() and redundant comments in default
error pages.
--------------------------------------+------------------------------------
Reporter: BeryCZ | Owner: BeryCZ
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):

* status: new => assigned
* cc: Claude Paroz (added)
* needs_better_patch: 0 => 1
* component: Uncategorized => Error reporting
* owner: nobody => BeryCZ
* has_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

Replying to [ticket:32480 BeryCZ]:
> **1)** the permission_denied() returns
> ...

> I suppose someone just forgot to read the whole code when making some
changes, so it might be better to use the same code to get the exception
message...?

That's not true, it was discussed in the original ticket #24733, see
[https://github.com/django/django/pull/4590#issuecomment-99842965
comments]. I don't see any reason to use the same special treatment for
403.

I agree with fixing comments and docstrings.

[https://github.com/django/django/pull/14024/ PR]

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

[Django]

#32480: Outdated docstring in permission_denied() and redundant comments in default
error pages.

-------------------------------------+-------------------------------------
Reporter: BeryCZ | Owner: BeryCZ
Type: | Status: assigned
Cleanup/optimization |

Component: Error reporting | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/32480#comment:2>

[Django]

#32480: Outdated docstring in permission_denied() and redundant comments in default
error pages.
-------------------------------------+-------------------------------------
Reporter: BeryCZ | Owner: BeryCZ

Type: | Status: closed

Cleanup/optimization |
Component: Error reporting | Version: master

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by GitHub <noreply@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"807226cd79dbe61a8b3ba9fdfae865c5685f0b42" 807226cd]:
{{{
#!CommitTicketReference repository=""
revision="807226cd79dbe61a8b3ba9fdfae865c5685f0b42"
Fixed #32480 -- Corrected docstring and removed redundant comments in
django/views/defaults.py.
}}}

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