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)