diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index d2cfba1b0..3afc5320a 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -42,7 +42,8 @@ position: relative; margin-bottom: $gutter-two-thirds; - .big-number { + .big-number, + .big-number-smaller { padding: $gutter-half; position: relative; background: $black; diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 7f47b081a..78e231fdf 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -215,6 +215,7 @@ def get_dashboard_partials(service_id): for job in job_api_client.get_jobs(service_id, limit_days=7, statuses=statuses_to_display)['data'] ] service = service_api_client.get_detailed_service(service_id) + column_width = 'column-third' if 'letter' in current_service['permissions'] else 'column-half' return { 'upcoming': render_template( @@ -231,7 +232,8 @@ def get_dashboard_partials(service_id): 'totals': render_template( 'views/dashboard/_totals.html', service_id=service_id, - statistics=get_dashboard_totals(service['data']['statistics']) + statistics=get_dashboard_totals(service['data']['statistics']), + column_width=column_width ), 'template-statistics': render_template( 'views/dashboard/template-statistics.html', diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index d34b9ac1e..6e5a2cc12 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -225,7 +225,7 @@ def get_notifications(service_id, message_type, status_override=None): 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']: + if message_type not in ['email', 'sms', 'letter']: abort(404) filter_args = _parse_filter_args(request.args) filter_args['status'] = _set_status_filters(filter_args) @@ -243,7 +243,6 @@ def get_notifications(service_id, message_type, status_override=None): headers={ 'Content-Disposition': 'inline; filename="notifications.csv"'} ) - notifications = notification_api_client.get_notifications_for_service( service_id=service_id, page=page, diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 38735279b..076ce4d7d 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -105,11 +105,16 @@ def create_global_stats(services): 'delivered': 0, 'failed': 0, 'requested': 0 + }, + 'letter': { + 'delivered': 0, + 'failed': 0, + 'requested': 0 } } for service in services: - for msg_type, status in itertools.product(('sms', 'email'), ('delivered', 'failed', 'requested')): + for msg_type, status in itertools.product(('sms', 'email', 'letter'), ('delivered', 'failed', 'requested')): stats[msg_type][status] += service['statistics'][msg_type][status] for stat in stats.values(): diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index cec5346cc..d46e6caba 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -49,7 +49,6 @@ class NotificationApiClient(NotifyAdminAPIClient): else: if limit_days is not None: params['limit_days'] = limit_days - return self.get( url='/service/{}/notifications'.format(service_id), params=params diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index 048a0f3df..454a0cc05 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -32,10 +32,12 @@ danger_zone=False, failure_link=None, link=None, - show_failures=True + show_failures=True, + smaller=False, + smallest=False ) %}
- {{ big_number(number, label, link=link) }} + {{ big_number(number, label, link=link, smaller=smaller, smallest=smallest) }} {% if show_failures %}
{% if failures %} diff --git a/app/templates/components/table.html b/app/templates/components/table.html index a9f9fa2ec..aaf0d635a 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -126,17 +126,23 @@ {% if notification.status|format_notification_status_as_url %} {% endif %} - {{ notification.status|format_notification_status( - notification.template.template_type - ) }} + {% if notification['notification_type'] != "letter" %} + {{ notification.status|format_notification_status( + notification.template.template_type + ) }} + {% endif %} {% if notification.status|format_notification_status_as_url %} {% endif %} - {{ notification.status|format_notification_status_as_time( - notification.created_at|format_datetime_short, - (notification.updated_at or notification.created_at)|format_datetime_short - ) }} + {% if notification['notification_type'] == "letter" %} + {{ notification.created_at|format_datetime_short }} + {% else %} + {{ notification.status|format_notification_status_as_time( + notification.created_at|format_datetime_short, + (notification.updated_at or notification.created_at)|format_datetime_short + ) }} + {% endif %} {% if notification.status in ['created', 'sending', 'delivered'] %}{% endif %} {% endcall %} diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 997e72559..70a086715 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -55,9 +55,7 @@ {{ item.preview_of_content }}

{% endcall %} - {{ notification_status_field( - '' if template.template_type == 'letter' else item - ) }} + {{ notification_status_field(item) }} {% endcall %} {% if more_than_one_page %} diff --git a/app/templates/views/activity/notifications.html b/app/templates/views/activity/notifications.html index cabcd436b..239b7dca0 100644 --- a/app/templates/views/activity/notifications.html +++ b/app/templates/views/activity/notifications.html @@ -15,7 +15,6 @@ field_headings=['Recipient', 'Status'], field_headings_visible=False ) %} - {% call row_heading() %} {{ item.to }}

diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index 281a66f7b..469a4b266 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -3,7 +3,7 @@

-
+
{{ big_number_with_status( statistics['email']['requested'], message_count_label(statistics['email']['requested'], 'email', suffix='sent'), @@ -14,7 +14,7 @@ link=url_for(".view_notifications", service_id=service_id, message_type='email', status='sending,delivered,failed') ) }}
-
+
{{ big_number_with_status( statistics['sms']['requested'], message_count_label(statistics['sms']['requested'], 'sms', suffix='sent'), @@ -25,5 +25,18 @@ link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='sending,delivered,failed') ) }}
+ {% if 'letter' in current_service['permissions'] %} +
+ {{ big_number_with_status( + statistics['letter']['requested'], + message_count_label(statistics['letter']['requested'], 'letter', suffix='sent'), + statistics['letter']['failed'], + statistics['letter']['failed_percentage'], + statistics['letter']['show_warning'], + failure_link=url_for(".view_notifications", service_id=service_id, message_type='letter', status='failed'), + link=url_for(".view_notifications", service_id=service_id, message_type='letter', status='') + ) }} +
+ {% endif %}
diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 7540720b2..40bf914a9 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -13,35 +13,36 @@

{{ message_count_label(99, message_type, suffix='') | capitalize }}

+ {% if not message_type == "letter" %} + {{ ajax_block( + partials, + url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status), + 'counts' + ) }} - {{ ajax_block( - partials, - url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status), - 'counts' - ) }} +
+
+ {{ textbox( + search_form.to, + width='1-1', + label='Search by {}'.format('email address' if message_type == 'email' else 'phone number') + ) }} +
+
+ + +
+
-
-
- {{ textbox( - search_form.to, - width='1-1', - label='Search by {}'.format('email address' if message_type == 'email' else 'phone number') - ) }} -
-
+ + - -
-
- -
- - -
+ + {% endif %} {{ ajax_block( partials, diff --git a/app/templates/views/platform-admin/_global_stats.html b/app/templates/views/platform-admin/_global_stats.html index e92e17fb2..0a8f8d688 100644 --- a/app/templates/views/platform-admin/_global_stats.html +++ b/app/templates/views/platform-admin/_global_stats.html @@ -1,22 +1,34 @@ {% from "components/big-number.html" import big_number_with_status %} {% from "components/message-count-label.html" import message_count_label %}
-
+
{{ big_number_with_status( global_stats.email.delivered + global_stats.email.failed, message_count_label(global_stats.email.delivered, 'email'), global_stats.email.failed, global_stats.email.failure_rate, global_stats.email.failure_rate|float > 3, + smaller=True ) }}
-
+
{{ big_number_with_status( global_stats.sms.delivered + global_stats.sms.failed, message_count_label(global_stats.sms.delivered, 'sms'), global_stats.sms.failed, global_stats.sms.failure_rate, global_stats.sms.failure_rate|float > 3, + smaller=True + ) }} +
+
+ {{ big_number_with_status( + global_stats.letter.delivered + global_stats.letter.failed, + message_count_label(global_stats.letter.delivered, 'letter'), + global_stats.letter.failed, + global_stats.letter.failure_rate, + global_stats.letter.failure_rate|float > 3, + smaller=True ) }}
diff --git a/tests/__init__.py b/tests/__init__.py index 301b6dc0d..ed53da305 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -217,7 +217,7 @@ def notification_json( service_id, job=None, template=None, - to='07123456789', + to=None, status=None, sent_at=None, job_row_number=None, @@ -230,6 +230,13 @@ def notification_json( ): if template is None: template = template_json(service_id, str(generate_uuid()), type_=template_type) + if to is None: + if template_type == 'letter': + to = '1 Example Street' + elif template_type == 'email': + to = 'example@gov.uk' + else: + to = '07123456789' if sent_at is None: sent_at = str(datetime.utcnow().time()) if created_at is None: @@ -266,7 +273,7 @@ def notification_json( 'service': service_id, 'template_version': template['version'], 'personalisation': personalisation or {}, - 'notification_type': 'sms', + 'notification_type': template_type, } for i in range(rows)], 'total': rows, 'page_size': 50, diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 0ac91737a..91d08e61d 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -8,8 +8,14 @@ from bs4 import BeautifulSoup from app.main.views.jobs import get_time_left, get_status_filters from tests import notification_json -from tests.conftest import SERVICE_ONE_ID, mock_get_notifications, normalize_spaces +from tests.conftest import ( + SERVICE_ONE_ID, + mock_get_notifications, + normalize_spaces, + mock_get_notifications +) from freezegun import freeze_time +from datetime import datetime @pytest.mark.parametrize( @@ -344,3 +350,63 @@ def test_redacts_templates_that_should_be_redacted( assert normalize_spaces(page.select('tbody tr th')[0].text) == ( '07123456789 hello hidden' ) + + +@pytest.mark.parametrize( + "message_type, tablist_visible, search_bar_visible", [ + ('email', True, True), + ('sms', True, True), + ('letter', False, False) + ] +) +def test_big_numbers_and_search_dont_show_for_letters( + client_request, + service_one, + mock_get_notifications, + active_user_with_permissions, + mock_get_detailed_service, + message_type, + tablist_visible, + search_bar_visible +): + page = client_request.get( + 'main.view_notifications', + service_id=service_one['id'], + message_type=message_type, + status='', + page=1, + ) + + assert (len(page.select("[role=tablist]")) > 0) == tablist_visible + assert (len(page.select("[type=search]")) > 0) == search_bar_visible + + +@freeze_time("2017-09-27 16:30:00.000000") +@pytest.mark.parametrize( + "message_type, hint_status_visible", [ + ('email', True), + ('sms', True), + ('letter', False) + ] +) +def test_sending_status_hint_does_not_include_status_for_letters( + client_request, + service_one, + active_user_with_permissions, + mock_get_detailed_service, + message_type, + hint_status_visible, + mocker +): + mock_get_notifications(mocker, True, diff_template_type=message_type) + + page = client_request.get( + 'main.view_notifications', + service_id=service_one['id'], + message_type=message_type + ) + + if message_type == 'letter': + assert normalize_spaces(page.select(".align-with-message-body")[0].text) == "27 September at 5:30pm" + else: + assert normalize_spaces(page.select(".align-with-message-body")[0].text) == "Delivered 27 September at 5:31pm" diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 27e386bc3..fddcd33ec 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -361,6 +361,33 @@ def test_should_show_upcoming_jobs_on_dashboard( assert table_rows[1].find_all('td')[0].text.strip() == '1' +@pytest.mark.parametrize('permissions, column_name, expected_column_count', [ + (['email', 'sms'], '.column-half', 2), + (['email', 'letter'], '.column-third', 3), + (['email', 'sms', 'letter'], '.column-third', 3) +]) +def test_correct_columns_display_on_dashboard( + client_request, + mock_get_service_templates, + mock_get_template_statistics, + mock_get_detailed_service, + mock_get_jobs, + service_one, + permissions, + expected_column_count, + column_name +): + + service_one['permissions'] = permissions + + page = client_request.get( + 'main.service_dashboard', + service_id=service_one['id'] + ) + + assert len(page.select(column_name)) == expected_column_count + + @freeze_time("2016-01-01 11:09:00.061258") def test_should_show_recent_jobs_on_dashboard( logged_in_client, diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 1059e0792..7595c61f9 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -8,7 +8,7 @@ from bs4 import BeautifulSoup from app.main.views.jobs import get_time_left, get_status_filters from tests import notification_json -from tests.conftest import SERVICE_ONE_ID, normalize_spaces +from tests.conftest import SERVICE_ONE_ID, normalize_spaces, mock_get_notifications from freezegun import freeze_time @@ -163,23 +163,25 @@ def test_should_show_letter_job( client_request, mock_get_service_letter_template, mock_get_job, - mock_get_notifications, fake_uuid, + active_user_with_permissions, + mocker, ): + get_notifications = mock_get_notifications(mocker, active_user_with_permissions, diff_template_type='letter') + page = client_request.get( 'main.view_job', service_id=SERVICE_ONE_ID, job_id=fake_uuid, ) - assert normalize_spaces(page.h1.text) == 'thisisatest.csv' assert normalize_spaces(page.select('p.bottom-gutter')[0].text) == ( 'Sent by Test User on 1 January at 11:09am' ) assert page.select('.banner-default-with-tick') == [] assert normalize_spaces(page.select('tbody tr')[0].text) == ( - '07123456789 template content' + '1 Example Street template content 1 January at 11:09am' ) assert normalize_spaces(page.select('.keyline-block')[0].text) == ( '1 Letter' @@ -190,7 +192,7 @@ def test_should_show_letter_job( assert page.select('[download=download]') == [] assert page.select('.hint') == [] - mock_get_notifications.assert_called_with( + get_notifications.assert_called_with( SERVICE_ONE_ID, fake_uuid, status=[ diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 2dd393656..2c5d652c2 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -171,18 +171,21 @@ def test_platform_admin_with_date_filter( 'main.platform_admin', ( '61 emails sent 6 failed – 5.5%', '121 text messages sent 11 failed – 5.0%', + '45 letters sent 13 failed – 28.9%' ), ), ( 'main.live_services', ( '55 emails sent 5 failed – 5.0%', '110 text messages sent 10 failed – 5.0%', + '15 letters sent 3 failed – 20.0%' ), ), ( 'main.trial_services', ( '6 emails sent 1 failed – 10.0%', '11 text messages sent 1 failed – 5.0%', + '30 letters sent 10 failed – 33.3%' ), ), ]) @@ -206,6 +209,9 @@ def test_should_show_total_on_platform_admin_pages( sms_requested=200, sms_delivered=100, sms_failed=10, + letters_requested=15, + letters_delivered=12, + letters_failed=3 ) services[1]['statistics'] = create_stats( @@ -215,6 +221,9 @@ def test_should_show_total_on_platform_admin_pages( sms_requested=20, sms_delivered=10, sms_failed=1, + letters_requested=30, + letters_delivered=20, + letters_failed=10 ) mock_get_detailed_services.return_value = {'data': services} @@ -227,6 +236,7 @@ def test_should_show_total_on_platform_admin_pages( assert ( normalize_spaces(page.select('.big-number-with-status')[0].text), normalize_spaces(page.select('.big-number-with-status')[1].text), + normalize_spaces(page.select('.big-number-with-status')[2].text), ) == expected_big_numbers @@ -259,6 +269,12 @@ def test_create_global_stats_sets_failure_rates(fake_uuid): 'failed': 0, 'requested': 0, 'failure_rate': '0' + }, + 'letter': { + 'delivered': 0, + 'failed': 0, + 'requested': 0, + 'failure_rate': '0' } } @@ -269,7 +285,10 @@ def create_stats( emails_failed=0, sms_requested=0, sms_delivered=0, - sms_failed=0 + sms_failed=0, + letters_requested=0, + letters_delivered=0, + letters_failed=0 ): return { 'sms': { @@ -281,7 +300,12 @@ def create_stats( 'requested': emails_requested, 'delivered': emails_delivered, 'failed': emails_failed, - } + }, + 'letter': { + 'requested': letters_requested, + 'delivered': letters_delivered, + 'failed': letters_failed, + }, } @@ -293,7 +317,10 @@ def test_format_stats_by_service_returns_correct_values(fake_uuid): emails_failed=5, sms_requested=50, sms_delivered=7, - sms_failed=11 + sms_failed=11, + letters_requested=40, + letters_delivered=20, + letters_failed=7 ) ret = list(format_stats_by_service(services)) @@ -307,6 +334,10 @@ def test_format_stats_by_service_returns_correct_values(fake_uuid): assert ret[0]['stats']['sms']['delivered'] == 7 assert ret[0]['stats']['sms']['failed'] == 11 + assert ret[0]['stats']['letter']['sending'] == 13 + assert ret[0]['stats']['letter']['delivered'] == 20 + assert ret[0]['stats']['letter']['failed'] == 7 + @pytest.mark.parametrize('endpoint, restricted, research_mode, trial_mode_services', [ ('main.trial_services', True, False, True), @@ -443,7 +474,10 @@ def test_should_show_correct_sent_totals_for_platform_admin( emails_failed=40, sms_requested=100, sms_delivered=10, - sms_failed=30 + sms_failed=30, + letters_requested=60, + letters_delivered=40, + letters_failed=5 ) mock_get_detailed_services.return_value = {'data': services} @@ -457,9 +491,11 @@ def test_should_show_correct_sent_totals_for_platform_admin( totals = page.find_all('div', 'big-number-with-status') email_total = int(totals[0].find_all('div', 'big-number-number')[0].text.strip()) sms_total = int(totals[1].find_all('div', 'big-number-number')[0].text.strip()) + letter_total = int(totals[2].find_all('div', 'big-number-number')[0].text.strip()) assert email_total == 60 assert sms_total == 40 + assert letter_total == 45 @pytest.mark.parametrize('endpoint, restricted, research_mode, trial_mode_services', [ diff --git a/tests/conftest.py b/tests/conftest.py index ed93ead64..47412355c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -183,7 +183,8 @@ def mock_get_detailed_service(mocker, api_user_active): 'free_sms_fragment_limit': 250000, 'statistics': { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 0, 'delivered': 0, 'failed': 0} }, 'created_at': str(datetime.utcnow()) } @@ -201,7 +202,8 @@ def mock_get_detailed_service_for_today(mocker, api_user_active): 'free_sms_fragment_limit': 250000, 'statistics': { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 0, 'delivered': 0, 'failed': 0} } } } @@ -229,11 +231,15 @@ def mock_get_detailed_services(mocker, fake_uuid): ) service_one['statistics'] = { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 0, 'delivered': 0, 'failed': 0} + } service_two['statistics'] = { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 0, 'delivered': 0, 'failed': 0} + } services = {'data': [service_one, service_two]} @@ -1236,6 +1242,7 @@ def mock_get_notifications( mocker, api_user_active, template_content=None, + diff_template_type=None, personalisation=None, redact_personalisation=False, ): @@ -1259,7 +1266,7 @@ def mock_get_notifications( template = template_json( service_id, id_=str(generate_uuid()), - type_=template_type[0], + type_=diff_template_type or template_type[0], content=template_content, redact_personalisation=redact_personalisation, ) @@ -1270,13 +1277,13 @@ def mock_get_notifications( content=template_content, redact_personalisation=redact_personalisation, ) - return notification_json( service_id, template=template, rows=rows, job=job, personalisation=personalisation, + template_type=diff_template_type ) return mocker.patch( @@ -1584,6 +1591,10 @@ def mock_get_monthly_notification_stats(mocker, service_one, fake_uuid): "sending": 1, "delivered": 1, }, + "letter": { + "sending": 1, + "delivered": 1, + } } }} return mocker.patch(