Refactor: class-based error views

[thekaveman]

Closes #3391

It turns out this was pretty straightforward, so instead of writing follow-up tickets to implement each of the error handlers, this PR just does it.

The Django error handlers (e.g. handler400) expect:

A callable... that should be called if the HTTP client has sent a request that caused an error condition and a response with a status code of 400.

For function-based views, we inherently have a callable to assign to these props (the function view itself). But this is also exactly the purpose of a CBV ViewClass.as_view() helper function:

Returns a callable view that takes a request and returns a response

So just like our standard URL path registrations for other CBVs that make use of the .as_view() helper, these error handlers can do the same thing!

There is a minor caveat for 2 of the error handlers:

• handler500 is very strict about the function signature, and .as_view() doesn't pass this strictness test
• CSRF_FAILURE_VIEW is defined in settings.py, where we don't want to import a view class (to avoid circular imports)

In both cases, we define a small helper function that simply wraps .as_view().

What this PR does

• Creates a base CBV for error handlers that:
  • Decorates dispatch with our common Middlewares for ViewedPageEvent and IndexOrAgencyIndexOrigin to reduce duplication
  • Sets a status_code attribute for the eventual HttpResponse
  • Inherits from TemplateView and optionally calculates the template_name from the status_code for easier inheritance
• Refactors all built-in Django error handler overrides to CBVs using the base CBV
• Moves all error handler CBVs to the top-level benefits.views
• Adds paths (in local only) for testing error handlers

Reviewing

1. Checkout this branch, rebuild and reopen the devcontainer
2. Start the app without debugging
3. Check that each of the following paths return the expected error template, and that the browser console shows the corresponding status code
   • /test400 (400)
   • /test403 (403)
   • /test404 (404)
   • /test500 (500)
   • /testcsrf (click to submit the form and trigger the handler - 403)

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits
settings.py
urls.py					57, 60, 63, 66, 69-71
views.py
benefits/core
middleware.py
urls.py
views.py
Project Total

This report was generated by python-coverage-comment-action

[lalver1]

This looks really good! While in general the required changes turned out to be relatively straightforward, there's a good bit going on in this PR (for example CSRF failures and Django specific requirements on the function signature of a view) so thanks for the notes 👍 they helped a lot with the review! I also tested locally and saw the expected templates/response codes.

I only had a general question about the templates that are shown for each type of HTTP error. For responses that return 400 and 403 errors, we return the 400.html template which alludes to the service for the site being down. I think the text in the template is meant to be generic to cover a bad request (400), a request for a forbidden resource (403), and a request that is rejected by the CSRF protection (403), but for most of these cases the issue would not originate with Benefits but with the client (browser) itself, correct?

[thekaveman]

For responses that return 400 and 403 errors, we return the 400.html template which alludes to the service for the site being down. I think the text in the template is meant to be generic

Yes I think this is correct, we wanted something more generic to let the user know something is wrong, without trying to diagnose exactly what might be wrong and provide potentially misleading or inaccurate information. This all came out of the error pages work that Sarah initially did (maybe 2 years ago? Or more...)

Since we didn't attempt change the actual language in this PR, I think we just leave everything as-is and perhaps review in another round with @indexing and @cmajel, maybe in concert with some ideas we've had around user-facing outage alerts/notifications.