diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 89b283ddd..29c376245 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -66,7 +66,6 @@ $path: '/static/images/'; @import 'views/api'; @import 'views/product-page'; @import 'views/template'; -@import 'views/notification'; // TODO: break this up @import 'app'; diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index c3bc7b9b7..a1fed973a 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -46,6 +46,17 @@ transition: width 0.6s ease-in-out; } + &-label { + @include bold-19; + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + color: $govuk-blue; + max-width: 100%; + background: $white; + } + } .file-list { @@ -56,17 +67,12 @@ white-space: nowrap; overflow: hidden; text-overflow: ellipsis; - padding-bottom: 30px; - padding-top: 10px; - margin-bottom: -30px; - margin-top: -10px; } &-hint { @include core-16; display: block; color: $secondary-text-colour; - pointer-events: none; } } @@ -90,6 +96,5 @@ } .align-with-message-body { - display: block; margin-top: $gutter * 5 / 6; } diff --git a/app/assets/stylesheets/views/notification.scss b/app/assets/stylesheets/views/notification.scss deleted file mode 100644 index 08def6684..000000000 --- a/app/assets/stylesheets/views/notification.scss +++ /dev/null @@ -1,18 +0,0 @@ -.notification-status { - - @include core-16; - color: $secondary-text-colour; - margin-top: -$gutter-half; - - &.error { - - color: $error-colour; - font-weight: bold; - - a { - color: $error-colour; - } - - } - -} diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 17d9996ed..266811651 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -14,10 +14,6 @@ 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 ( @@ -114,7 +110,21 @@ def view_job(service_id, job_id): 'views/jobs/job.html', finished=(total_notifications == processed_notifications), uploaded_file_name=job['original_file_name'], - template_id=job['template'], + template=get_template( + service_api_client.get_service_template( + service_id=service_id, + template_id=job['template'], + version=job['template_version'] + )['data'], + current_service, + letter_preview_url=url_for( + '.view_template_version_preview', + service_id=service_id, + template_id=job['template'], + version=job['template_version'], + filetype='png', + ), + ), status=request.args.get('status', ''), updates_url=url_for( ".view_job_updates", @@ -261,7 +271,7 @@ def get_notifications(service_id, message_type, status_override=None): ), 'notifications': render_template( 'views/activity/notifications.html', - notifications=add_preview_of_content_to_notifications(notifications['notifications']), + notifications=notifications['notifications'], page=page, prev_page=prev_page, next_page=next_page, @@ -350,11 +360,6 @@ def get_job_partials(job): notifications = notification_api_client.get_notifications_for_service( job['service'], job['id'], status=filter_args['status'] ) - template = service_api_client.get_service_template( - service_id=current_service['id'], - template_id=job['template'], - version=job['template_version'] - )['data'] return { 'counts': render_template( 'partials/count.html', @@ -363,7 +368,7 @@ def get_job_partials(job): ), 'notifications': render_template( 'partials/jobs/notifications.html', - notifications=add_preview_of_content_to_notifications(notifications['notifications']), + 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( @@ -374,26 +379,10 @@ def get_job_partials(job): ), help=get_help_argument(), time_left=get_time_left(job['created_at']), - job=job, - template=template, - template_version=job['template_version'], + job=job ), 'status': render_template( 'partials/jobs/status.html', 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/main/views/notifications.py b/app/main/views/notifications.py index 20d309a21..ec51789bd 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -10,7 +10,6 @@ from flask_login import login_required from app import ( notification_api_client, - job_api_client, current_service ) from app.main import main @@ -40,34 +39,27 @@ def get_status_arg(filter_args): return REQUESTED_STATUSES -@main.route("/services//notification/") +@main.route("/services//one-off-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) - template = get_template( - notification['template'], - current_service, - letter_preview_url=url_for( - '.view_template_version_preview', - service_id=service_id, - template_id=notification['template']['id'], - version=notification['template_version'], - filetype='png', - ), - show_recipient=True, - ) - template.values = notification['personalisation'] - if notification['job']: - 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)), uploaded_file_name='Report', - template=template, - job=job, + template=get_template( + notification['template'], + current_service, + letter_preview_url=url_for( + '.view_template_version_preview', + service_id=service_id, + template_id=notification['template']['id'], + version=notification['template_version'], + filetype='png', + ), + ), + status=request.args.get('status'), updates_url=url_for( ".view_notification_updates", service_id=service_id, @@ -76,13 +68,11 @@ def view_notification(service_id, notification_id): help=get_help_argument() ), partials=get_single_notification_partials(notification), - created_by=notification.get('created_by'), - created_at=notification['created_at'], help=get_help_argument() ) -@main.route("/services//notification/.json") +@main.route("/services//one-off-notification/.json") @user_has_permissions('view_activity', admin_override=True) def view_notification_updates(service_id, notification_id): return jsonify(**get_single_notification_partials( @@ -90,10 +80,49 @@ def view_notification_updates(service_id, notification_id): )) +def _get_single_notification_counts(notification, help_argument): + return [ + ( + label, + query_param, + url_for( + ".view_notification", + service_id=notification['service'], + notification_id=notification['id'], + status=query_param, + help=help_argument + ), + count + ) for label, query_param, count in [ + [ + 'total', '', + 1 + ], + [ + 'sending', 'sending', + int(notification['status'] in SENDING_STATUSES) + ], + [ + 'delivered', 'delivered', + int(notification['status'] in DELIVERED_STATUSES) + ], + [ + 'failed', 'failed', + int(notification['status'] in FAILURE_STATUSES) + ] + ] + ] + + def get_single_notification_partials(notification): status_args = get_status_arg(request.args) return { + 'counts': render_template( + 'partials/count.html', + counts=_get_single_notification_counts(notification, request.args.get('help', 0)), + status=status_args + ), 'notifications': render_template( 'partials/notifications/notifications.html', notification=notification, diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 5ad942fe6..8708465b2 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -120,7 +120,6 @@ {% macro notification_status_field(notification) %} {% call field(status=notification.status|format_notification_status_as_field_status, align='right') %} - {% if notification.status in ['created', 'sending', 'delivered'] %}{% endif %} {% if notification.status|format_notification_status_as_url %} {% endif %} @@ -136,7 +135,6 @@ (notification.updated_at or notification.created_at)|format_datetime_short ) }} - {% if notification.status in ['created', 'sending', 'delivered'] %}{% endif %} {% endcall %} {% endmacro %} diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 3c0ca9ef2..2dc588d0e 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -32,7 +32,7 @@ Notify delivers the message

