From 7e6076a41fb43a5f154cfbef28c999c321e7a9b0 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 20 Jun 2018 14:04:38 +0100 Subject: [PATCH 1/5] Add in new endpoint with data for platform admin stats Added in a new endpoint and DAO function to provide the data for the new platform admin statistics page. The DAO method gets different data from the Notifications / NotificationHistory table and also groups it differently. The old endpoint has not been deleted yet to allow the numbers on the old and new pages to be compared. --- app/dao/services_dao.py | 28 +++++++++++ app/service/rest.py | 14 ++++++ app/service/statistics.py | 31 ++++++++++++ tests/app/dao/test_services_dao.py | 73 +++++++++++++++++++++++++--- tests/app/service/test_rest.py | 22 +++++++++ tests/app/service/test_statistics.py | 66 ++++++++++++++++++++++++- 6 files changed, 226 insertions(+), 8 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 68f0ea781..d6172fd98 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -453,6 +453,34 @@ def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date, i return query.all() +@statsd(namespace='dao') +def fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date): + start_date = get_london_midnight_in_utc(start_date) + end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) + table = NotificationHistory + + if start_date >= datetime.utcnow() - timedelta(days=7): + table = Notification + + query = db.session.query( + table.notification_type, + table.status, + table.key_type, + func.count(table.id).label('count') + ).filter( + table.created_at >= start_date, + table.created_at < end_date + ).group_by( + table.notification_type, + table.key_type, + table.status + ).order_by( + table.notification_type, + ) + + return query.all() + + @transactional @version_class(Service) @version_class(ApiKey) diff --git a/app/service/rest.py b/app/service/rest.py index 4889fd3b1..13b77c317 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -44,6 +44,7 @@ from app.dao.services_dao import ( dao_suspend_service, dao_update_service, fetch_aggregate_stats_by_date_range_for_all_services, + fetch_new_aggregate_stats_by_date_range_for_all_services, fetch_stats_by_date_range_for_all_services ) from app.dao.service_whitelist_dao import ( @@ -131,6 +132,19 @@ def get_platform_stats(): return result +@service_blueprint.route('/platform-stats-new', methods=['GET']) +def get_new_platform_stats(): + # If start and end date are not set, we are expecting today's stats. + today = str(datetime.utcnow().date()) + + start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date() + end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date() + data = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date=start_date, end_date=end_date) + stats = statistics.format_admin_stats(data) + + return jsonify(stats) + + @service_blueprint.route('', methods=['GET']) def get_services(): only_active = request.args.get('only_active') == 'True' diff --git a/app/service/statistics.py b/app/service/statistics.py index 26b3fb289..df49b7ff2 100644 --- a/app/service/statistics.py +++ b/app/service/statistics.py @@ -14,6 +14,37 @@ def format_statistics(statistics): return counts +def format_admin_stats(statistics): + counts = create_stats_dict() + + for row in statistics: + if row.key_type == 'test': + counts[row.notification_type]['test-key'] += row.count + else: + counts[row.notification_type]['total'] += row.count + if row.status in ('technical-failure', 'permanent-failure', 'temporary-failure', 'virus-scan-failed'): + counts[row.notification_type]['failures'][row.status] += row.count + + return counts + + +def create_stats_dict(): + stats_dict = {} + for template in TEMPLATE_TYPES: + stats_dict[template] = {} + + for status in ('total', 'test-key'): + stats_dict[template][status] = 0 + + stats_dict[template]['failures'] = { + 'technical-failure': 0, + 'permanent-failure': 0, + 'temporary-failure': 0, + 'virus-scan-failed': 0, + } + return stats_dict + + def format_monthly_template_notification_stats(year, rows): stats = { datetime.strftime(date, '%Y-%m'): {} diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 997920fd5..8338ce6ed 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta +from datetime import date, datetime, timedelta import uuid import functools @@ -33,7 +33,8 @@ from app.dao.services_dao import ( dao_fetch_active_users_for_service, dao_fetch_service_by_inbound_number, dao_fetch_monthly_historical_stats_by_template, - dao_fetch_monthly_historical_usage_by_template_for_service) + dao_fetch_monthly_historical_usage_by_template_for_service, + fetch_new_aggregate_stats_by_date_range_for_all_services) from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( @@ -674,7 +675,7 @@ def test_fetch_monthly_historical_stats_separates_months(notify_db, notify_db_se result = dao_fetch_monthly_historical_stats_for_service(sample_template.service_id, 2016) - for date, status, count in ( + for day, status, count in ( ('2016-04', 'sending', 0), ('2016-04', 'delivered', 0), ('2016-04', 'pending', 0), @@ -692,9 +693,9 @@ def test_fetch_monthly_historical_stats_separates_months(notify_db, notify_db_se ('2017-03', 'created', 2), ): - assert result[date]['sms'][status] == count - assert result[date]['email'][status] == 0 - assert result[date]['letter'][status] == 0 + assert result[day]['sms'][status] == count + assert result[day]['email'][status] == 0 + assert result[day]['letter'][status] == 0 assert result.keys() == { '2016-04', '2016-05', '2016-06', @@ -817,6 +818,66 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session result_one.service.created_at, 'sms', 'created', 2) +@pytest.mark.parametrize('table_name, days_ago', [ + ('Notification', 8), + ('NotificationHistory', 3), +]) +@freeze_time('2018-01-08') +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( + mocker, + notify_db_session, + table_name, + days_ago +): + start_date = datetime.now().date() - timedelta(days=days_ago) + end_date = datetime.now().date() + + # mock the table that should not be used, then check it is not being called + unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) + fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + + unused_table_mock.assert_not_called() + + +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): + start_date = date(2018, 1, 1) + end_date = date(2018, 1, 5) + + result = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + assert result == [] + + +@freeze_time('2018-01-08') +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( + notify_db, + notify_db_session, + sample_template, + sample_email_template, + sample_letter_template, +): + today = datetime.now().date() + + for i in range(3): + create_notification(notify_db, notify_db_session, template=sample_email_template, status='permanent-failure', + created_at=today) + + create_notification(notify_db, notify_db_session, template=sample_email_template, status='sent', created_at=today) + create_notification(notify_db, notify_db_session, template=sample_template, status='sent', created_at=today) + create_notification(notify_db, notify_db_session, template=sample_template, status='sent', created_at=today, + key_type=KEY_TYPE_TEAM) + create_notification(notify_db, notify_db_session, template=sample_letter_template, status='virus-scan-failed', + created_at=today) + + result = fetch_new_aggregate_stats_by_date_range_for_all_services(today, today) + + assert len(result) == 5 + assert result[0] == ('email', 'permanent-failure', 'normal', 3) + assert result[1] == ('email', 'sent', 'normal', 1) + assert result[2] == ('sms', 'sent', 'normal', 1) + assert result[3] == ('sms', 'sent', 'team', 1) + assert result[4] == ('letter', 'virus-scan-failed', 'normal', 1) + + @freeze_time('2001-01-01T23:59:00') def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(sample_service, sample_api_key): dao_suspend_service(sample_service.id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b9665dfc9..44b928c38 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -3085,6 +3085,28 @@ def test_get_platform_stats_creates_zero_stats(client, notify_db_session): assert json_resp['sms'] == {'failed': 0, 'requested': 4, 'delivered': 3} +@freeze_time('2018-06-01') +def test_get_new_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker): + today = datetime.now().date() + dao_mock = mocker.patch('app.service.rest.fetch_new_aggregate_stats_by_date_range_for_all_services') + mocker.patch('app.service.rest.statistics.format_statistics') + + admin_request.get('service.get_new_platform_stats') + + dao_mock.assert_called_once_with(start_date=today, end_date=today) + + +def test_get_new_platform_stats_can_filter_by_date(admin_request, mocker): + start_date = date(2017, 1, 1) + end_date = date(2018, 1, 1) + dao_mock = mocker.patch('app.service.rest.fetch_new_aggregate_stats_by_date_range_for_all_services') + mocker.patch('app.service.rest.statistics.format_statistics') + + admin_request.get('service.get_new_platform_stats', start_date=start_date, end_date=end_date) + + dao_mock.assert_called_once_with(start_date=start_date, end_date=end_date) + + @pytest.mark.parametrize('today_only, stats', [ (False, {'requested': 2, 'delivered': 1, 'failed': 0}), (True, {'requested': 1, 'delivered': 0, 'failed': 0}) diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index 7f56a5b9a..856bee499 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -3,11 +3,14 @@ import collections import pytest from app.service.statistics import ( + format_admin_stats, format_statistics, + create_stats_dict, create_zeroed_stats_dicts, ) StatsRow = collections.namedtuple('row', ('notification_type', 'status', 'count')) +NewStatsRow = collections.namedtuple('row', ('notification_type', 'status', 'key_type', 'count')) # email_counts and sms_counts are 3-tuple of requested, delivered, failed @@ -65,5 +68,64 @@ def test_create_zeroed_stats_dicts(): } -def _stats(requested, delivered, failed): - return {'requested': requested, 'delivered': delivered, 'failed': failed} +def test_create_stats_dict(): + assert create_stats_dict() == { + 'sms': {'total': 0, + 'test-key': 0, + 'failures': {'technical-failure': 0, + 'permanent-failure': 0, + 'temporary-failure': 0, + 'virus-scan-failed': 0}}, + 'email': {'total': 0, + 'test-key': 0, + 'failures': {'technical-failure': 0, + 'permanent-failure': 0, + 'temporary-failure': 0, + 'virus-scan-failed': 0}}, + 'letter': {'total': 0, + 'test-key': 0, + 'failures': {'technical-failure': 0, + 'permanent-failure': 0, + 'temporary-failure': 0, + 'virus-scan-failed': 0}} + } + + +def test_format_admin_stats_only_includes_test_key_notifications_in_test_key_section(): + rows = [ + NewStatsRow('email', 'technical-failure', 'test', 3), + NewStatsRow('sms', 'permanent-failure', 'test', 4), + NewStatsRow('letter', 'virus-scan-failed', 'test', 5), + ] + stats_dict = format_admin_stats(rows) + + assert stats_dict['email']['total'] == 0 + assert stats_dict['email']['failures']['technical-failure'] == 0 + assert stats_dict['email']['test-key'] == 3 + + assert stats_dict['sms']['total'] == 0 + assert stats_dict['sms']['failures']['permanent-failure'] == 0 + assert stats_dict['sms']['test-key'] == 4 + + assert stats_dict['letter']['total'] == 0 + assert stats_dict['letter']['failures']['virus-scan-failed'] == 0 + assert stats_dict['letter']['test-key'] == 5 + + +def test_format_admin_stats_counts_non_test_key_notifications_correctly(): + rows = [ + NewStatsRow('email', 'technical-failure', 'normal', 1), + NewStatsRow('email', 'created', 'team', 3), + NewStatsRow('sms', 'temporary-failure', 'normal', 6), + NewStatsRow('sms', 'sent', 'normal', 2), + NewStatsRow('letter', 'pending-virus-check', 'normal', 1), + ] + stats_dict = format_admin_stats(rows) + + assert stats_dict['email']['total'] == 4 + assert stats_dict['email']['failures']['technical-failure'] == 1 + + assert stats_dict['sms']['total'] == 8 + assert stats_dict['sms']['failures']['permanent-failure'] == 0 + + assert stats_dict['letter']['total'] == 1 From 5381491aaec7f91770e268a108be769b8fb9dddb Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 27 Jun 2018 15:12:11 +0100 Subject: [PATCH 2/5] Move DAO function for new platform stats page Moved the `fetch_new_aggregate_stats_by_date_range_for_all_services` DAO function from the services DAO to the Notifications DAO since this function queries the `notification` and `notification_history` tables. Also added a test to check that the data returned from the function takes BST into account. --- app/dao/notifications_dao.py | 28 ++++++++ app/dao/services_dao.py | 28 -------- app/service/rest.py | 2 +- .../notification_dao/test_notification_dao.py | 72 ++++++++++++++++++- tests/app/dao/test_services_dao.py | 65 +---------------- 5 files changed, 102 insertions(+), 93 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index f531a15c6..1d6130ad0 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -609,3 +609,31 @@ def guess_notification_type(search_term): return EMAIL_TYPE else: return SMS_TYPE + + +@statsd(namespace='dao') +def fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date): + start_date = get_london_midnight_in_utc(start_date) + end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) + table = NotificationHistory + + if start_date >= datetime.utcnow() - timedelta(days=7): + table = Notification + + query = db.session.query( + table.notification_type, + table.status, + table.key_type, + func.count(table.id).label('count') + ).filter( + table.created_at >= start_date, + table.created_at < end_date + ).group_by( + table.notification_type, + table.key_type, + table.status + ).order_by( + table.notification_type, + ) + + return query.all() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index d6172fd98..68f0ea781 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -453,34 +453,6 @@ def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date, i return query.all() -@statsd(namespace='dao') -def fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date): - start_date = get_london_midnight_in_utc(start_date) - end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) - table = NotificationHistory - - if start_date >= datetime.utcnow() - timedelta(days=7): - table = Notification - - query = db.session.query( - table.notification_type, - table.status, - table.key_type, - func.count(table.id).label('count') - ).filter( - table.created_at >= start_date, - table.created_at < end_date - ).group_by( - table.notification_type, - table.key_type, - table.status - ).order_by( - table.notification_type, - ) - - return query.all() - - @transactional @version_class(Service) @version_class(ApiKey) diff --git a/app/service/rest.py b/app/service/rest.py index 13b77c317..5622f20db 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -18,6 +18,7 @@ from app.dao.api_key_dao import ( get_unsigned_secret, expire_api_key) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service +from app.dao.notifications_dao import fetch_new_aggregate_stats_by_date_range_for_all_services from app.dao.organisation_dao import dao_get_organisation_by_service_id from app.dao.service_sms_sender_dao import ( archive_sms_sender, @@ -44,7 +45,6 @@ from app.dao.services_dao import ( dao_suspend_service, dao_update_service, fetch_aggregate_stats_by_date_range_for_all_services, - fetch_new_aggregate_stats_by_date_range_for_all_services, fetch_stats_by_date_range_for_all_services ) from app.dao.service_whitelist_dao import ( diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 775598015..e4c408cd4 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -33,7 +33,8 @@ from app.dao.notifications_dao import ( dao_get_notification_by_reference, dao_get_notifications_by_references, dao_get_notification_history_by_reference, - notifications_not_yet_sent + notifications_not_yet_sent, + fetch_new_aggregate_stats_by_date_range_for_all_services, ) from app.dao.services_dao import dao_update_service from app.models import ( @@ -1933,3 +1934,72 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_ results = notifications_not_yet_sent(older_than, notification_type) assert len(results) == 0 + + +@pytest.mark.parametrize('table_name, days_ago', [ + ('Notification', 8), + ('NotificationHistory', 3), +]) +@freeze_time('2018-01-08') +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( + mocker, + notify_db_session, + table_name, + days_ago +): + start_date = datetime.now().date() - timedelta(days=days_ago) + end_date = datetime.now().date() + + # mock the table that should not be used, then check it is not being called + unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) + fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + + unused_table_mock.assert_not_called() + + +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): + start_date = date(2018, 1, 1) + end_date = date(2018, 1, 5) + + result = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + assert result == [] + + +@freeze_time('2018-01-08') +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( + sample_template, + sample_email_template, + sample_letter_template, +): + today = datetime.now().date() + + for i in range(3): + create_notification(template=sample_email_template, status='permanent-failure', + created_at=today) + + create_notification(template=sample_email_template, status='sent', created_at=today) + create_notification(template=sample_template, status='sent', created_at=today) + create_notification(template=sample_template, status='sent', created_at=today, + key_type=KEY_TYPE_TEAM) + create_notification(template=sample_letter_template, status='virus-scan-failed', + created_at=today) + + result = fetch_new_aggregate_stats_by_date_range_for_all_services(today, today) + + assert len(result) == 5 + assert result[0] == ('email', 'permanent-failure', 'normal', 3) + assert result[1] == ('email', 'sent', 'normal', 1) + assert result[2] == ('sms', 'sent', 'normal', 1) + assert result[3] == ('sms', 'sent', 'team', 1) + assert result[4] == ('letter', 'virus-scan-failed', 'normal', 1) + + +def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_bst_date(sample_template): + query_day = datetime(2018, 6, 5).date() + create_notification(sample_template, status='sent', created_at=datetime(2018, 6, 4, 23, 59)) + create_notification(sample_template, status='created', created_at=datetime(2018, 6, 5, 23, 00)) + + result = fetch_new_aggregate_stats_by_date_range_for_all_services(query_day, query_day) + + assert len(result) == 1 + assert result[0].status == 'sent' diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 8338ce6ed..8cd5acca7 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,4 +1,4 @@ -from datetime import date, datetime, timedelta +from datetime import datetime, timedelta import uuid import functools @@ -33,8 +33,7 @@ from app.dao.services_dao import ( dao_fetch_active_users_for_service, dao_fetch_service_by_inbound_number, dao_fetch_monthly_historical_stats_by_template, - dao_fetch_monthly_historical_usage_by_template_for_service, - fetch_new_aggregate_stats_by_date_range_for_all_services) + dao_fetch_monthly_historical_usage_by_template_for_service) from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( @@ -818,66 +817,6 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session result_one.service.created_at, 'sms', 'created', 2) -@pytest.mark.parametrize('table_name, days_ago', [ - ('Notification', 8), - ('NotificationHistory', 3), -]) -@freeze_time('2018-01-08') -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( - mocker, - notify_db_session, - table_name, - days_ago -): - start_date = datetime.now().date() - timedelta(days=days_ago) - end_date = datetime.now().date() - - # mock the table that should not be used, then check it is not being called - unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) - fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) - - unused_table_mock.assert_not_called() - - -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): - start_date = date(2018, 1, 1) - end_date = date(2018, 1, 5) - - result = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) - assert result == [] - - -@freeze_time('2018-01-08') -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( - notify_db, - notify_db_session, - sample_template, - sample_email_template, - sample_letter_template, -): - today = datetime.now().date() - - for i in range(3): - create_notification(notify_db, notify_db_session, template=sample_email_template, status='permanent-failure', - created_at=today) - - create_notification(notify_db, notify_db_session, template=sample_email_template, status='sent', created_at=today) - create_notification(notify_db, notify_db_session, template=sample_template, status='sent', created_at=today) - create_notification(notify_db, notify_db_session, template=sample_template, status='sent', created_at=today, - key_type=KEY_TYPE_TEAM) - create_notification(notify_db, notify_db_session, template=sample_letter_template, status='virus-scan-failed', - created_at=today) - - result = fetch_new_aggregate_stats_by_date_range_for_all_services(today, today) - - assert len(result) == 5 - assert result[0] == ('email', 'permanent-failure', 'normal', 3) - assert result[1] == ('email', 'sent', 'normal', 1) - assert result[2] == ('sms', 'sent', 'normal', 1) - assert result[3] == ('sms', 'sent', 'team', 1) - assert result[4] == ('letter', 'virus-scan-failed', 'normal', 1) - - @freeze_time('2001-01-01T23:59:00') def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(sample_service, sample_api_key): dao_suspend_service(sample_service.id) From 4b030b1583f980d24e6cdad5f2177059df37eadb Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 28 Jun 2018 08:39:25 +0100 Subject: [PATCH 3/5] Create platform-stats blueprint Created a platform-stats blueprint and moved the new platform stats endpoint to the new blueprint (it was previously in the service blueprint). Since the original platform stats route and the new platform stats route are now in different blueprints, their view functions can have the same name without any issues. --- app/__init__.py | 4 +++ app/dao/notifications_dao.py | 2 +- app/platform_stats/__init__.py | 0 app/platform_stats/rest.py | 24 ++++++++++++++++++ app/service/rest.py | 14 ----------- .../notification_dao/test_notification_dao.py | 18 ++++++------- tests/app/platform_stats/__init__.py | 0 tests/app/platform_stats/test_rest.py | 25 +++++++++++++++++++ tests/app/service/test_rest.py | 22 ---------------- 9 files changed, 63 insertions(+), 46 deletions(-) create mode 100644 app/platform_stats/__init__.py create mode 100644 app/platform_stats/rest.py create mode 100644 tests/app/platform_stats/__init__.py create mode 100644 tests/app/platform_stats/test_rest.py diff --git a/app/__init__.py b/app/__init__.py index 46554c32e..2ae2d497e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -113,6 +113,7 @@ def register_blueprint(application): from app.organisation.rest import organisation_blueprint from app.organisation.invite_rest import organisation_invite_blueprint from app.complaint.complaint_rest import complaint_blueprint + from app.platform_stats.rest import platform_stats_blueprint service_blueprint.before_request(requires_admin_auth) application.register_blueprint(service_blueprint, url_prefix='/service') @@ -192,6 +193,9 @@ def register_blueprint(application): complaint_blueprint.before_request(requires_admin_auth) application.register_blueprint(complaint_blueprint) + platform_stats_blueprint.before_request(requires_admin_auth) + application.register_blueprint(platform_stats_blueprint, url_prefix='/platform-stats') + def register_v2_blueprints(application): from app.v2.inbound_sms.get_inbound_sms import v2_inbound_sms_blueprint as get_inbound_sms diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 1d6130ad0..88319de34 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -612,7 +612,7 @@ def guess_notification_type(search_term): @statsd(namespace='dao') -def fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date): +def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date): start_date = get_london_midnight_in_utc(start_date) end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) table = NotificationHistory diff --git a/app/platform_stats/__init__.py b/app/platform_stats/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/platform_stats/rest.py b/app/platform_stats/rest.py new file mode 100644 index 000000000..a41c8e3ee --- /dev/null +++ b/app/platform_stats/rest.py @@ -0,0 +1,24 @@ +from datetime import datetime + +from flask import Blueprint, jsonify, request + +from app.dao.notifications_dao import fetch_aggregate_stats_by_date_range_for_all_services +from app.errors import register_errors +from app.service.statistics import format_admin_stats + +platform_stats_blueprint = Blueprint('platform_stats', __name__) + +register_errors(platform_stats_blueprint) + + +@platform_stats_blueprint.route('') +def get_new_platform_stats(): + # If start and end date are not set, we are expecting today's stats. + today = str(datetime.utcnow().date()) + + start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date() + end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date() + data = fetch_aggregate_stats_by_date_range_for_all_services(start_date=start_date, end_date=end_date) + stats = format_admin_stats(data) + + return jsonify(stats) diff --git a/app/service/rest.py b/app/service/rest.py index 5622f20db..4889fd3b1 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -18,7 +18,6 @@ from app.dao.api_key_dao import ( get_unsigned_secret, expire_api_key) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service -from app.dao.notifications_dao import fetch_new_aggregate_stats_by_date_range_for_all_services from app.dao.organisation_dao import dao_get_organisation_by_service_id from app.dao.service_sms_sender_dao import ( archive_sms_sender, @@ -132,19 +131,6 @@ def get_platform_stats(): return result -@service_blueprint.route('/platform-stats-new', methods=['GET']) -def get_new_platform_stats(): - # If start and end date are not set, we are expecting today's stats. - today = str(datetime.utcnow().date()) - - start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date() - end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date() - data = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date=start_date, end_date=end_date) - stats = statistics.format_admin_stats(data) - - return jsonify(stats) - - @service_blueprint.route('', methods=['GET']) def get_services(): only_active = request.args.get('only_active') == 'True' diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index e4c408cd4..d7f1401f5 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -34,7 +34,7 @@ from app.dao.notifications_dao import ( dao_get_notifications_by_references, dao_get_notification_history_by_reference, notifications_not_yet_sent, - fetch_new_aggregate_stats_by_date_range_for_all_services, + fetch_aggregate_stats_by_date_range_for_all_services, ) from app.dao.services_dao import dao_update_service from app.models import ( @@ -1941,7 +1941,7 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_ ('NotificationHistory', 3), ]) @freeze_time('2018-01-08') -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( +def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( mocker, notify_db_session, table_name, @@ -1952,21 +1952,21 @@ def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_the_corre # mock the table that should not be used, then check it is not being called unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) - fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date) unused_table_mock.assert_not_called() -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): +def test_fetch_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): start_date = date(2018, 1, 1) end_date = date(2018, 1, 5) - result = fetch_new_aggregate_stats_by_date_range_for_all_services(start_date, end_date) + result = fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date) assert result == [] @freeze_time('2018-01-08') -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( +def test_fetch_aggregate_stats_by_date_range_for_all_services_groups_stats( sample_template, sample_email_template, sample_letter_template, @@ -1984,7 +1984,7 @@ def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( create_notification(template=sample_letter_template, status='virus-scan-failed', created_at=today) - result = fetch_new_aggregate_stats_by_date_range_for_all_services(today, today) + result = fetch_aggregate_stats_by_date_range_for_all_services(today, today) assert len(result) == 5 assert result[0] == ('email', 'permanent-failure', 'normal', 3) @@ -1994,12 +1994,12 @@ def test_fetch_new_aggregate_stats_by_date_range_for_all_services_groups_stats( assert result[4] == ('letter', 'virus-scan-failed', 'normal', 1) -def test_fetch_new_aggregate_stats_by_date_range_for_all_services_uses_bst_date(sample_template): +def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_bst_date(sample_template): query_day = datetime(2018, 6, 5).date() create_notification(sample_template, status='sent', created_at=datetime(2018, 6, 4, 23, 59)) create_notification(sample_template, status='created', created_at=datetime(2018, 6, 5, 23, 00)) - result = fetch_new_aggregate_stats_by_date_range_for_all_services(query_day, query_day) + result = fetch_aggregate_stats_by_date_range_for_all_services(query_day, query_day) assert len(result) == 1 assert result[0].status == 'sent' diff --git a/tests/app/platform_stats/__init__.py b/tests/app/platform_stats/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py new file mode 100644 index 000000000..b2e380980 --- /dev/null +++ b/tests/app/platform_stats/test_rest.py @@ -0,0 +1,25 @@ +from datetime import date, datetime + +from freezegun import freeze_time + + +@freeze_time('2018-06-01') +def test_get_new_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker): + today = datetime.now().date() + dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services') + mocker.patch('app.service.rest.statistics.format_statistics') + + admin_request.get('platform_stats.get_new_platform_stats') + + dao_mock.assert_called_once_with(start_date=today, end_date=today) + + +def test_get_new_platform_stats_can_filter_by_date(admin_request, mocker): + start_date = date(2017, 1, 1) + end_date = date(2018, 1, 1) + dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services') + mocker.patch('app.service.rest.statistics.format_statistics') + + admin_request.get('platform_stats.get_new_platform_stats', start_date=start_date, end_date=end_date) + + dao_mock.assert_called_once_with(start_date=start_date, end_date=end_date) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 44b928c38..b9665dfc9 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -3085,28 +3085,6 @@ def test_get_platform_stats_creates_zero_stats(client, notify_db_session): assert json_resp['sms'] == {'failed': 0, 'requested': 4, 'delivered': 3} -@freeze_time('2018-06-01') -def test_get_new_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker): - today = datetime.now().date() - dao_mock = mocker.patch('app.service.rest.fetch_new_aggregate_stats_by_date_range_for_all_services') - mocker.patch('app.service.rest.statistics.format_statistics') - - admin_request.get('service.get_new_platform_stats') - - dao_mock.assert_called_once_with(start_date=today, end_date=today) - - -def test_get_new_platform_stats_can_filter_by_date(admin_request, mocker): - start_date = date(2017, 1, 1) - end_date = date(2018, 1, 1) - dao_mock = mocker.patch('app.service.rest.fetch_new_aggregate_stats_by_date_range_for_all_services') - mocker.patch('app.service.rest.statistics.format_statistics') - - admin_request.get('service.get_new_platform_stats', start_date=start_date, end_date=end_date) - - dao_mock.assert_called_once_with(start_date=start_date, end_date=end_date) - - @pytest.mark.parametrize('today_only, stats', [ (False, {'requested': 2, 'delivered': 1, 'failed': 0}), (True, {'requested': 1, 'delivered': 0, 'failed': 0}) From cbb9f71e610d30398bd9113bb82c0df609e9cd3b Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 28 Jun 2018 15:02:19 +0100 Subject: [PATCH 4/5] Fix bug with test notifications not showing Test notifications are only stored in the notification table, not the notification_history table. We were deciding which table to query for the platform admin stats based on the start date passed to the DAO function. This means that if the start date given was more than 7 days ago and the end date was within the last 7 days, test notifications were not being returned. We now use the end date to choose which table to query which means that test notifications always get returned when they should. --- app/dao/notifications_dao.py | 2 +- .../notification_dao/test_notification_dao.py | 35 ++++++++----------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 88319de34..e0d39e87f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -617,7 +617,7 @@ def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date): end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) table = NotificationHistory - if start_date >= datetime.utcnow() - timedelta(days=7): + if end_date >= datetime.utcnow() - timedelta(days=7): table = Notification query = db.session.query( diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index d7f1401f5..1e05a509c 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1936,27 +1936,6 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_ assert len(results) == 0 -@pytest.mark.parametrize('table_name, days_ago', [ - ('Notification', 8), - ('NotificationHistory', 3), -]) -@freeze_time('2018-01-08') -def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( - mocker, - notify_db_session, - table_name, - days_ago -): - start_date = datetime.now().date() - timedelta(days=days_ago) - end_date = datetime.now().date() - - # mock the table that should not be used, then check it is not being called - unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) - fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date) - - unused_table_mock.assert_not_called() - - def test_fetch_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): start_date = date(2018, 1, 1) end_date = date(2018, 1, 5) @@ -2003,3 +1982,17 @@ def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_bst_date(samp assert len(result) == 1 assert result[0].status == 'sent' + + +@freeze_time('2018-01-08T12:00:00') +def test_fetch_aggregate_stats_by_date_range_for_all_services_gets_test_notifications_when_start_date_over_7_days_ago( + sample_template +): + ten_days_ago = datetime.utcnow().date() - timedelta(days=10) + today = datetime.utcnow().date() + + create_notification(sample_template, key_type=KEY_TYPE_TEST, created_at=datetime.utcnow()) + + result = fetch_aggregate_stats_by_date_range_for_all_services(ten_days_ago, today) + + assert len(result) == 1 From b4654781be2d9f07d2b9e423948dd599cba92eb3 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 29 Jun 2018 15:54:32 +0100 Subject: [PATCH 5/5] Add schema and validate platform stats requests Added a schema to check that the start_date and end_date arguments are both in the date format. --- app/platform_stats/platform_stats_schema.py | 10 ++++++++++ app/platform_stats/rest.py | 7 ++++++- tests/app/platform_stats/test_rest.py | 20 ++++++++++++++++---- 3 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 app/platform_stats/platform_stats_schema.py diff --git a/app/platform_stats/platform_stats_schema.py b/app/platform_stats/platform_stats_schema.py new file mode 100644 index 000000000..cb28a2620 --- /dev/null +++ b/app/platform_stats/platform_stats_schema.py @@ -0,0 +1,10 @@ +platform_stats_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "platform stats request schema", + "type": "object", + "title": "Platform stats request", + "properties": { + "start_date": {"type": ["string", "null"], "format": "date"}, + "end_date": {"type": ["string", "null"], "format": "date"}, + } +} diff --git a/app/platform_stats/rest.py b/app/platform_stats/rest.py index a41c8e3ee..54e94cc47 100644 --- a/app/platform_stats/rest.py +++ b/app/platform_stats/rest.py @@ -4,7 +4,9 @@ from flask import Blueprint, jsonify, request from app.dao.notifications_dao import fetch_aggregate_stats_by_date_range_for_all_services from app.errors import register_errors +from app.platform_stats.platform_stats_schema import platform_stats_request from app.service.statistics import format_admin_stats +from app.schema_validation import validate platform_stats_blueprint = Blueprint('platform_stats', __name__) @@ -12,7 +14,10 @@ register_errors(platform_stats_blueprint) @platform_stats_blueprint.route('') -def get_new_platform_stats(): +def get_platform_stats(): + if request.args: + validate(request.args, platform_stats_request) + # If start and end date are not set, we are expecting today's stats. today = str(datetime.utcnow().date()) diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index b2e380980..5afa7fa43 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -4,22 +4,34 @@ from freezegun import freeze_time @freeze_time('2018-06-01') -def test_get_new_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker): +def test_get_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker): today = datetime.now().date() dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services') mocker.patch('app.service.rest.statistics.format_statistics') - admin_request.get('platform_stats.get_new_platform_stats') + admin_request.get('platform_stats.get_platform_stats') dao_mock.assert_called_once_with(start_date=today, end_date=today) -def test_get_new_platform_stats_can_filter_by_date(admin_request, mocker): +def test_get_platform_stats_can_filter_by_date(admin_request, mocker): start_date = date(2017, 1, 1) end_date = date(2018, 1, 1) dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services') mocker.patch('app.service.rest.statistics.format_statistics') - admin_request.get('platform_stats.get_new_platform_stats', start_date=start_date, end_date=end_date) + admin_request.get('platform_stats.get_platform_stats', start_date=start_date, end_date=end_date) dao_mock.assert_called_once_with(start_date=start_date, end_date=end_date) + + +def test_get_platform_stats_validates_the_date(admin_request): + start_date = '1234-56-78' + + response = admin_request.get( + 'platform_stats.get_platform_stats', start_date=start_date, + _expected_status=400 + ) + + assert response['errors'][0]['message'] == 'start_date time data {} does not match format %Y-%m-%d'.format( + start_date)