From 63e9ef44e739eb96768af6b6d45c7c6289e31452 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 13 Jul 2017 13:05:41 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20show=20status=20for=20indiv?= =?UTF-8?q?idual=20letters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status won’t ever change from sending for letters. For now at least. And even when we do come up with more useful statuses I’m not convinced it’s useful to expose them to our admin users. A more useful piece of information to show is when we think the letter will be delivered. --- app/main/views/notifications.py | 5 +++- .../views/notifications/notification.html | 10 +++++++- tests/__init__.py | 7 +++--- tests/app/main/views/test_notifications.py | 24 +++++++++++++++++++ tests/conftest.py | 5 +++- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 5779db9b5..70978931d 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -19,6 +19,7 @@ from app.utils import ( get_help_argument, get_template, get_time_left, + get_letter_timings, REQUESTED_STATUSES, FAILURE_STATUSES, SENDING_STATUSES, @@ -63,6 +64,7 @@ def view_notification(service_id, notification_id): job = job_api_client.get_job(service_id, notification['job']['id'])['data'] else: job = None + return render_template( 'views/notifications/notification.html', finished=(notification['status'] in (DELIVERED_STATUSES + FAILURE_STATUSES)), @@ -79,7 +81,8 @@ def view_notification(service_id, notification_id): partials=get_single_notification_partials(notification), created_by=notification.get('created_by'), created_at=notification['created_at'], - help=get_help_argument() + help=get_help_argument(), + estimated_letter_delivery_date=get_letter_timings(notification['created_at']).earliest_delivery, ) diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index f2d4c4ea7..90e6ef54a 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -30,8 +30,16 @@ on {{ created_at|format_datetime_short }}

+ {% if template.template_type == 'letter' %} +

+ Estimated delivery date: {{ estimated_letter_delivery_date|string|format_date_short }} +

+ {% endif %} + {{ template|string }} - {{ ajax_block(partials, updates_url, 'status', finished=finished) }} + {% if template.template_type != 'letter' %} + {{ ajax_block(partials, updates_url, 'status', finished=finished) }} + {% endif %} {% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index ab20273e6..629fedb6d 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -86,7 +86,7 @@ def service_json( def template_json(service_id, id_, name="sample template", - type_="sms", + type_=None, content=None, subject=None, version=1, @@ -97,7 +97,7 @@ def template_json(service_id, template = { 'id': id_, 'name': name, - 'template_type': type_, + 'template_type': type_ or "sms", 'content': content, 'service': service_id, 'version': version, @@ -226,9 +226,10 @@ def notification_json( with_links=False, rows=5, personalisation=None, + template_type=None, ): if template is None: - template = template_json(service_id, str(generate_uuid())) + template = template_json(service_id, str(generate_uuid()), type_=template_type) if sent_at is None: sent_at = str(datetime.utcnow().time()) if created_at is None: diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 996394fda..0233f3117 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -110,3 +110,27 @@ def test_notification_page_doesnt_link_to_template_in_tour( 'sample template sent by Test User on 1 January at 1:01am' ) assert len(page.select('main p:nth-of-type(1) a')) == 0 + + +@freeze_time("2016-01-01 01:01") +def test_notification_page_shows_status_of_letter_notification( + client_request, + mocker, + fake_uuid, +): + + mock_get_notification(mocker, fake_uuid, template_type='letter') + + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert normalize_spaces(page.select('main p:nth-of-type(1)')[0].text) == ( + 'sample template sent by Test User on 1 January at 1:01am' + ) + assert normalize_spaces(page.select('main p:nth-of-type(2)')[0].text) == ( + 'Estimated delivery date: 6 January' + ) + assert page.select('p.notification-status') == [] diff --git a/tests/conftest.py b/tests/conftest.py index c224ac513..bc5739459 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1679,6 +1679,7 @@ def mock_get_notification( fake_uuid, notification_status='delivered', redact_personalisation=False, + template_type=None, ): def _get_notification( service_id, @@ -1687,7 +1688,8 @@ def mock_get_notification( noti = notification_json( service_id, rows=1, - status=notification_status + status=notification_status, + template_type=template_type, )['notifications'][0] noti['id'] = notification_id @@ -1702,6 +1704,7 @@ def mock_get_notification( '5407f4db-51c7-4150-8758-35412d42186a', content='hello ((name))', redact_personalisation=redact_personalisation, + type_=template_type, ) return noti From 68078041dd7347013a50b8f0a7cb15eb18939a12 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 13 Jul 2017 13:28:11 +0100 Subject: [PATCH 2/2] Show personalisation in individual letters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new endpoint that works like view template/view preview of letter, so that this page works the same way it does for emails/text messages (ie showing the full content of the message, including personalisation). We’re not worrying about redaction in letters for now. --- app/main/views/notifications.py | 33 ++++++++++++++++++---- tests/app/main/views/test_notifications.py | 27 ++++++++++++++++++ tests/conftest.py | 1 + 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 70978931d..81f72343c 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -14,6 +14,7 @@ from app import ( current_service ) from app.main import main +from app.template_previews import TemplatePreview from app.utils import ( user_has_permissions, get_help_argument, @@ -41,20 +42,18 @@ def get_status_arg(filter_args): return REQUESTED_STATUSES -@main.route("/services//notification/") +@main.route("/services//notification/") @login_required @user_has_permissions('view_activity', admin_override=True) def view_notification(service_id, notification_id): - notification = notification_api_client.get_notification(service_id, notification_id) + notification = notification_api_client.get_notification(service_id, str(notification_id)) template = get_template( notification['template'], current_service, letter_preview_url=url_for( - '.view_template_version_preview', + '.view_letter_notification_as_image', service_id=service_id, - template_id=notification['template']['id'], - version=notification['template_version'], - filetype='png', + notification_id=notification_id, ), show_recipient=True, redact_missing_personalisation=True, @@ -86,6 +85,28 @@ def view_notification(service_id, notification_id): ) +@main.route("/services//notification/.png") +@login_required +@user_has_permissions('view_activity', admin_override=True) +def view_letter_notification_as_image(service_id, notification_id): + + notification = notification_api_client.get_notification(service_id, notification_id) + + template = get_template( + notification['template'], + current_service, + letter_preview_url=url_for( + '.view_letter_notification_as_image', + service_id=service_id, + notification_id=notification_id, + ), + ) + + template.values = notification['personalisation'] + + return TemplatePreview.from_utils_template(template, 'png', page=request.args.get('page')) + + @main.route("/services//notification/.json") @user_has_permissions('view_activity', admin_override=True) def view_notification_updates(service_id, notification_id): diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 0233f3117..c3edc957f 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -1,4 +1,5 @@ from freezegun import freeze_time +from flask import url_for import pytest from app.utils import ( @@ -8,6 +9,7 @@ from app.utils import ( DELIVERED_STATUSES, ) +from notifications_utils.template import LetterImageTemplate from tests.conftest import mock_get_notification, SERVICE_ONE_ID, normalize_spaces @@ -134,3 +136,28 @@ def test_notification_page_shows_status_of_letter_notification( 'Estimated delivery date: 6 January' ) assert page.select('p.notification-status') == [] + + +def test_should_show_image_of_letter_notification( + logged_in_client, + fake_uuid, + mocker +): + + mock_get_notification(mocker, fake_uuid, template_type='letter') + + mocked_preview = mocker.patch( + 'app.main.views.templates.TemplatePreview.from_utils_template', + return_value='foo' + ) + + response = logged_in_client.get(url_for( + 'main.view_letter_notification_as_image', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + )) + + assert response.status_code == 200 + assert response.get_data(as_text=True) == 'foo' + assert isinstance(mocked_preview.call_args[0][0], LetterImageTemplate) + assert mocked_preview.call_args[0][1] == 'png' diff --git a/tests/conftest.py b/tests/conftest.py index bc5739459..e5c9f2256 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1703,6 +1703,7 @@ def mock_get_notification( service_id, '5407f4db-51c7-4150-8758-35412d42186a', content='hello ((name))', + subject='blah', redact_personalisation=redact_personalisation, type_=template_type, )