{% if help == '3' %} - + Now go to your dashboard {% endif %} diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 4bec657b3..8c2964bdd 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -5,9 +5,7 @@ {% if job.job_status == 'scheduled' %}

- Sending - {{ template.name }} - {{ job.scheduled_for|format_datetime_relative }} + Sending will start {{ job.scheduled_for|format_datetime_relative }}

@@ -50,10 +48,7 @@ field_headings_visible=False ) %} {% call row_heading() %} - {{ item.to }} -

- {{ item.preview_of_content }} -

+

{{ item.to }}

{% endcall %} {{ notification_status_field(item) }} {% endcall %} diff --git a/app/templates/partials/jobs/status.html b/app/templates/partials/jobs/status.html index c243ce572..c66bc6427 100644 --- a/app/templates/partials/jobs/status.html +++ b/app/templates/partials/jobs/status.html @@ -1,5 +1,5 @@
-

+

{% if job.scheduled_for %} {% if job.processing_started %} Sent by {{ job.created_by.name }} on {{ job.processing_started|format_datetime_short }} diff --git a/app/templates/partials/notifications/status.html b/app/templates/partials/notifications/status.html index 730eb6954..481b32446 100644 --- a/app/templates/partials/notifications/status.html +++ b/app/templates/partials/notifications/status.html @@ -1,13 +1,6 @@

-

- {% if notification.status|format_notification_status_as_url %} - - {% endif %} - {{ notification.status|format_notification_status( - notification.template.template_type - ) }} - {% if notification.status|format_notification_status_as_url %} - - {% endif %} +

+ Sent {% if notification.created_by %}by {{ notification.created_by.name }} {% endif %} + on {{ notification.created_at|format_datetime_short }}

diff --git a/app/templates/views/activity/notifications.html b/app/templates/views/activity/notifications.html index cabcd436b..06158fb75 100644 --- a/app/templates/views/activity/notifications.html +++ b/app/templates/views/activity/notifications.html @@ -17,9 +17,19 @@ ) %} {% call row_heading() %} - {{ item.to }} -

- {{ item.preview_of_content }} +

