From 6013d93182fe6ba86e275a247139fc58d2fad783 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 26 Sep 2019 17:22:04 -0400 Subject: [PATCH 1/5] Styling error page --- atst/routes/errors.py | 20 ++++++++++++++++-- styles/atat.scss | 1 + styles/components/_error_page.scss | 33 ++++++++++++++++++++++++++++++ templates/error.html | 30 ++++++++++++++++----------- templates/error_base.html | 4 ++++ translations.yaml | 4 ++++ 6 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 styles/components/_error_page.scss diff --git a/atst/routes/errors.py b/atst/routes/errors.py index cc3f5f21..36d19b34 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -11,6 +11,7 @@ from atst.domain.invitations import ( from atst.domain.authnid.crl import CRLInvalidException from atst.domain.portfolios import PortfolioError from atst.utils.flash import formatted_flash as flash +from atst.utils.localization import translate NO_NOTIFY_STATUS_CODES = set([404, 401]) @@ -25,10 +26,25 @@ def notify(e, message, code): current_app.notification_sender.send(message) -def handle_error(e, message="Not Found", code=404): +def handle_error( + e, + message=translate("errors.not_found"), + code=404, + submessage=translate("errors.not_found_sub"), + more_info=translate("common.lorem"), +): log_error(e) notify(e, message, code) - return render_template("error.html", message=message), code + return ( + render_template( + "error.html", + message=message, + code=code, + submessage=submessage, + more_info=more_info, + ), + code, + ) def make_error_pages(app): diff --git a/styles/atat.scss b/styles/atat.scss index 73d68a03..346b5d44 100644 --- a/styles/atat.scss +++ b/styles/atat.scss @@ -38,6 +38,7 @@ @import "components/usa_banner"; @import "components/dod_login_notice.scss"; @import "components/sticky_cta.scss"; +@import "components/error_page.scss"; @import "sections/login"; @import "sections/home"; diff --git a/styles/components/_error_page.scss b/styles/components/_error_page.scss new file mode 100644 index 00000000..45a8b8f5 --- /dev/null +++ b/styles/components/_error_page.scss @@ -0,0 +1,33 @@ +.error-page { + max-width: 475px; + margin: auto; + + .panel__heading { + text-align: center; + padding: $gap 0; + + hr { + width: 100%; + border: 1px solid $color-red; + } + + h1 { + margin-bottom: $gap; + } + } + + .panel__body { + padding: $gap * 2; + margin: 0; + } + + .icon { + @include icon-size(60); + } + + hr { + margin-bottom: $gap * 4; + width: 80%; + border: 0.5px solid $color-gray-light; + } +} diff --git a/templates/error.html b/templates/error.html index f58eac86..c3675e0e 100644 --- a/templates/error.html +++ b/templates/error.html @@ -1,19 +1,25 @@ {% extends "error_base.html" %} +{% from "components/icon.html" import Icon %} + {% block content %} -
- -{% if message %} -

{{ message }}

-{% else %} -

An error occurred.

-{% endif %} - -{% if g.current_user %} -

Return home.

-{% endif %} - +
+
+ {{ Icon('cloud', classes="icon--red icon--large")}} +
+

{{ code }} - {{ message }}

+

{{ submessage }}

+
+
+
+

+ {{ more_info }} +

+

+ More lorem +

+
{% endblock %} diff --git a/templates/error_base.html b/templates/error_base.html index efe3f113..253a62f8 100644 --- a/templates/error_base.html +++ b/templates/error_base.html @@ -13,6 +13,10 @@ {% block template_vars %}{% endblock %} + {% include 'components/usa_header.html' %} + + {% include 'navigation/topbar.html' %} +
diff --git a/translations.yaml b/translations.yaml index 509dc8c7..625870aa 100644 --- a/translations.yaml +++ b/translations.yaml @@ -55,6 +55,7 @@ common: disable: Disable edit: Edit email: Email + lorem: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. members: Members name: Name next: Next @@ -96,6 +97,9 @@ components: The https:// ensures that you are connecting to the official website and that any information you provide is encrypted and transmitted securely.

title: Here’s how you know +errors: + not_found: Page Not Found + not_found_sub: Looks like that page does not exist! email: application_invite: "{inviter_name} has invited you to a JEDI cloud application" portfolio_invite: "{inviter_name} has invited you to a JEDI cloud portfolio" From 1435734969b25cb590459e6d3c495b2c40d770b6 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 30 Sep 2019 13:29:20 -0400 Subject: [PATCH 2/5] Add monkeypatch to mock handle_error() --- .secrets.baseline | 2 +- tests/test_access.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.secrets.baseline b/.secrets.baseline index 26e8796a..4c29a6d9 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -194,7 +194,7 @@ "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", "is_secret": false, "is_verified": false, - "line_number": 543, + "line_number": 638, "type": "Hex High Entropy String" } ] diff --git a/tests/test_access.py b/tests/test_access.py index e864ff4f..283823d2 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -74,6 +74,11 @@ def test_all_protected_routes_have_access_control( ) monkeypatch.setattr("atst.app.assign_resources", lambda *a: None) + # monkeypatch the error handler + monkeypatch.setattr( + "atst.routes.errors.handle_error", lambda *a, **k: ("error", 500) + ) + # patch the internal function the access decorator uses so that # we can check that it was called mocker.patch("atst.domain.authz.decorator.check_access") From b990a59dc9e1b5c7056600acf090b94876b64e3b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 1 Oct 2019 13:54:13 -0400 Subject: [PATCH 3/5] Clean up stylesheet --- styles/components/_error_page.scss | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/styles/components/_error_page.scss b/styles/components/_error_page.scss index 45a8b8f5..4f29fa37 100644 --- a/styles/components/_error_page.scss +++ b/styles/components/_error_page.scss @@ -2,25 +2,27 @@ max-width: 475px; margin: auto; - .panel__heading { - text-align: center; - padding: $gap 0; + .panel { + &__heading { + text-align: center; + padding: $gap 0; - hr { - width: 100%; - border: 1px solid $color-red; + hr { + width: 100%; + border: 1px solid $color-red; + } + + h1 { + margin-bottom: $gap; + } } - h1 { - margin-bottom: $gap; + &__body { + padding: $gap * 2; + margin: 0; } } - .panel__body { - padding: $gap * 2; - margin: 0; - } - .icon { @include icon-size(60); } From 7db38ede5930690b619487514962b2f3c166f107 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 1 Oct 2019 13:58:29 -0400 Subject: [PATCH 4/5] Remove extra kwargs and put the text in the template since all error pages will show the same text in MVP --- atst/routes/errors.py | 19 ++----------------- templates/error.html | 4 ++-- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/atst/routes/errors.py b/atst/routes/errors.py index 36d19b34..a4f92baf 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -26,25 +26,10 @@ def notify(e, message, code): current_app.notification_sender.send(message) -def handle_error( - e, - message=translate("errors.not_found"), - code=404, - submessage=translate("errors.not_found_sub"), - more_info=translate("common.lorem"), -): +def handle_error(e, message=translate("errors.not_found"), code=404): log_error(e) notify(e, message, code) - return ( - render_template( - "error.html", - message=message, - code=code, - submessage=submessage, - more_info=more_info, - ), - code, - ) + return (render_template("error.html", message=message, code=code), code) def make_error_pages(app): diff --git a/templates/error.html b/templates/error.html index c3675e0e..00dd5f51 100644 --- a/templates/error.html +++ b/templates/error.html @@ -9,12 +9,12 @@ {{ Icon('cloud', classes="icon--red icon--large")}}

