diff --git a/app/__init__.py b/app/__init__.py index 9463305c3..2f39d2bee 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -243,7 +243,8 @@ def format_notification_status(status, template_type): 'temporary-failure': 'Inbox not accepting messages right now', 'permanent-failure': 'Email address doesn’t exist', 'delivered': 'Delivered', - 'sending': 'Sending' + 'sending': 'Sending', + 'created': 'Sending' }, 'sms': { 'failed': 'Failed', @@ -251,7 +252,8 @@ def format_notification_status(status, template_type): 'temporary-failure': 'Phone not accepting messages right now', 'permanent-failure': 'Phone number doesn’t exist', 'delivered': 'Delivered', - 'sending': 'Sending' + 'sending': 'Sending', + 'created': 'Sending' } }.get(template_type).get(status, status) @@ -263,7 +265,8 @@ def format_notification_status_as_field_status(status): 'temporary-failure': 'error', 'permanent-failure': 'error', 'delivered': None, - 'sending': 'default' + 'sending': 'default', + 'created': 'default' }.get(status, 'error') diff --git a/app/assets/stylesheets/_grids.scss b/app/assets/stylesheets/_grids.scss index 5231b67ae..183a85630 100644 --- a/app/assets/stylesheets/_grids.scss +++ b/app/assets/stylesheets/_grids.scss @@ -31,6 +31,10 @@ margin-bottom: $gutter-two-thirds; } +.bottom-gutter-1-2 { + margin-bottom: $gutter-half; +} + .align-with-heading { display: block; text-align: center; diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index f043ddfca..749fbfa31 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,8 +1,10 @@ # -*- coding: utf-8 -*- +import ago import time import dateutil +from orderedset import OrderedSet from datetime import datetime, timedelta, timezone -import ago +from itertools import chain from flask import ( render_template, @@ -47,15 +49,14 @@ def _parse_filter_args(filter_dict): def _set_status_filters(filter_args): + status_filters = filter_args.get('status', []) all_failure_statuses = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] - all_statuses = ['sending', 'delivered'] + all_failure_statuses - if filter_args.get('status'): - if 'processed' in filter_args.get('status') or not filter_args.get('status'): - filter_args['status'] = all_statuses - elif 'failed' in filter_args.get('status'): - filter_args['status'].extend(all_failure_statuses[1:]) - else: - filter_args['status'] = all_statuses + all_sending_statuses = ['created', 'sending'] + return list(OrderedSet(chain( + (status_filters or all_sending_statuses + ['delivered'] + all_failure_statuses), + all_sending_statuses if 'sending' in status_filters else [], + all_failure_statuses if 'failed' in status_filters else [] + ))) @main.route("/services//jobs") @@ -75,7 +76,7 @@ def view_job(service_id, job_id): job = job_api_client.get_job(service_id, job_id)['data'] filter_args = _parse_filter_args(request.args) - _set_status_filters(filter_args) + filter_args['status'] = _set_status_filters(filter_args) return render_template( 'views/jobs/job.html', @@ -117,7 +118,7 @@ def view_job_csv(service_id, job_id): version=job['template_version'] )['data'] filter_args = _parse_filter_args(request.args) - _set_status_filters(filter_args) + filter_args['status'] = _set_status_filters(filter_args) return ( generate_notifications_csv( @@ -161,7 +162,7 @@ def view_notifications(service_id, message_type): abort(404) filter_args = _parse_filter_args(request.args) - _set_status_filters(filter_args) + filter_args['status'] = _set_status_filters(filter_args) notifications = notification_api_client.get_notifications_for_service( service_id=service_id, @@ -210,7 +211,7 @@ def view_notifications(service_id, message_type): page=page, prev_page=prev_page, next_page=next_page, - request_args=request.args, + status=request.args.get('status'), message_type=message_type, download_link=url_for( '.view_notifications_csv', @@ -250,7 +251,7 @@ def get_status_filters(service, message_type, statistics): filters = [ # key, label, option - ('requested', 'processed', 'sending,delivered,failed'), + ('requested', 'total', 'sending,delivered,failed'), ('sending', 'sending', 'sending'), ('delivered', 'delivered', 'delivered'), ('failed', 'failed', 'failed'), @@ -287,24 +288,22 @@ def _get_job_counts(job, help_argument): count ) for label, query_param, count in [ [ - 'Processed', '', - job.get('notifications_sent', 0) + 'total', '', + job.get('notification_count', 0) ], [ - 'Sending', 'sending', - ( - job.get('notifications_sent', 0) - - job.get('notifications_delivered', 0) - - job.get('notifications_failed', 0) - ) + 'sending', 'sending', + job.get('notification_count', 0) - + job.get('notifications_delivered', 0) - + job.get('notifications_failed', 0) ], [ - 'Delivered', 'delivered', + 'delivered', 'delivered', job.get('notifications_delivered', 0) ], [ - 'Failed', 'failed', - job.get('notifications_failed') + 'failed', 'failed', + job.get('notifications_failed', 0) ] ] ] @@ -312,24 +311,27 @@ def _get_job_counts(job, help_argument): def get_job_partials(job): filter_args = _parse_filter_args(request.args) - _set_status_filters(filter_args) + filter_args['status'] = _set_status_filters(filter_args) + notifications = notification_api_client.get_notifications_for_service( + job['service'], job['id'], status=filter_args['status'] + ) return { 'counts': render_template( 'partials/jobs/count.html', job=job, counts=_get_job_counts(job, request.args.get('help', 0)), - status=request.args.get('status', '') + status=filter_args['status'] ), 'notifications': render_template( 'partials/jobs/notifications.html', - notifications=notification_api_client.get_notifications_for_service( - job['service'], job['id'], status=filter_args.get('status') - )['notifications'], + notifications=notifications['notifications'], + more_than_one_page=bool(notifications.get('links', {}).get('next')), + percentage_complete=(job['notifications_sent'] / job['notification_count'] * 100), download_link=url_for( '.view_job_csv', service_id=current_service['id'], job_id=job['id'], - status=request.args.get('status', '') + status=request.args.get('status') ), help=get_help_argument(), time_left=get_time_left(job['created_at']) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 27243bfbb..f915e6a11 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -4,12 +4,18 @@
{% endif %} - {% if notifications and not help %} -

- Download this report -   - {{ time_left }} -

+ {% if not help %} + {% if percentage_complete < 100 %} +

+ Report is {{ "{:.0f}%".format(percentage_complete) }} complete… +

+ {% elif notifications %} +

+ Download this report +   + {{ time_left }} +

+ {% endif %} {% endif %} {% call(item, row_number) list_table( @@ -44,6 +50,12 @@ {% endcall %} {% endcall %} + {% if more_than_one_page %} +

+ Only showing the first 50 rows +

+ {% endif %} + {% if notifications %}
{% endif %} diff --git a/app/templates/views/dashboard/_jobs.html b/app/templates/views/dashboard/_jobs.html index b414e4456..0a2b3a2f7 100644 --- a/app/templates/views/dashboard/_jobs.html +++ b/app/templates/views/dashboard/_jobs.html @@ -23,7 +23,7 @@ {% endcall %} {% call field() %} {{ big_number( - item.get('notifications_sent', 0) - item.get('notifications_delivered', 0) - item.get('notifications_failed', 0), + item.get('notification_count', 0) - item.get('notifications_delivered', 0) - item.get('notifications_failed', 0), smallest=True ) }} {% endcall %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 69d8ed40e..78b285549 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -14,9 +14,9 @@

@@ -29,15 +29,15 @@ {{ pill( 'Status', status_filters, - request_args.get('status', '') + status ) }} {% if notifications %} -

- Download as a CSV file +

+ Download this report   - Delivery information is available for 7 days + Data available for 7 days

{% endif %} diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 7e3378e63..7bfbcefb2 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -32,15 +32,11 @@ def test_should_return_list_of_all_jobs(app_, "status_argument, expected_api_call", [ ( '', - ['sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] - ), - ( - 'processed', - ['sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] + ['created', 'sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] ), ( 'sending', - ['sending'] + ['sending', 'created'] ), ( 'delivered', @@ -99,6 +95,7 @@ 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 page.find('p', {'class': 'table-show-more-link'}).text.strip() == 'Only showing the first 50 rows' mock_get_notifications.assert_called_with( service_one['id'], fake_uuid, @@ -106,6 +103,29 @@ def test_should_show_page_for_one_job( ) +def test_should_show_job_in_progress( + app_, + service_one, + active_user_with_permissions, + mock_get_service_template, + mock_get_job_in_progress, + mocker, + mock_get_notifications, + fake_uuid +): + with app_.test_request_context(), app_.test_client() as client: + client.login(active_user_with_permissions, mocker, service_one) + response = client.get(url_for( + 'main.view_job', + service_id=service_one['id'], + job_id=fake_uuid + )) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.find('p', {'class': 'hint'}).text.strip() == 'Report is 50% complete…' + + def test_should_show_not_show_csv_download_in_tour( app_, service_one, @@ -179,12 +199,12 @@ def test_should_show_updates_for_one_job_as_json( @pytest.mark.parametrize( "status_argument, expected_api_call", [ ( - 'processed', - ['sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] + '', + ['created', 'sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] ), ( 'sending', - ['sending'] + ['sending', 'created'] ), ( 'delivered', @@ -275,10 +295,21 @@ def test_should_show_notifications_for_a_service_with_next_previous( )) assert response.status_code == 200 content = response.get_data(as_text=True) - 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 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + next_page_link = page.find('a', {'rel': 'next'}) + prev_page_link = page.find('a', {'rel': 'previous'}) + assert ( + url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=3) in + next_page_link['href'] + ) + assert 'Next page' in next_page_link.text.strip() + assert 'page 3' in next_page_link.text.strip() + assert ( + url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=1) in + prev_page_link['href'] + ) + assert 'Previous page' in prev_page_link.text.strip() + assert 'page 1' in prev_page_link.text.strip() @freeze_time("2016-01-01 11:09:00.061258") @@ -335,7 +366,7 @@ def test_get_status_filters_calculates_stats(app_): ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS) assert {label: count for label, _option, _link, count in ret} == { - 'processed': 6, + 'total': 6, 'sending': 3, 'failed': 2, 'delivered': 1 @@ -347,7 +378,7 @@ def test_get_status_filters_in_right_order(app_): ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS) assert [label for label, _option, _link, _count in ret] == [ - 'processed', 'sending', 'delivered', 'failed' + 'total', 'sending', 'delivered', 'failed' ] diff --git a/tests/conftest.py b/tests/conftest.py index 2ace067a9..1535d0080 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -858,6 +858,18 @@ def mock_get_job(mocker, api_user_active): return mocker.patch('app.job_api_client.get_job', side_effect=_get_job) +@pytest.fixture(scope='function') +def mock_get_job_in_progress(mocker, api_user_active): + def _get_job(service_id, job_id): + return {"data": job_json( + service_id, api_user_active, job_id=job_id, + notification_count=10, + notifications_sent=5 + )} + + return mocker.patch('app.job_api_client.get_job', side_effect=_get_job) + + @pytest.fixture(scope='function') def mock_get_jobs(mocker, api_user_active): def _get_jobs(service_id, limit_days=None): @@ -898,14 +910,16 @@ def mock_get_notifications(mocker, api_user_active): template={'template_type': set_template_type, 'name': 'name', 'id': 'id', 'version': 1}, rows=rows, status=set_status, - job=job + job=job, + with_links=True ) else: return notification_json( service_id, rows=rows, status=set_status, - job=job + job=job, + with_links=True ) return mocker.patch(