+ {{ 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 %}

{% endcall %} diff --git a/app/templates/views/dashboard/all-template-statistics.html b/app/templates/views/dashboard/all-template-statistics.html index 202e3bc9c..a0c803fce 100644 --- a/app/templates/views/dashboard/all-template-statistics.html +++ b/app/templates/views/dashboard/all-template-statistics.html @@ -41,9 +41,11 @@ field_headings_visible=False ) %} {% call row_heading() %} - {{ item.name }} - - {{ message_count_label(1, item.type, suffix='template')|capitalize }} + + {{ item.name }} + + {{ message_count_label(1, item.type, suffix='template')|capitalize }} + {% endcall %} {{ spark_bar_field(item.requested_count, most_used_template_count) }} diff --git a/app/templates/views/dashboard/inbox.html b/app/templates/views/dashboard/inbox.html index 6dbc2fb5b..fe343b1bc 100644 --- a/app/templates/views/dashboard/inbox.html +++ b/app/templates/views/dashboard/inbox.html @@ -31,10 +31,10 @@ > {{ item.user_number | format_phone_number_human_readable }} - {{ item.content }} + {{ item.content }} {% endcall %} {% call field(align='right') %} - + {{ item.created_at | format_delta }} {% endcall %} diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 94564d0e3..2c3833655 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -17,9 +17,11 @@ ) %} {% call row_heading() %} - {{ item.template_name }} - - {{ message_count_label(1, item.template_type, suffix='template')|capitalize }} + + {{ item.template_name }} + + {{ message_count_label(1, item.template_type, suffix='template')|capitalize }} + {% endcall %} {% if template_statistics|length > 1 %} diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 45d0b8892..14aa297bf 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -13,8 +13,17 @@ {{ uploaded_file_name }} + {{ template|string }} + {{ ajax_block(partials, updates_url, 'status', finished=finished) }} {{ ajax_block(partials, updates_url, 'counts', finished=finished) }} {{ ajax_block(partials, updates_url, 'notifications', finished=finished) }} + {% if not help %} + {{ page_footer( + secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), + secondary_link_text='Back to {}'.format(template.name) + ) }} + {% endif %} + {% endblock %} diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index aba8beee4..f4936fb53 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -1,7 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/banner.html" import banner %} {% from "components/ajax-block.html" import ajax_block %} -{% from "components/message-count-label.html" import message_count_label %} {% from "components/page-footer.html" import page_footer %} {% block service_page_title %} @@ -11,23 +10,18 @@ {% block maincolumn_content %}

- {{ message_count_label(1, template.template_type, suffix='') | capitalize }} + Report

-

- {{ template.name }} - sent - {% if job and job.original_file_name != 'Report' %} - from - {{ job.original_file_name }} - {% elif created_by %} - by {{ created_by.name }} - {% endif %} - on {{ created_at|format_datetime_short }} -

