{{ notification.status|format_notification_status_as_time( notification.created_at|format_datetime_short, - (notification.updated_at or notification.created_at)|format_datetime_short + (notification.sent_at or notification.created_at)|format_datetime_short ) }}
{% if displayed_on_single_line %}{% endif %} From 802ebfa243aad8957c668d27564c1cf039b1e3cd Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 2 Feb 2024 08:31:05 -0800 Subject: [PATCH 2/4] use sent_at instead of updated_at --- app/main/views/notifications.py | 2 +- tests/app/main/views/test_activity.py | 16 ++++++++-------- tests/app/main/views/test_jobs.py | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 019bdfb4d..ac05e05ff 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -90,7 +90,7 @@ def view_notification(service_id, notification_id, error_message=None): partials=get_single_notification_partials(notification), created_by=notification.get("created_by"), created_at=notification["created_at"], - updated_at=notification["updated_at"], + updated_at=notification["sent_at"], help=get_help_argument(), notification_id=notification["id"], can_receive_inbound=(current_service.has_permission("inbound_sms")), diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 69e704827..96c50a308 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -160,8 +160,8 @@ def test_can_show_notifications( assert normalize_spaces( first_row.select_one(".table-field-right-aligned .align-with-message-body").text ) in [ - "Delivered 1 January at 02:01 US/Eastern", - "Delivered 1 January at 01:01 US/Eastern", + "Delivered 1 January at 02:00 US/Eastern", + "Delivered 1 January at 01:00 US/Eastern", ] assert page_title in page.h1.text.strip() @@ -661,31 +661,31 @@ def test_redacts_templates_that_should_be_redacted( ( "email", "temporary-failure", - "Inbox not accepting messages right now 27 September at 08:31 US/Eastern", + "Inbox not accepting messages right now 27 September at 08:30 US/Eastern", False, ), ( "email", "permanent-failure", - "Email address does not exist 27 September at 08:31 US/Eastern", + "Email address does not exist 27 September at 08:30 US/Eastern", False, ), - ("email", "delivered", "Delivered 27 September at 08:31 US/Eastern", True), + ("email", "delivered", "Delivered 27 September at 08:30 US/Eastern", True), ("sms", "created", "Sending since 27 September at 08:30 US/Eastern", True), ("sms", "sending", "Sending since 27 September at 08:30 US/Eastern", True), ( "sms", "temporary-failure", - "Phone not accepting messages right now 27 September at 08:31 US/Eastern", + "Phone not accepting messages right now 27 September at 08:30 US/Eastern", False, ), ( "sms", "permanent-failure", - "Not delivered 27 September at 08:31 US/Eastern", + "Not delivered 27 September at 08:30 US/Eastern", False, ), - ("sms", "delivered", "Delivered 27 September at 08:31 US/Eastern", True), + ("sms", "delivered", "Delivered 27 September at 08:30 US/Eastern", True), ], ) def test_sending_status_hint_displays_correctly_on_notifications_page( diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 42cd5ecbf..323a1fe00 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -90,7 +90,7 @@ def test_should_show_page_for_one_job( assert page.h1.text.strip() == "thisisatest.csv" assert " ".join(page.find("tbody").find("tr").text.split()) == ( - "2021234567 template content Delivered 1 January at 06:10 US/Eastern" + "2021234567 template content Delivered 1 January at 06:09 US/Eastern" ) assert page.find("div", {"data-key": "notifications"})["data-resource"] == url_for( "main.view_job_updates", @@ -109,7 +109,7 @@ def test_should_show_page_for_one_job( assert page.find("span", {"id": "time-left"}).text == "Data available for 7 days" assert normalize_spaces(page.select_one("tbody tr").text) == normalize_spaces( - "2021234567 " "template content " "Delivered 1 January at 06:10 US/Eastern" + "2021234567 " "template content " "Delivered 1 January at 06:09 US/Eastern" ) assert page.select_one("tbody tr a")["href"] == url_for( "main.view_notification", @@ -424,7 +424,7 @@ def test_should_show_updates_for_one_job_as_json( assert "2021234567" in content["notifications"] assert "Status" in content["notifications"] assert "Delivered" in content["notifications"] - assert "00:01" in content["notifications"] + assert "00:00" in content["notifications"] assert "Sent by Test User on 1 January at 00:00" in content["status"] @@ -466,7 +466,7 @@ def test_should_show_updates_for_scheduled_job_as_json( assert "2021234567" in content["notifications"] assert "Status" in content["notifications"] assert "Delivered" in content["notifications"] - assert "00:01" in content["notifications"] + assert "00:00" in content["notifications"] assert "Sent by Test User on 1 June at 16:00" in content["status"] From 338a6426c315d011c969f1a5313c7773ed7d12e3 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 6 Feb 2024 13:17:09 -0800 Subject: [PATCH 3/4] inline code for govuk_frontend_jinja --- app/__init__.py | 2 +- app/utils/govuk_frontend_jinja/__init__.py | 1 + app/utils/govuk_frontend_jinja/flask_ext.py | 30 ++ app/utils/govuk_frontend_jinja/templates.py | 292 ++++++++++++++++++++ poetry.lock | 35 +-- pyproject.toml | 1 - 6 files changed, 325 insertions(+), 36 deletions(-) create mode 100644 app/utils/govuk_frontend_jinja/__init__.py create mode 100644 app/utils/govuk_frontend_jinja/flask_ext.py create mode 100644 app/utils/govuk_frontend_jinja/templates.py diff --git a/app/__init__.py b/app/__init__.py index af9b3f9a9..3f008f7a1 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -21,7 +21,6 @@ from flask_login import LoginManager, current_user from flask_talisman import Talisman from flask_wtf import CSRFProtect from flask_wtf.csrf import CSRFError -from govuk_frontend_jinja.flask_ext import init_govuk_frontend from itsdangerous import BadSignature from notifications_python_client.errors import HTTPError from notifications_utils import logging, request_helper @@ -114,6 +113,7 @@ from app.notify_client.template_statistics_api_client import template_statistics from app.notify_client.upload_api_client import upload_api_client from app.notify_client.user_api_client import user_api_client from app.url_converters import SimpleDateTypeConverter, TemplateTypeConverter +from app.utils.govuk_frontend_jinja.flask_ext import init_govuk_frontend login_manager = LoginManager() csrf = CSRFProtect() diff --git a/app/utils/govuk_frontend_jinja/__init__.py b/app/utils/govuk_frontend_jinja/__init__.py new file mode 100644 index 000000000..34ac8d172 --- /dev/null +++ b/app/utils/govuk_frontend_jinja/__init__.py @@ -0,0 +1 @@ +from .templates import Environment # noqa diff --git a/app/utils/govuk_frontend_jinja/flask_ext.py b/app/utils/govuk_frontend_jinja/flask_ext.py new file mode 100644 index 000000000..41f1e59ce --- /dev/null +++ b/app/utils/govuk_frontend_jinja/flask_ext.py @@ -0,0 +1,30 @@ +from flask.templating import Environment as FlaskEnvironment +from jinja2 import select_autoescape + +from app.utils.govuk_frontend_jinja.templates import Environment as NunjucksEnvironment +from app.utils.govuk_frontend_jinja.templates import ( + NunjucksExtension, + NunjucksUndefined, +) + + +class Environment(NunjucksEnvironment, FlaskEnvironment): + pass + + +def init_govuk_frontend(app): + """Use the govuk_frontend_jinja Jinja environment in a Flask app + + >>> from flask import Flask + >>> app = Flask("cheeseshop_service") + >>> init_govuk_frontend(app) + """ + app.jinja_environment = Environment + app.select_jinja_autoescape = select_autoescape( + ("html", "htm", "xml", "xhtml", "njk") + ) + jinja_options = app.jinja_options.copy() + jinja_options["extensions"].append(NunjucksExtension) + jinja_options["undefined"] = NunjucksUndefined + app.jinja_options = jinja_options + return app diff --git a/app/utils/govuk_frontend_jinja/templates.py b/app/utils/govuk_frontend_jinja/templates.py new file mode 100644 index 000000000..1295fd616 --- /dev/null +++ b/app/utils/govuk_frontend_jinja/templates.py @@ -0,0 +1,292 @@ +import builtins +import os.path as path +import re +from collections.abc import Sized + +import jinja2 +import jinja2.ext +from jinja2.lexer import Token +from markupsafe import Markup + + +def njk_to_j2(template): + # Some component templates (such as radios) use `items` as the key of + # an object element. However `items` is also the name of a dictionary + # method in Python, and Jinja2 will prefer to return this attribute + # over the dict item. Handle specially. + template = re.sub(r"\.items\b", ".items__njk", template) + + # Some component templates (such as radios) append the loop index to a + # string. As the loop index is an integer this causes a TypeError in + # Python. Jinja2 has an operator `~` for string concatenation that + # converts integers to strings. + template = template.replace("+ loop.index", "~ loop.index") + + # The Character Count component in version 3 concatenates the word count + # with the hint text. As the word count is an integer this causes a + # TypeError in Python. Jinja2 has an operator `~` for string + # concatenation that converts integers to strings. + template = template.replace( + "+ (params.maxlength or params.maxwords) +", + "~ (params.maxlength or params.maxwords) ~", + ) + + # Nunjucks uses elseif, Jinja uses elif + template = template.replace("elseif", "elif") + + # Some component templates (such as input) call macros with params as + # an object which has unqoted keys. This causes Jinja to silently + # ignore the values. + template = re.sub( + r"""^([ ]*)([^ '"#\r\n:]+?)\s*:""", r"\1'\2':", template, flags=re.M + ) + + # govukFieldset can accept a call block argument, however the Jinja + # compiler does not detect this as the macro body is included from + # the template file. A workaround is to patch the declaration of the + # macro to include an explicit caller argument. + template = template.replace( + "macro govukFieldset(params)", "macro govukFieldset(params, caller=none)" + ) + + # Many components feature an attributes field, which is supposed to be + # a dictionary. In the template for these components, the keys and values + # are iterated. In Python, the default iterator for a dict is .keys(), but + # we want .items(). + # This only works because our undefined implements .items() + # We've tested this explicitly with: govukInput, govukCheckbox, govukTable, + # govukSummaryList + template = re.sub( + r"for attribute, value in (params|item|cell|action).attributes", + r"for attribute, value in \1.attributes.items()", + template, + flags=re.M, + ) + + # Some templates try to set a variable in an outer block, which is not + # supported in Jinja. We create a namespace in those templates to get + # around this. + template = re.sub( + r"""^([ ]*)({% set describedBy =( params.*describedBy if params.*describedBy else)? "" %})""", + r"\1{%- set nonlocal = namespace() -%}\n\1\2", + template, + flags=re.M, + ) + # Change any references to describedBy to be nonlocal.describedBy, + # unless describedBy is a dictionary key (i.e. quoted or dotted). + template = re.sub(r"""(?>> foo = ChainableUndefined(name='foo') + >>> str(foo.bar['baz']) + '' + >>> foo.bar['baz'] + 42 + Traceback (most recent call last): + ... + jinja2.exceptions.UndefinedError: 'foo' is undefined + """ + return self + + __getitem__ = __getattr__ + + # Allow treating undefined as an (empty) dictionary. + # This works because Undefined is an iterable. + def items(self): + return self + + # Allow escaping with Markup. This is required when + # autoescape is enabled. Debugging this issue was + # annoying; the error messages were not clear as to + # the cause of the issue (see upstream pull request + # for info https://github.com/pallets/jinja/pull/1047) + def __html__(self): + return str(self) + + # attempt to behave a bit like js's `undefined` when concatenation is attempted + def __add__(self, other): + if isinstance(other, str): + return "undefined" + other + return super().__add__(other) + + def __radd__(self, other): + if isinstance(other, str): + return other + "undefined" + return super().__radd__(other) + + +class NunjucksCodeGenerator(jinja2.compiler.CodeGenerator): + def visit_CondExpr(self, node, frame): + if not (self.filename or "").endswith(".njk"): + return super().visit_CondExpr(node, frame) + + # else our replacement, which is based on that in + # https://github.com/pallets/jinja/blob/c4c4088945a2c12535f539be7f5453b9ca94666c/jinja2/compiler.py#L1613 + def write_expr2(): + if node.expr2 is not None: + return self.visit(node.expr2, frame) + # rather than complaining about a missing else + # clause we just assume it to be the empty + # string for nunjucks compatibility + return self.write('""') + + self.write("(") + self.visit(node.expr1, frame) + self.write(" if ") + self.visit(node.test, frame) + self.write(" else ") + write_expr2() + self.write(")") + + +_njk_signature = "__njk" +_builtin_function_or_method_type = type({}.keys) + + +class Environment(jinja2.Environment): + code_generator_class = NunjucksCodeGenerator + + def __init__(self, *args, **kwargs): + kwargs.setdefault("extensions", [NunjucksExtension]) + kwargs.setdefault("undefined", NunjucksUndefined) + super().__init__(*args, **kwargs) + self.filters["indent_njk"] = indent_njk + + def join_path(self, template, parent): + """Enable the use of relative paths in template import statements""" + if template.startswith(("./", "../")): + return path.normpath(path.join(path.dirname(parent), template)) + else: + return template + + def _handle_njk(method_name): + def inner(self, obj, argument): + if isinstance(argument, str) and argument.endswith(_njk_signature): + # a njk-originated access will always be assuming a dict lookup before an attr + final_method_name = "getitem" + final_argument = argument[: -len(_njk_signature)] + else: + final_argument = argument + final_method_name = method_name + + # pleasantly surprised that super() works in this context + retval = builtins.getattr(super(), final_method_name)(obj, final_argument) + + if ( + argument == f"length{_njk_signature}" + and isinstance(retval, jinja2.runtime.Undefined) + and isinstance(obj, Sized) + ): + return len(obj) + if ( + isinstance(argument, str) + and argument.endswith(_njk_signature) + and isinstance(retval, _builtin_function_or_method_type) + ): + # the lookup has probably gone looking for attributes and found a builtin method. because + # any njk-originated lookup will have been made to prefer dict lookups over attributes, we + # can be fairly sure there isn't a dict key matching this - so we should just call this a + # failure. + return self.undefined(obj=obj, name=final_argument) + return retval + + return inner + + getitem = _handle_njk("getitem") + + getattr = _handle_njk("getattr") diff --git a/poetry.lock b/poetry.lock index da354843e..64314d70b 100644 --- a/poetry.lock +++ b/poetry.lock @@ -929,28 +929,6 @@ files = [ [package.dependencies] requests = "*" -[[package]] -name = "govuk-frontend-jinja" -version = "0.5.8-alpha" -description = "Tools to use the GOV.UK Design System with Jinja-powered Python apps" -optional = false -python-versions = "*" -files = [] -develop = false - -[package.dependencies] -jinja2 = "*" - -[package.extras] -dev = ["pytest", "pytest-flakes", "pytest-helpers-namespace"] -flask = ["Flask"] - -[package.source] -type = "git" -url = "https://github.com/alphagov/govuk-frontend-jinja.git" -reference = "v0.5.8-alpha" -resolved_reference = "15845e4cca3a05df72c6e13ec6a7e35acc682f52" - [[package]] name = "greenlet" version = "3.0.1" @@ -1428,16 +1406,6 @@ files = [ {file = "MarkupSafe-2.1.3-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:5bbe06f8eeafd38e5d0a4894ffec89378b6c6a625ff57e3028921f8ff59318ac"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win32.whl", hash = "sha256:dd15ff04ffd7e05ffcb7fe79f1b98041b8ea30ae9234aed2a9168b5797c3effb"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win_amd64.whl", hash = "sha256:134da1eca9ec0ae528110ccc9e48041e0828d79f24121a1a146161103c76e686"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:f698de3fd0c4e6972b92290a45bd9b1536bffe8c6759c62471efaa8acb4c37bc"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:aa57bd9cf8ae831a362185ee444e15a93ecb2e344c8e52e4d721ea3ab6ef1823"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ffcc3f7c66b5f5b7931a5aa68fc9cecc51e685ef90282f4a82f0f5e9b704ad11"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:47d4f1c5f80fc62fdd7777d0d40a2e9dda0a05883ab11374334f6c4de38adffd"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1f67c7038d560d92149c060157d623c542173016c4babc0c1913cca0564b9939"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:9aad3c1755095ce347e26488214ef77e0485a3c34a50c5a5e2471dff60b9dd9c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:14ff806850827afd6b07a5f32bd917fb7f45b046ba40c57abdb636674a8b559c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8f9293864fe09b8149f0cc42ce56e3f0e54de883a9de90cd427f191c346eb2e1"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win32.whl", hash = "sha256:715d3562f79d540f251b99ebd6d8baa547118974341db04f5ad06d5ea3eb8007"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win_amd64.whl", hash = "sha256:1b8dd8c3fd14349433c79fa8abeb573a55fc0fdd769133baac1f5e07abf54aeb"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:8e254ae696c88d98da6555f5ace2279cf7cd5b3f52be2b5cf97feafe883b58d2"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:cb0932dc158471523c9637e807d9bfb93e06a95cbf010f1a38b98623b929ef2b"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9402b03f1a1b4dc4c19845e5c749e3ab82d5078d16a2a4c2cd2df62d57bb0707"}, @@ -2476,7 +2444,6 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, @@ -3088,4 +3055,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = ">=3.9,<3.12" -content-hash = "94e6fe2e143f12eaa31f01b5a9206012a6a48908d98daa9fa2f531e986e4a6d3" +content-hash = "be46f8e90d6213db5c54d62c3a364c0a2a7e50f5c8e5007ad662067a6191c51f" diff --git a/pyproject.toml b/pyproject.toml index 2aed42337..49ea4dbe3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,6 @@ flask-login = "^0.6" flask-talisman = "*" flask-wtf = "^1.2" govuk-bank-holidays = "==0.13" -govuk-frontend-jinja = {git = "https://github.com/alphagov/govuk-frontend-jinja.git", tag = "v0.5.8-alpha"} gunicorn = {version = "==21.2.0", extras = ["eventlet"]} humanize = "~=4.9" itsdangerous = "~=2.1" From df54b066f9ba8f7d3b50e223df7edc524d2b1760 Mon Sep 17 00:00:00 2001 From: Carlo Costino