From b4894e7a0352c9b6e92fcf323a5b2a358bf2005d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 May 2019 17:33:22 +0100 Subject: [PATCH 1/2] Always go back to previous page from notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When looking at a notification you can either be coming from the page of all notifications, or from a job. Currently the back link always takes you to the page of all notifications. This commit makes it a bit more sophisticated so if you’ve come from looking at a job, you go back to the job. --- app/main/views/notifications.py | 16 ++++++++ .../partials/jobs/notifications.html | 2 +- .../views/notifications/notification.html | 2 +- tests/__init__.py | 2 +- tests/app/main/views/test_jobs.py | 14 +++++++ tests/app/main/views/test_notifications.py | 40 +++++++++++++++++++ 6 files changed, 73 insertions(+), 3 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 30611432e..fb6143a5a 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -101,6 +101,21 @@ def view_notification(service_id, notification_id): show_cancel_button = notification['notification_type'] == 'letter' and \ letter_can_be_cancelled(notification['status'], notification_created) + if request.args.get('help') == '0': + back_link = None + elif request.args.get('from_job'): + back_link = url_for( + 'main.view_job', + service_id=current_service.id, + job_id=request.args.get('from_job'), + ) + else: + back_link = url_for( + 'main.view_notifications', + service_id=current_service.id, + message_type=template.template_type, + ) + return render_template( 'views/notifications/notification.html', finished=(notification['status'] in (DELIVERED_STATUSES + FAILURE_STATUSES)), @@ -133,6 +148,7 @@ def view_notification(service_id, notification_id): sent_with_test_key=( notification.get('key_type') == KEY_TYPE_TEST ), + back_link=back_link, ) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 9bf9bc4bb..7ff70cc7f 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -55,7 +55,7 @@ field_headings_visible=False ) %} {% call row_heading() %} - {{ item.to }} + {{ item.to }}

{{ item.preview_of_content }}

diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index d02282124..0446406ef 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -13,7 +13,7 @@ {{ page_header( message_count_label(1, template.template_type, suffix='') | capitalize, - back_link=None if request.args.get('help') == '0' else url_for('main.view_notifications', service_id=current_service.id, message_type=template.template_type) + back_link=back_link ) }}

diff --git a/tests/__init__.py b/tests/__init__.py index c54c5db6d..5055fc18a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -435,7 +435,7 @@ def notification_json( data = { 'notifications': [{ - 'id': uuid.uuid4(), + 'id': sample_uuid(), 'to': to, 'template': template, 'job': job_payload, diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 9d145a60e..bd28e0405 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -5,6 +5,7 @@ from flask import url_for from freezegun import freeze_time from app.main.views.jobs import get_time_left +from tests import sample_uuid from tests.conftest import ( SERVICE_ONE_ID, active_caseworking_user, @@ -218,6 +219,19 @@ def test_should_show_page_for_one_job( ) assert csv_link.text == 'Download this report' assert page.find('span', {'id': 'time-left'}).text == 'Data available for 7 days' + + assert normalize_spaces(page.select_one('tbody tr').text) == normalize_spaces( + '07123456789 ' + 'template content ' + 'Delivered 1 January at 11:10am' + ) + assert page.select_one('tbody tr a')['href'] == url_for( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=sample_uuid(), + from_job=fake_uuid, + ) + mock_get_notifications.assert_called_with( SERVICE_ONE_ID, fake_uuid, diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index b856924ba..8f9fd9e7f 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -115,6 +115,46 @@ def test_notification_status_page_respects_redaction( ) +@pytest.mark.parametrize('extra_args, expected_back_link', [ + ( + {}, + partial(url_for, 'main.view_notifications', message_type='sms'), + ), + ( + {'from_job': 'job_id'}, + partial(url_for, 'main.view_job', job_id='job_id'), + ), + ( + {'help': '0'}, + None, + ), + ( + {'help': '0', 'from_job': 'job_id'}, + None, + ), +]) +def test_notification_status_shows_expected_back_link( + client_request, + mocker, + mock_get_notification, + fake_uuid, + extra_args, + expected_back_link, +): + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + **extra_args + ) + back_link = page.select_one('.govuk-back-link') + + if expected_back_link: + assert back_link['href'] == expected_back_link(service_id=SERVICE_ONE_ID) + else: + assert back_link is None + + @freeze_time("2012-01-01 01:01") def test_notification_page_doesnt_link_to_template_in_tour( client_request, From 4fc74984153254f995bdf0fcf4dc2028781c7dbc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 15 May 2019 10:04:43 +0100 Subject: [PATCH 2/2] Include status when linking to notifications page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this argument the 'sending' filter doesn’t get highlighted by default. --- app/main/views/notifications.py | 1 + tests/app/main/views/test_notifications.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index fb6143a5a..f4defa9b7 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -114,6 +114,7 @@ def view_notification(service_id, notification_id): 'main.view_notifications', service_id=current_service.id, message_type=template.template_type, + status='sending,delivered,failed', ) return render_template( diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 8f9fd9e7f..228a662c7 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -118,7 +118,7 @@ def test_notification_status_page_respects_redaction( @pytest.mark.parametrize('extra_args, expected_back_link', [ ( {}, - partial(url_for, 'main.view_notifications', message_type='sms'), + partial(url_for, 'main.view_notifications', message_type='sms', status='sending,delivered,failed'), ), ( {'from_job': 'job_id'},