- {{ template|string }} {{ ajax_block(partials, updates_url, 'status', finished=finished) }} + {{ ajax_block(partials, updates_url, 'counts', finished=finished) }} + {{ ajax_block(partials, updates_url, 'notifications', finished=finished) }} + + {{ page_footer( + secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), + secondary_link_text='Back to {}'.format(template.name) + ) }} {% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index d5afea427..bbe7dad18 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -107,8 +107,6 @@ 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 @@ -186,7 +184,7 @@ def job_json( if job_id is None: job_id = str(generate_uuid()) if template_id is None: - template_id = "5d729fbd-239c-44ab-b498-75a985f3198f" + template_id = str(generate_uuid()) if created_at is None: created_at = str(datetime.now(timezone.utc).strftime('%Y-%m-%dT%H:%M:%S.%f%z')) data = { @@ -222,8 +220,7 @@ def notification_json( created_at=None, updated_at=None, with_links=False, - rows=5, - personalisation=None, + rows=5 ): if template is None: template = template_json(service_id, str(generate_uuid())) @@ -262,8 +259,7 @@ def notification_json( 'updated_at': updated_at, 'job_row_number': job_row_number, 'service': service_id, - 'template_version': template['version'], - 'personalisation': personalisation or {}, + 'template_version': template['version'] } 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 e53afd442..b0f230ced 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -92,17 +92,13 @@ 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] - 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 notification['to'] in content + assert notification['status'] in content + assert notification['template']['name'] in content + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') 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 242ad6ca9..1034fca7d 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -8,7 +8,6 @@ from bs4 import BeautifulSoup from app.main.views.jobs import get_time_left, get_status_filters from tests import notification_json -from tests.app.test_utils import normalize_spaces from tests.conftest import SERVICE_ONE_ID from freezegun import freeze_time @@ -81,7 +80,6 @@ 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'], @@ -92,8 +90,11 @@ def test_should_show_page_for_one_job( assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text.strip() == 'thisisatest.csv' + assert page.find('div', {'class': 'sms-message-wrapper'}).text.strip() == ( + '{}: Template content with & entity'.format(service_one['name']) + ) assert ' '.join(page.find('tbody').find('tr').text.split()) == ( - '07123456789 template content Delivered 1 January at 11:10am' + '07123456789 Delivered 1 January at 11:10am' ) assert page.find('div', {'data-key': 'notifications'})['data-resource'] == url_for( 'main.view_job_updates', @@ -162,6 +163,7 @@ def test_should_show_job_in_progress( @freeze_time("2016-01-01T00:00:00.061258") def test_should_show_scheduled_job( logged_in_client, + service_one, active_user_with_permissions, mock_get_service_template, mock_get_scheduled_job, @@ -171,21 +173,13 @@ def test_should_show_scheduled_job( ): response = logged_in_client.get(url_for( 'main.view_job', - service_id=SERVICE_ONE_ID, + service_id=service_one['id'], job_id=fake_uuid )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert normalize_spaces(page.select('main p')[1].text) == ( - 'Sending Two week reminder today at midnight' - ) - assert page.select('main p a')[0]['href'] == url_for( - 'main.view_template_version', - service_id=SERVICE_ONE_ID, - template_id='5d729fbd-239c-44ab-b498-75a985f3198f', - version=1, - ) + assert page.find('main').find_all('p')[1].text.strip() == 'Sending will start today at midnight' assert page.find('input', {'type': 'submit', 'value': 'Cancel sending'}) @@ -262,7 +256,6 @@ def test_should_show_updates_for_one_job_as_json( service_one, active_user_with_permissions, mock_get_notifications, - mock_get_service_template, mock_get_job, mocker, fake_uuid, diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 10cfa2485..c03b4969a 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -1,6 +1,8 @@ from freezegun import freeze_time import pytest +from werkzeug.datastructures import MultiDict +from app.main.views.notifications import get_status_arg from app.utils import ( REQUESTED_STATUSES, FAILURE_STATUSES, @@ -8,34 +10,63 @@ from app.utils import ( DELIVERED_STATUSES, ) -from tests.app.test_utils import normalize_spaces from tests.conftest import mock_get_notification -@pytest.mark.parametrize('notification_status, expected_status', [ - ('created', 'Sending'), - ('sending', 'Sending'), - ('delivered', 'Delivered'), - ('failed', 'Failed'), - ('temporary-failure', 'Phone not accepting messages right now'), - ('permanent-failure', 'Phone number doesn’t exist'), - ('technical-failure', 'Technical failure'), +@pytest.mark.parametrize('multidict_args, expected_statuses', [ + ([], REQUESTED_STATUSES), + ([('status', '')], REQUESTED_STATUSES), + ([('status', 'garbage')], REQUESTED_STATUSES), + ([('status', 'sending')], SENDING_STATUSES), + ([('status', 'delivered')], DELIVERED_STATUSES), + ([('status', 'failed')], FAILURE_STATUSES), ]) +def test_status_filters(mocker, multidict_args, expected_statuses): + mocker.patch('app.main.views.notifications.current_app') + + args = MultiDict(multidict_args) + args['status'] = get_status_arg(args) + + assert sorted(args['status']) == sorted(expected_statuses) + + @freeze_time("2016-01-01 11:09:00.061258") def test_notification_status_page_shows_details( + client_request, + mock_get_notification, + service_one, + fake_uuid, +): + page = client_request.get( + 'main.view_notification', + service_id=service_one['id'], + notification_id=fake_uuid + ) + + assert page.find('div', {'class': 'sms-message-wrapper'}).text.strip() == 'service one: template content' + assert ' '.join(page.find('tbody').find('tr').text.split()) == '07123456789 Delivered 1 January at 11:10am' + + mock_get_notification.assert_called_with( + service_one['id'], + fake_uuid + ) + + +@pytest.mark.parametrize('notification_status, expected_big_number_vals', [ + ('created', [1, 1, 0, 0]), + ('sending', [1, 1, 0, 0]), + ('delivered', [1, 0, 1, 0]), + ('temporary-failure', [1, 0, 0, 1]), +]) +def test_notification_status_page_shows_correct_numbers( client_request, mocker, service_one, fake_uuid, notification_status, - expected_status, + expected_big_number_vals ): - - _mock_get_notification = mock_get_notification( - mocker, - fake_uuid, - notification_status=notification_status - ) + mock_get_notification(mocker, fake_uuid, notification_status=notification_status) page = client_request.get( 'main.view_notification', @@ -43,14 +74,5 @@ def test_notification_status_page_shows_details( notification_id=fake_uuid ) - assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( - 'service one: hello Jo' - ) - assert normalize_spaces(page.select('.ajax-block-container p')[0].text) == ( - expected_status - ) - - _mock_get_notification.assert_called_with( - service_one['id'], - fake_uuid - ) + big_numbers = page.find_all('div', {'class': 'big-number-number'}) + assert expected_big_number_vals == [int(num.text.strip()) for num in big_numbers] diff --git a/tests/conftest.py b/tests/conftest.py index 3f6519b29..288f5263a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1650,8 +1650,7 @@ def mock_get_notification(mocker, fake_uuid, notification_status='delivered'): 'name': 'Test User', 'email_address': 'test@user.gov.uk' } - noti['personalisation'] = {'name': 'Jo'} - noti['template'] = template_json(service_id, str(generate_uuid()), content='hello ((name))') + noti['template'] = template_json(service_id, str(generate_uuid())) return noti return mocker.patch(