diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index a2bb1b944..de0e90d8f 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -51,11 +51,6 @@ def _set_status_filters(filter_args): filter_args['status'] = ['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] -def _set_template_filters(filter_args): - if not filter_args.get('template_type'): - filter_args['template_type'] = ['email', 'sms'] - - @main.route("/services//jobs") @login_required @user_has_permissions('view_activity', admin_override=True) @@ -144,26 +139,30 @@ def view_job_updates(service_id, job_id): }) -@main.route('/services//notifications') +@main.route('/services//notifications/') @login_required @user_has_permissions('view_activity', admin_override=True) -def view_notifications(service_id): +def view_notifications(service_id, message_type): # 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 ({}) reverting to page 1.".format(request.args['page'], None)) + if message_type not in ['email', 'sms']: + abort(404) filter_args = _parse_filter_args(request.args) _set_status_filters(filter_args) - _set_template_filters(filter_args) notifications = notification_api_client.get_notifications_for_service( service_id=service_id, page=page, - template_type=filter_args.get('template_type'), + template_type=[message_type], status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS']) - view_dict = MultiDict(request.args) + view_dict = dict( + message_type=message_type, + status=request.args.get('status') + ) prev_page = None if notifications['links'].get('prev', None): prev_page = generate_previous_next_dict( @@ -188,7 +187,7 @@ def view_notifications(service_id): service_id=service_id, page=page, page_size=notifications['total'], - template_type=filter_args.get('template_type') if 'template_type' in filter_args else ['email', 'sms'], + template_type=[message_type], status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])['notifications']) return csv_content, 200, { @@ -202,28 +201,17 @@ def view_notifications(service_id): prev_page=prev_page, next_page=next_page, request_args=request.args, - type_filters=[ - [item[0], item[1], url_for( - '.view_notifications', - service_id=current_service['id'], - template_type=item[1], - status=request.args.get('status', 'delivered,failed') - )] for item in [ - ['Emails', 'email'], - ['Text messages', 'sms'], - ['Both', 'email,sms'] - ] - ], + message_type=message_type, status_filters=[ [item[0], item[1], url_for( '.view_notifications', service_id=current_service['id'], - template_type=request.args.get('template_type', 'email,sms'), + message_type=message_type, status=item[1] )] for item in [ ['Successful', 'delivered'], ['Failed', 'failed'], - ['Both', 'delivered,failed'] + ['', 'delivered,failed'] ] ] ) diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index 1b34d6e76..cb50dd08f 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -18,8 +18,8 @@ statistics.emails_failed, statistics.get('emails_failure_rate', 0.0), statistics.get('emails_failure_rate', 0)|float > 3, - failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='failed'), - label_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='delivered,failed') + failure_link=url_for(".view_notifications", service_id=current_service.id, message_type='email', status='failed'), + label_link=url_for(".view_notifications", service_id=current_service.id, message_type='email', status='delivered,failed') ) }}
@@ -29,8 +29,8 @@ statistics.sms_failed, statistics.get('sms_failure_rate', 0.0), statistics.get('sms_failure_rate', 0)|float > 3, - failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='failed'), - label_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='delivered,failed') + failure_link=url_for(".view_notifications", service_id=current_service.id, message_type='sms', status='failed'), + label_link=url_for(".view_notifications", service_id=current_service.id, message_type='sms', status='delivered,failed') ) }}
diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index e811692f2..ef465dcc8 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -3,60 +3,34 @@ {% from "components/previous-next-navigation.html" import previous_next_navigation %} {% from "components/page-footer.html" import page_footer %} {% from "components/pill.html" import pill %} +{% from "components/message-count-label.html" import message_count_label %} {% block page_title %} - Activity – GOV.UK Notify + {{ message_count_label(99, message_type, suffix='') | capitalize }} – GOV.UK Notify {% endblock %} {% block maincolumn_content %}

- {%- if (request_args.get('template_type', 'email,sms') == 'email,sms') and (request_args.get('status', 'delivered,failed') == 'delivered,failed') -%} - - Activity - - {%- else -%} + - {%- if request_args.get('template_type', 'email,sms') == 'email,sms' %} emails and text messages - {%- else -%} - - {%- for template_label, template_option, _ in type_filters -%} - {%- if request_args.get('template_type') == template_option -%} - {%- if request_args.get('status', 'delivered,failed') == 'delivered,failed' -%} - {{ template_label }} - {%- else -%} - {{ template_label | lower }} - {%- endif -%} - {%- endif -%} - {%- endfor -%} - - {%- endif -%} - - {%- endif -%} + {{- message_count_label(99, message_type, suffix='') | capitalize }}

-
-
- {{ pill( - 'Status', - status_filters, - request_args.get('status', '') - ) }} -
-
- {{ pill( - 'Type', - type_filters, - request_args.get('template_type', '') - ) }} -
+
+ {{ pill( + 'Status', + status_filters, + request_args.get('status', '') + ) }}
{% if notifications %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 114ae5437..d143166d2 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -175,7 +175,6 @@ def test_menu_send_messages(mocker, 'main.choose_template', service_id=service_one['id'], template_type='sms')in page - assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.documentation') in page @@ -209,7 +208,6 @@ def test_menu_manage_service(mocker, 'main.choose_template', service_id=service_one['id'], template_type='sms') in page - assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.service_settings', service_id=service_one['id']) in page assert url_for('main.documentation') in page @@ -242,7 +240,6 @@ def test_menu_manage_api_keys(mocker, 'main.choose_template', service_id=service_one['id'], template_type='sms') in page - assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.service_settings', service_id=service_one['id']) not in page assert url_for('main.show_all_services') not in page @@ -270,7 +267,8 @@ def test_menu_all_services_for_platform_admin_user(mocker, assert url_for('main.choose_template', service_id=service_one['id'], template_type='email') in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.service_settings', service_id=service_one['id']) in page - assert url_for('main.view_notifications', service_id=service_one['id']) in page + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page assert url_for('main.api_keys', service_id=service_one['id']) not in page # Should this be here?? diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 3e5b1268e..27cbee52f 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -78,41 +78,20 @@ def test_should_show_updates_for_one_job_as_json( assert 'Uploaded by Test User on 1 January at 11:09' in content['status'] -def test_should_show_notifications_for_a_service(app_, - service_one, - active_user_with_permissions, - mock_get_notifications, - mocker): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(active_user_with_permissions, mocker, service_one) - response = client.get(url_for('main.view_notifications', service_id=service_one['id'])) - assert response.status_code == 200 - 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 - assert 'csv' in content - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Activity' - - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email', 'sms']) # noqa - - -def test_can_view_only_sms_notifications_for_a_service(app_, - service_one, - active_user_with_permissions, - mock_get_notifications, - mocker): +def test_can_see_sms( + app_, + service_one, + active_user_with_permissions, + mock_get_notifications, + mocker +): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user_with_permissions, mocker, service_one) response = client.get(url_for( 'main.view_notifications', service_id=service_one['id'], - template_type='sms', + message_type='sms', status='delivered,failed')) assert response.status_code == 200 content = response.get_data(as_text=True) @@ -124,16 +103,18 @@ def test_can_view_only_sms_notifications_for_a_service(app_, assert notification['template']['name'] in content assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Text messages' + assert page.h1.text.strip() == 'Text messages' mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['sms']) # noqa -def test_can_view_only_email_notifications_for_a_service(app_, - service_one, - active_user_with_permissions, - mock_get_notifications, - mocker): +def test_can_see_emails( + app_, + service_one, + active_user_with_permissions, + mock_get_notifications, + mocker +): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user_with_permissions, mocker, service_one) @@ -141,7 +122,7 @@ def test_can_view_only_email_notifications_for_a_service(app_, 'main.view_notifications', service_id=service_one['id'], status='delivered,failed', - template_type='email')) + message_type='email')) assert response.status_code == 200 content = response.get_data(as_text=True) @@ -152,48 +133,25 @@ def test_can_view_only_email_notifications_for_a_service(app_, assert notification['template']['name'] in content assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Emails' + assert page.h1.text.strip() == 'Emails' mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email']) # noqa -def test_can_view_successful_notifications_for_a_service(app_, - service_one, - active_user_with_permissions, - mock_get_notifications, - mocker): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(active_user_with_permissions, mocker, service_one) - response = client.get(url_for( - 'main.view_notifications', - service_id=service_one['id'], - status='delivered')) - assert response.status_code == 200 - 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 - assert 'csv' in content - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Successful emails and text messages' - - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered'], template_type=['email', 'sms']) # noqa - - -def test_can_view_failed_notifications_for_a_service(app_, - service_one, - active_user_with_permissions, - mock_get_notifications, - mocker): +def test_can_view_failed_emails( + app_, + service_one, + active_user_with_permissions, + mock_get_notifications, + mocker +): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user_with_permissions, mocker, service_one) response = client.get(url_for( 'main.view_notifications', service_id=service_one['id'], + message_type='email', status='failed')) assert response.status_code == 200 content = response.get_data(as_text=True) @@ -204,12 +162,12 @@ def test_can_view_failed_notifications_for_a_service(app_, assert notification['template']['name'] in content assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Failed emails and text messages' + assert page.h1.text.strip() == 'Failed Emails' - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email', 'sms']) # noqa + mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email']) # noqa -def test_can_view_failed_combination_of_notification_type_and_status( +def test_can_view_failed_sms_messages( app_, service_one, active_user_with_permissions, @@ -223,10 +181,10 @@ def test_can_view_failed_combination_of_notification_type_and_status( 'main.view_notifications', service_id=service_one['id'], status='failed', - template_type='sms')) + message_type='sms')) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Failed text messages' + assert page.h1.text.strip() == 'Failed Text messages' mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['sms']) # noqa @@ -239,11 +197,16 @@ def test_should_show_notifications_for_a_service_with_next_previous(app_, with app_.test_request_context(): with app_.test_client() as client: client.login(active_user_with_permissions, mocker, service_one) - response = client.get(url_for('main.view_notifications', service_id=service_one['id'], page=2)) + response = client.get(url_for( + 'main.view_notifications', + service_id=service_one['id'], + message_type='sms', + page=2 + )) assert response.status_code == 200 content = response.get_data(as_text=True) - assert url_for('main.view_notifications', service_id=service_one['id'], page=3) in content - assert url_for('main.view_notifications', service_id=service_one['id'], page=1) in content + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=3) in content + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=1) in content assert 'Previous page' in content assert 'Next page' in content @@ -260,6 +223,7 @@ def test_should_download_notifications_for_a_service(app_, response = client.get(url_for( 'main.view_notifications', service_id=service_one['id'], + message_type='email', download='csv')) csv_content = generate_notifications_csv( mock_get_notifications(service_one['id'])['notifications'])