{{ code }} - {{ message }}

-

{{ submessage }}

+

{{ "errors.not_found_sub" | translate }}

- {{ more_info }} + {{ "common.lorem" | translate }}

More lorem From 738d73dc03a833b0324dbe0beaefe6e23499524b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 2 Oct 2019 10:29:42 -0400 Subject: [PATCH 5/5] Add in a default error sub heading --- .secrets.baseline | 4 ++-- templates/error.html | 8 +++++++- translations.yaml | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 4c29a6d9..13d1bfb6 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$", "lines": null }, - "generated_at": "2019-09-30T13:51:34Z", + "generated_at": "2019-10-02T14:52:59Z", "plugins_used": [ { "base64_limit": 4.5, @@ -199,5 +199,5 @@ } ] }, - "version": "0.12.5" + "version": "0.12.6" } diff --git a/templates/error.html b/templates/error.html index 00dd5f51..74ecb956 100644 --- a/templates/error.html +++ b/templates/error.html @@ -9,7 +9,13 @@ {{ Icon('cloud', classes="icon--red icon--large")}}

{{ code }} - {{ message }}

-

{{ "errors.not_found_sub" | translate }}

+

+ {% if code == 404 -%} + {{ "errors.not_found_sub" | translate }} + {% else %} + {{ "errors.default_sub" | translate }} + {%- endif %} +

diff --git a/translations.yaml b/translations.yaml index 625870aa..83cbe0ad 100644 --- a/translations.yaml +++ b/translations.yaml @@ -98,6 +98,7 @@ components:

title: Here’s how you know errors: + default_sub: An error has occured! not_found: Page Not Found not_found_sub: Looks like that page does not exist! email: