From 37327dfbb0c86163db7cb5ab5cc996bc3516be82 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 18 May 2016 09:54:50 +0100 Subject: [PATCH 1/3] Add a filter to format notification status to a readible label. --- app/__init__.py | 12 ++++++++++++ app/templates/partials/jobs/notifications.html | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 4ffa3447a..acd4f73bc 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -108,6 +108,7 @@ def create_app(): application.add_template_filter(linkable_name) application.add_template_filter(format_date) application.add_template_filter(format_date_short) + application.add_template_filter(format_notification_status) application.after_request(useful_headers_after_request) application.after_request(save_service_after_request) @@ -218,6 +219,17 @@ def valid_phone_number(phone_number): return False +def format_notification_status(status): + m = {'failed': 'Failed', + 'technical-failure': 'Technical failure', + 'temporary-failure': 'Temporarily failed', + 'permanent-failure': 'Permanently failed', + 'delivered': 'Delivered', + 'sending': 'Sending' + } + return m.get(status, status) + + @login_manager.user_loader def load_user(user_id): return user_api_client.get_user(user_id) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index f08637ce4..98b0d1854 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -27,7 +27,7 @@ align='right', status='error' if item.status == 'Failed' else 'default' ) %} - {{ item.status|title }} at {{ item.updated_at|format_time }} + {{ item.status|format_notification_status }} at {{ item.updated_at|format_time }} {% endcall %} {% endcall %} From a20fc1aa47493ca117634b3a9f9ea8b95f048bd3 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 18 May 2016 11:44:58 +0100 Subject: [PATCH 2/3] If user clicks failed or both filters for notifications status view, append all other failure states to query to api. --- app/main/views/jobs.py | 23 +++++++++++++++++++---- app/templates/views/notifications.html | 2 +- tests/app/main/views/test_jobs.py | 10 +++++----- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index bb8275d2f..a9f969deb 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -41,6 +41,20 @@ def _parse_filter_args(filter_dict): ) +def _set_status_filters(filter_args): + if filter_args.get('status'): + if 'failed' in filter_args.get('status'): + filter_args['status'].extend(['temporary-failure', 'permanent-failure', 'technical-failure']) + else: + # default to everything + 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) @@ -119,12 +133,14 @@ def view_notifications(service_id): abort(404, "Invalid page argument ({}) reverting to page 1.".format(request.args['page'], None)) 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') if 'template_type' in filter_args else ['email', 'sms'], - status=filter_args.get('status') if 'status' in filter_args else ['delivered', 'failed'], + template_type=filter_args.get('template_type'), + status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS']) view_dict = MultiDict(request.args) prev_page = None @@ -152,8 +168,7 @@ def view_notifications(service_id): page=page, page_size=notifications['total'], template_type=filter_args.get('template_type') if 'template_type' in filter_args else ['email', 'sms'], - status=filter_args.get('status') - if 'status' in filter_args else ['delivered', 'failed'], + status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])['notifications']) return csv_content, 200, { 'Content-Type': 'text/csv; charset=utf-8', diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 3e9b87438..3be1a0792 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -91,7 +91,7 @@

{% endcall %} - {{ text_field(item.status|capitalize) }} + {{ text_field(item.status|capitalize|replace('-', ' ')) }} {% call field(align='right') %} {{ item.updated_at|format_datetime_short }} diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index f0d834fbc..ff0de4afe 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -95,7 +95,7 @@ def test_should_show_notifications_for_a_service(app_, 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'], template_type=['email', 'sms']) # noqa + 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_, @@ -123,7 +123,7 @@ def test_can_view_only_sms_notifications_for_a_service(app_, page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Text messages' - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['sms']) # noqa + 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_, @@ -151,7 +151,7 @@ def test_can_view_only_email_notifications_for_a_service(app_, page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Emails' - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['email']) # noqa + 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_, @@ -203,7 +203,7 @@ def test_can_view_failed_notifications_for_a_service(app_, page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Failed emails and text messages' - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed'], 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', 'sms']) # noqa def test_can_view_failed_combination_of_notification_type_and_status( @@ -225,7 +225,7 @@ def test_can_view_failed_combination_of_notification_type_and_status( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Failed text messages' - mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed'], template_type=['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=['sms']) # noqa def test_should_show_notifications_for_a_service_with_next_previous(app_, From 6c14d925c59f755c891930309508e92682b436b4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 18 May 2016 12:05:26 +0100 Subject: [PATCH 3/3] Fix test to use a real status --- app/templates/views/notifications.html | 2 +- tests/__init__.py | 2 +- tests/app/main/views/test_jobs.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 3be1a0792..6b7d30d29 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -91,7 +91,7 @@

{% endcall %} - {{ text_field(item.status|capitalize|replace('-', ' ')) }} + {{ text_field(item.status|format_notification_status) }} {% call field(align='right') %} {{ item.updated_at|format_datetime_short }} diff --git a/tests/__init__.py b/tests/__init__.py index d78aaec33..bd0265812 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -159,7 +159,7 @@ def notification_json(service_id, job=None, template=None, to='07123456789', - status='sent', + status='delivered', sent_at=None, created_at=None, updated_at=None, diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index ff0de4afe..5e4007fb4 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -47,7 +47,7 @@ def test_should_show_page_for_one_job( content = response.get_data(as_text=True) assert "{}: Your vehicle tax is about to expire".format(service_one['name']) in content assert file_name in content - assert "Sent at 11:10" in content + assert "Delivered at 11:10" in content def test_should_show_updates_for_one_job_as_json(