Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use default Django handlers for 400, 403, 404, and 500 errors #858

Merged
merged 2 commits into from
Mar 5, 2019

Conversation

Jkrzy
Copy link
Contributor

@Jkrzy Jkrzy commented Dec 28, 2018

Description

For #842, include a 400.html template and fallback to default Django error handlers.

Additional information

Current implementation for these handlers provides no functionality beyond what Django provides by default.

views.defaults.permission_denied for example, renders the 403.html template with the exception made available in the template context.

{% extends "base.html" %}

{% block content %}
<h2>Bad Request</h2>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's ping someone from content or writing lab and see if they have any suggestions on more user-friendly copy here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amandacostello can be of assistance. Looping in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amandacostello any update?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Still getting my feet under me with GitHub. You're looking for copy related to just 400 error returns, or also 403, 404, and 500?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, 400, but if there was general guidance for the others we'd be happy to incorporate that as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got some writing for each, though what usually triggers the 400 on the client side? Also, as a new GitHub person, how/where should I put the new content? Or just put it in a comment here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: Bad Request (400)

We can't process your request due to something wrong on your end, like a message being too large or (other common example seen in Tock)

Error: Access forbidden (403)

Your account doesn't have access. Make sure you're logged in, check your VPN connections, and that your account has the correct permissions enabled.

Error: File not found (404)

Looks like the file you're requesting isn't here anymore. Contact (name/email) or try (search)

Error: Internal Server Error (500)

Something's gone wrong on our side. Contact (name/email) to let them know.

handler400 = 'tock.views.handler400'
handler403 = 'tock.views.handler403'
handler404 = 'tock.views.handler404'
handler500 = 'tock.views.handler500'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the smell of red backgrounds in a pull request. Smells like... victory.

@tbaxter-18f
Copy link
Contributor

@Jkrzy Do you think we'll be able to get the content updates in for the March 5 release?

@Jkrzy
Copy link
Contributor Author

Jkrzy commented Mar 5, 2019

Thanks @amandacostello!! We're going to roll the content updates into a new issue and close this one, focused on the back-end implementation.

@codecov-io
Copy link

Codecov Report

Merging #858 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #858      +/-   ##
==========================================
+ Coverage   91.62%   91.66%   +0.03%     
==========================================
  Files          39       39              
  Lines        1684     1668      -16     
==========================================
- Hits         1543     1529      -14     
+ Misses        141      139       -2
Impacted Files Coverage Δ
tock/urls.py 90.9% <0%> (-1.4%) ⬇️
tock/views.py 81.25% <0%> (-0.9%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f342bf9...9c41938. Read the comment docs.

@tbaxter-18f tbaxter-18f merged commit aeb4a7e into master Mar 5, 2019
@tbaxter-18f tbaxter-18f deleted the handlers branch March 5, 2019 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants