diff --git a/app/__init__.py b/app/__init__.py index 92397a688..0221981a8 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -373,6 +373,7 @@ def format_notification_status(status, template_type): 'pending-virus-check': 'Pending virus check', 'virus-scan-failed': 'Virus detected', 'returned-letter': 'Delivered', + 'cancelled': 'Cancelled,' } }[template_type].get(status, status) @@ -398,6 +399,7 @@ def format_notification_status_as_field_status(status, notification_type): 'pending-virus-check': None, 'virus-scan-failed': 'error', 'returned-letter': None, + 'cancelled': 'error', } }.get( notification_type, diff --git a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss index 34bb42526..03584dcdf 100644 --- a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss +++ b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss @@ -51,6 +51,10 @@ min-height: 50px; } + .page-footer-delete-link-without-button { + margin-top: 10px; + } + .notification-status { margin: 0; } diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 6b08fc86c..5b2a2d99e 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -8,7 +8,9 @@ from dateutil import parser from flask import ( Response, abort, + flash, jsonify, + redirect, render_template, request, stream_with_context, @@ -83,6 +85,11 @@ def view_notification(service_id, notification_id): letter_print_day = get_letter_printing_statement(notification['status'], notification['created_at']) + notification_created = parser.parse(notification['created_at']).replace(tzinfo=None) + + show_cancel_button = notification['notification_type'] == 'letter' and \ + letter_can_be_cancelled(notification['status'], notification_created) + return render_template( 'views/notifications/notification.html', finished=(notification['status'] in (DELIVERED_STATUSES + FAILURE_STATUSES)), @@ -110,10 +117,24 @@ def view_notification(service_id, notification_id): postage=notification['postage'], can_receive_inbound=(current_service.has_permission('inbound_sms')), is_precompiled_letter=notification['template']['is_precompiled_letter'], - letter_print_day=letter_print_day + letter_print_day=letter_print_day, + show_cancel_button=show_cancel_button ) +@main.route("/services//notification//cancel", methods=['GET', 'POST']) +@login_required +@user_has_permissions('view_activity', 'send_messages') +def cancel_letter(service_id, notification_id): + + if request.method == 'POST': + notification_api_client.update_notification_to_cancelled(current_service.id, notification_id) + return redirect(url_for('main.view_notification', service_id=service_id, notification_id=notification_id)) + + flash("Are you sure you want to cancel sending this letter?", 'cancel') + return view_notification(service_id, notification_id) + + def get_letter_printing_statement(status, created_at): created_at_dt = parser.parse(created_at).replace(tzinfo=None) diff --git a/app/navigation.py b/app/navigation.py index 5236a261a..2fb37551d 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -121,6 +121,7 @@ class HeaderNavigation(Navigation): 'cancel_invited_org_user', 'cancel_invited_user', 'cancel_job', + 'cancel_letter', 'check_and_resend_text_code', 'check_and_resend_verification_code', 'check_messages', @@ -397,6 +398,7 @@ class MainNavigation(Navigation): 'cancel_invited_org_user', 'cancel_invited_user', 'cancel_job', + 'cancel_letter', 'check_and_resend_text_code', 'check_and_resend_verification_code', 'check_messages_preview', @@ -569,6 +571,7 @@ class CaseworkNavigation(Navigation): 'cancel_invited_org_user', 'cancel_invited_user', 'cancel_job', + 'cancel_letter', 'check_and_resend_text_code', 'check_and_resend_verification_code', 'check_messages', @@ -806,6 +809,7 @@ class OrgNavigation(Navigation): 'cancel_invited_org_user', 'cancel_invited_user', 'cancel_job', + 'cancel_letter', 'check_and_resend_text_code', 'check_and_resend_verification_code', 'check_messages', diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 34dfdbe88..c40feebce 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -25,7 +25,8 @@ class JobApiClient(NotifyAdminAPIClient): def __convert_statistics(job): results = defaultdict(int) for outcome in job['statistics']: - if outcome['status'] in ['failed', 'technical-failure', 'temporary-failure', 'permanent-failure']: + if outcome['status'] in ['failed', 'technical-failure', 'temporary-failure', + 'permanent-failure', 'cancelled']: results['failed'] += outcome['count'] if outcome['status'] in ['sending', 'pending', 'created']: results['sending'] += outcome['count'] diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 10f0d3e6f..dae4f5825 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -93,5 +93,10 @@ class NotificationApiClient(NotifyAdminAPIClient): return self.get(url=get_url) + def update_notification_to_cancelled(self, service_id, notification_id): + return self.post( + url='/service/{}/notifications/{}/cancel'.format(service_id, notification_id), + data={}) + notification_api_client = NotificationApiClient() diff --git a/app/templates/components/table.html b/app/templates/components/table.html index bb0c76196..cb5622c60 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -162,7 +162,7 @@ notification.template.template_type ) }} {% endif %} - {% if notification.notification_type == "letter" and notification.status in ['permanent-failure', 'validation-failed'] %} + {% if notification.notification_type == "letter" and notification.status in ['permanent-failure', 'validation-failed', 'cancelled'] %} Cancelled {% endif %} {% if notification.status|format_notification_status_as_url(notification.notification_type) %} diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index 55617e7fd..a95f2fb19 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -6,7 +6,7 @@ {{ banner( message if message is string else message[0], 'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous', - delete_button="Yes, {}".format(category) if category in ['delete', 'suspend', 'resume', 'remove', 'revoke this API key'] else None, + delete_button="Yes, {}".format(category) if category in ['cancel', 'delete', 'suspend', 'resume', 'remove', 'revoke this API key'] else None, with_tick=True if category == 'default_with_tick' else False, context=message[1] if message is not string )}} diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index 78a618908..1149ef9db 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -37,7 +37,7 @@

{% if template.template_type == 'letter' %} - {% if notification_status == 'permanent-failure' %} + {% if notification_status in ('permanent-failure', 'cancelled') %}

Cancelled {{ updated_at|format_datetime_short }}

@@ -64,7 +64,13 @@ {% if template.template_type == 'letter' %}
-
 
+ {% if show_cancel_button %} + + Cancel sending this letter + + {% else %} +
 
+ {% endif %} Download as a PDF
diff --git a/app/utils.py b/app/utils.py index d15ed9cc6..0d771275c 100644 --- a/app/utils.py +++ b/app/utils.py @@ -39,7 +39,8 @@ from werkzeug.datastructures import MultiDict SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check'] DELIVERED_STATUSES = ['delivered', 'sent', 'returned-letter'] -FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] +FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', + 'technical-failure', 'virus-scan-failed', 'cancelled'] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 1a028867b..b6c2f1a52 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -52,7 +52,8 @@ from tests.conftest import ( [ 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', - 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', + 'virus-scan-failed', 'cancelled', ] ), ( @@ -65,7 +66,7 @@ from tests.conftest import ( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] + ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', 'cancelled'] ) ] ) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 98729d3f4..22d5ac11a 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -156,7 +156,8 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( [ 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', - 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', + 'virus-scan-failed', 'cancelled', ] ), ( @@ -169,7 +170,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] + ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', 'cancelled'] ) ] ) @@ -339,6 +340,7 @@ def test_should_show_letter_job( 'permanent-failure', 'technical-failure', 'virus-scan-failed', + 'cancelled', ], ) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 48c234b27..353c6bc94 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -195,6 +195,10 @@ def test_notification_page_shows_page_for_letter_notification( 'permanent-failure', 'Cancelled 1 January at 1:02am', ), + ( + 'cancelled', + 'Cancelled 1 January at 1:02am', + ), ( 'validation-failed', 'Cancelled 1 January at 1:02am (letter has content outside the printable area)', @@ -237,6 +241,82 @@ def test_notification_page_shows_cancelled_letter( assert page.select_one('main img')['src'].endswith('.png?page=1') +@pytest.mark.parametrize('notification_type', ['email', 'sms']) +@freeze_time('2016-01-01 15:00') +def test_notification_page_does_not_show_cancel_link_for_sms_or_email_notifications( + client_request, + mocker, + fake_uuid, + notification_type, +): + mock_get_notification( + mocker, + fake_uuid, + template_type=notification_type, + notification_status='created', + ) + + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert 'Cancel sending this letter' not in normalize_spaces(page.text) + + +@freeze_time('2016-01-01 15:00') +def test_notification_page_shows_cancel_link_for_letter_which_can_be_cancelled( + client_request, + mocker, + fake_uuid, +): + mock_get_notification( + mocker, + fake_uuid, + template_type='letter', + notification_status='created', + ) + mocker.patch( + 'app.main.views.notifications.get_page_count_for_letter', + return_value=1 + ) + + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert 'Cancel sending this letter' in normalize_spaces(page.text) + + +@freeze_time('2016-01-01 15:00') +def test_notification_page_does_not_show_cancel_link_for_letter_which_cannot_be_cancelled( + client_request, + mocker, + fake_uuid, +): + mock_get_notification( + mocker, + fake_uuid, + template_type='letter', + notification_status='delivered', + ) + mocker.patch( + 'app.main.views.notifications.get_page_count_for_letter', + return_value=1 + ) + + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert 'Cancel sending this letter' not in normalize_spaces(page.text) + + @freeze_time("2016-01-01 18:00") def test_notification_page_shows_page_for_first_class_letter_notification( client_request, @@ -527,3 +607,62 @@ def test_get_letter_printing_statement_for_letter_that_has_been_sent(created_at, statement = get_letter_printing_statement('delivered', created_at) assert statement == 'Printed on {}'.format(print_day) + + +@freeze_time('2016-01-01 15:00') +def test_show_cancel_letter_confirmation( + client_request, + mocker, + fake_uuid, +): + mock_get_notification( + mocker, + fake_uuid, + template_type='letter', + notification_status='created', + ) + mocker.patch( + 'app.main.views.notifications.get_page_count_for_letter', + return_value=1 + ) + + page = client_request.get( + 'main.cancel_letter', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + flash_message = normalize_spaces(page.find('div', class_='banner-dangerous').text) + + assert 'Are you sure you want to cancel sending this letter?' in flash_message + + +@freeze_time('2016-01-01 15:00') +def test_cancelling_a_letter_calls_the_api( + client_request, + mocker, + fake_uuid, +): + mock_get_notification( + mocker, + fake_uuid, + template_type='letter', + notification_status='created', + ) + mocker.patch( + 'app.main.views.notifications.get_page_count_for_letter', + return_value=1 + ) + cancel_endpoint = mocker.patch( + 'app.main.views.notifications.notification_api_client.update_notification_to_cancelled' + ) + + client_request.post( + 'main.cancel_letter', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + _follow_redirects=True, + _expected_redirect=None, + ) + + assert cancel_endpoint.called diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index aeb051994..6486741e3 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -85,3 +85,12 @@ def test_get_api_notifications_changes_letter_statuses(mocker): assert ret['notifications'][0]['status'] == 'created' assert ret['notifications'][1]['status'] == 'created' assert ret['notifications'][2]['status'] == 'accepted' + + +def test_update_notification_to_cancelled(mocker): + mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post') + NotificationApiClient().update_notification_to_cancelled('foo', 'bar') + mock_post.assert_called_once_with( + url='/service/foo/notifications/bar/cancel', + data={}, + )