From b0cb9ff58e20e0e1fd32f4f8cb25d9c9bc095852 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 13 Jun 2018 14:16:10 +0100 Subject: [PATCH] Add sent notifications page for caseworkers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The other task that caseworkers have to do (much less often than sending messages) is look at the messages which they’ve sent. The reason for doing this is usually to find a specific message which someone has complained about. This commit adds: - a page where they can do that - a navigation item so they can get to that page We reckon that because this is about finding specific messages, not reporting that it’s fine to mush all the channels (email, text, letter) into one table. --- app/main/views/jobs.py | 44 ++++++++++++++++++-------- app/navigation.py | 4 ++- app/templates/main_nav.html | 4 +++ app/templates/views/notifications.html | 1 - tests/app/main/views/test_activity.py | 10 ++++++ tests/app/test_navigation.py | 2 +- 6 files changed, 48 insertions(+), 17 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index fd9e80c30..5527c6367 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -164,10 +164,11 @@ def view_job_updates(service_id, job_id): )) +@main.route('/services//notifications', methods=['GET', 'POST']) @main.route('/services//notifications/', methods=['GET', 'POST']) @login_required -@user_has_permissions('view_activity') -def view_notifications(service_id, message_type): +@user_has_permissions('view_activity', 'send_messages') +def view_notifications(service_id, message_type=None): return render_template( 'views/notifications.html', partials=get_notifications(service_id, message_type), @@ -185,22 +186,23 @@ def view_notifications(service_id, message_type): ) +@main.route('/services//notifications.json', methods=['GET', 'POST']) @main.route('/services//notifications/.json', methods=['GET', 'POST']) -@user_has_permissions('view_activity') -def get_notifications_as_json(service_id, message_type): +@user_has_permissions('view_activity', 'send_messages') +def get_notifications_as_json(service_id, message_type=None): return jsonify(get_notifications( service_id, message_type, status_override=request.args.get('status') )) @main.route('/services//notifications/.csv', endpoint="view_notifications_csv") -@user_has_permissions('view_activity') +@user_has_permissions('view_activity', 'send_messages') def get_notifications(service_id, message_type, status_override=None): # TODO get the api to return count of pages as well. page = get_page_from_request() if page is None: abort(404, "Invalid page argument ({}).".format(request.args.get('page'))) - if message_type not in ['email', 'sms', 'letter']: + if message_type not in ['email', 'sms', 'letter', None]: abort(404) filter_args = parse_filter_args(request.args) filter_args['status'] = set_status_filters(filter_args) @@ -221,7 +223,7 @@ def get_notifications(service_id, message_type, status_override=None): notifications = notification_api_client.get_notifications_for_service( service_id=service_id, page=page, - template_type=[message_type], + template_type=[message_type] if message_type else [], status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'], to=request.form.get('to', ''), @@ -239,6 +241,16 @@ def get_notifications(service_id, message_type, status_override=None): if 'links' in notifications and notifications['links'].get('next', None): next_page = generate_next_dict('main.view_notifications', service_id, page, url_args) + if message_type: + download_link = url_for( + '.view_notifications_csv', + service_id=current_service['id'], + message_type=message_type, + status=request.args.get('status') + ) + else: + download_link = None + return { 'counts': render_template( 'views/activity/counts.html', @@ -259,18 +271,22 @@ def get_notifications(service_id, message_type, status_override=None): next_page=next_page, status=request.args.get('status'), message_type=message_type, - download_link=url_for( - '.view_notifications_csv', - service_id=current_service['id'], - message_type=message_type, - status=request.args.get('status') - ) + download_link=download_link, ), } def get_status_filters(service, message_type, statistics): - stats = statistics[message_type] + if message_type is None: + stats = { + key: sum( + statistics[message_type][key] + for message_type in {'email', 'sms', 'letter'} + ) + for key in {'requested', 'delivered', 'failed'} + } + else: + stats = statistics[message_type] stats['sending'] = stats['requested'] - stats['delivered'] - stats['failed'] filters = [ diff --git a/app/navigation.py b/app/navigation.py index 7a85cd378..056d66110 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -506,6 +506,9 @@ class CaseworkNavigation(Navigation): 'send_one_off_step', 'send_test', 'send_test_step', + }, + 'sent-messages': { + 'view_notifications', 'view_notification', }, } @@ -704,7 +707,6 @@ class CaseworkNavigation(Navigation): 'view_letter_notification_as_preview', 'view_letter_template_preview', 'view_notification_updates', - 'view_notifications', 'view_notifications_csv', 'view_provider', 'view_providers', diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 623bf3f40..5ff2dac9f 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -22,5 +22,9 @@ + + {% endif %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 56e2be966..50a6b8be3 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -29,7 +29,6 @@ {{ textbox( search_form.to, width='1-1', - label='Search by {}'.format('email address' if message_type == 'email' else 'phone number') ) }}
diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index e7408d216..de1f4d420 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -10,11 +10,17 @@ from freezegun import freeze_time from app.main.views.jobs import get_status_filters, get_time_left from tests.conftest import ( SERVICE_ONE_ID, + active_caseworking_user, + active_user_view_permissions, mock_get_notifications, normalize_spaces, ) +@pytest.mark.parametrize('user', ( + active_user_view_permissions, + active_caseworking_user, +)) @pytest.mark.parametrize( "message_type,page_title", [ ('email', 'Emails'), @@ -72,7 +78,11 @@ def test_can_show_notifications( expected_page_argument, to_argument, expected_to_argument, + mocker, + user, + fake_uuid, ): + mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid)) if expected_to_argument: response = logged_in_client.post( url_for( diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 97f222a04..3cbada0e5 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -159,5 +159,5 @@ def test_caseworkers_get_caseworking_navigation( ) page = client_request.get('main.choose_template', service_id=SERVICE_ONE_ID) assert normalize_spaces(page.select_one('#content nav').text) == ( - 'Send a message' + 'Send a message Sent messages' )