From c554e9e32dac20c93f3fbad9477809dab994b2a7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 19 Jun 2017 16:37:28 +0100 Subject: [PATCH] Put template content or subject in tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In tables where we show rows and rows of information we used to give some meta information about the notification, or at least as much as we could give in the very limited space available. This information is now on the notifications page, so the information we show in these tables should just be whatever helps users identify the right message. I reckon that this is: - the content of the message for text messages - the subject for emails and letters This also makes these pages consistent with: - the inbound SMS page - the way the people’s inboxes work for their text messsages/Whatsapps/emails For consistency’s sake this makes the job page work the same way. It may be slightly less useful here because on the job page every message is sent from the same template, so will have broadly the same content. --- app/main/views/jobs.py | 22 +++++++++++++++++-- .../partials/jobs/notifications.html | 3 ++- .../views/activity/notifications.html | 12 ++-------- tests/__init__.py | 8 +++++-- tests/app/main/views/test_activity.py | 12 ++++++---- tests/app/main/views/test_jobs.py | 3 ++- 6 files changed, 40 insertions(+), 20 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 266811651..dba4adeb4 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -14,6 +14,10 @@ from flask import ( stream_with_context ) from flask_login import login_required +from notifications_utils.template import ( + Template, + WithSubjectTemplate, +) from werkzeug.datastructures import MultiDict from app import ( @@ -271,7 +275,7 @@ def get_notifications(service_id, message_type, status_override=None): ), 'notifications': render_template( 'views/activity/notifications.html', - notifications=notifications['notifications'], + notifications=add_preview_of_content_to_notifications(notifications['notifications']), page=page, prev_page=prev_page, next_page=next_page, @@ -368,7 +372,7 @@ def get_job_partials(job): ), 'notifications': render_template( 'partials/jobs/notifications.html', - notifications=notifications['notifications'], + notifications=add_preview_of_content_to_notifications(notifications['notifications']), more_than_one_page=bool(notifications.get('links', {}).get('next')), percentage_complete=(job['notifications_requested'] / job['notification_count'] * 100), download_link=url_for( @@ -386,3 +390,17 @@ def get_job_partials(job): job=job ), } + + +def add_preview_of_content_to_notifications(notifications): + return ( + dict( + preview_of_content=( + str(Template(notification['template'], notification['personalisation'])) + if notification['template']['template_type'] == 'sms' else + WithSubjectTemplate(notification['template'], notification['personalisation']).subject + ), + **notification + ) + for notification in notifications + ) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index d1c319d59..a6f781e0d 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -48,8 +48,9 @@ field_headings_visible=False ) %} {% call row_heading() %} + {{ item.to }}

- {{ item.to }} + {{ item.preview_of_content }}

{% endcall %} {{ notification_status_field(item) }} diff --git a/app/templates/views/activity/notifications.html b/app/templates/views/activity/notifications.html index d6f96a37b..2b6234d50 100644 --- a/app/templates/views/activity/notifications.html +++ b/app/templates/views/activity/notifications.html @@ -20,16 +20,8 @@

{{ item.to }}

-

- {% if item.job and item.job.original_file_name == 'Report' %} - {{ item.template.name }} - sent to one recipient - {% elif item.job %} - From {{ item.job.original_file_name }} - {% else %} - {{ item.template.name }} - from an API call - {% endif %} +

+ {{ item.preview_of_content }}

{% endcall %} diff --git a/tests/__init__.py b/tests/__init__.py index 70fbaf095..8e2ebbe9e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -103,6 +103,8 @@ def template_json(service_id, 'archived': archived, 'process_type': process_type } + if subject is None and type_ != 'sms': + template['subject'] = "template subject" if subject is not None: template['subject'] = subject return template @@ -216,7 +218,8 @@ def notification_json( created_at=None, updated_at=None, with_links=False, - rows=5 + rows=5, + personalisation=None, ): if template is None: template = template_json(service_id, str(generate_uuid())) @@ -255,7 +258,8 @@ def notification_json( 'updated_at': updated_at, 'job_row_number': job_row_number, 'service': service_id, - 'template_version': template['version'] + 'template_version': template['version'], + 'personalisation': personalisation or {}, } for i in range(rows)], 'total': rows, 'page_size': 50, diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index b0f230ced..e53afd442 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -92,13 +92,17 @@ def test_can_show_notifications( page=page_argument, )) assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') content = response.get_data(as_text=True) notifications = notification_json(service_one['id']) notification = notifications['notifications'][0] - assert notification['to'] in content - assert notification['status'] in content - assert notification['template']['name'] in content - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + text_of_first_row = page.select('tbody tr')[0].text + assert '07123456789' in text_of_first_row + assert ( + 'template content' in text_of_first_row or + 'template subject' in text_of_first_row + ) + assert 'Delivered' in text_of_first_row assert page_title in page.h1.text.strip() path_to_json = page.find("div", {'data-key': 'notifications'})['data-resource'] diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 1034fca7d..decd53bdb 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -80,6 +80,7 @@ def test_should_show_page_for_one_job( status_argument, expected_api_call, ): + response = logged_in_client.get(url_for( 'main.view_job', service_id=service_one['id'], @@ -94,7 +95,7 @@ def test_should_show_page_for_one_job( '{}: Template content with & entity'.format(service_one['name']) ) assert ' '.join(page.find('tbody').find('tr').text.split()) == ( - '07123456789 Delivered 1 January at 11:10am' + '07123456789 template content Delivered 1 January at 11:10am' ) assert page.find('div', {'data-key': 'notifications'})['data-resource'] == url_for( 'main.view_job_updates',