#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.

-- 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/049.f06d500e70eccce13cbee38b206fab33%40djangoproject.com.

Reply via email to