diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 8925c81f0..c273ca066 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.sql.expression import literal from sqlalchemy.types import DateTime, Integer from app import db -from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST +from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service from app.utils import get_london_midnight_in_utc, midnight_n_days_ago @@ -197,3 +197,97 @@ def fetch_notification_statuses_for_job(job_id): ).group_by( FactNotificationStatus.notification_status ).all() + + +def fetch_stats_for_all_services_by_date_range(start_date, end_date, include_from_test_key=True): + stats = db.session.query( + FactNotificationStatus.service_id.label('service_id'), + Service.name.label('name'), + Service.restricted.label('restricted'), + Service.research_mode.label('research_mode'), + Service.active.label('active'), + Service.created_at.label('created_at'), + FactNotificationStatus.notification_type.label('notification_type'), + FactNotificationStatus.notification_status.label('status'), + func.sum(FactNotificationStatus.notification_count).label('count') + ).filter( + FactNotificationStatus.bst_date >= start_date, + FactNotificationStatus.bst_date <= end_date, + FactNotificationStatus.service_id == Service.id, + ).group_by( + FactNotificationStatus.service_id.label('service_id'), + Service.name, + Service.restricted, + Service.research_mode, + Service.active, + Service.created_at, + FactNotificationStatus.notification_type, + FactNotificationStatus.notification_status, + ).order_by( + FactNotificationStatus.service_id, + FactNotificationStatus.notification_type + ) + if not include_from_test_key: + stats = stats.filter(FactNotificationStatus.key_type != KEY_TYPE_TEST) + + if start_date <= datetime.utcnow().date() <= end_date: + today = get_london_midnight_in_utc(datetime.utcnow()) + subquery = db.session.query( + Notification.notification_type.cast(db.Text).label('notification_type'), + Notification.status.label('status'), + Notification.service_id.label('service_id'), + func.count(Notification.id).label('count') + ).filter( + Notification.created_at >= today + ).group_by( + Notification.notification_type, + Notification.status, + Notification.service_id + ) + if not include_from_test_key: + subquery = subquery.filter(FactNotificationStatus.key_type != KEY_TYPE_TEST) + subquery = subquery.subquery() + + stats_for_today = db.session.query( + Service.id.label('service_id'), + Service.name.label('name'), + Service.restricted.label('restricted'), + Service.research_mode.label('research_mode'), + Service.active.label('active'), + Service.created_at.label('created_at'), + subquery.c.notification_type.label('notification_type'), + subquery.c.status.label('status'), + subquery.c.count.label('count') + ).outerjoin( + subquery, + subquery.c.service_id == Service.id + ).order_by(Service.id) + + all_stats_table = stats.union_all(stats_for_today).subquery() + query = db.session.query( + all_stats_table.c.service_id, + all_stats_table.c.name, + all_stats_table.c.restricted, + all_stats_table.c.research_mode, + all_stats_table.c.active, + all_stats_table.c.created_at, + all_stats_table.c.notification_type, + all_stats_table.c.status, + func.cast(func.sum(all_stats_table.c.count), Integer).label('count'), + ).group_by( + all_stats_table.c.service_id, + all_stats_table.c.name, + all_stats_table.c.restricted, + all_stats_table.c.research_mode, + all_stats_table.c.active, + all_stats_table.c.created_at, + all_stats_table.c.notification_type, + all_stats_table.c.status, + ).order_by( + all_stats_table.c.name, + all_stats_table.c.notification_type, + all_stats_table.c.status + ) + else: + query = stats + return query.all() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 40918f7e7..fb5cba297 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -335,51 +335,6 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True, only_act return query.all() -@statsd(namespace='dao') -def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True, only_active=True): - 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 - subquery = db.session.query( - table.notification_type, - table.status, - table.service_id, - func.count(table.id).label('count') - ).filter( - table.created_at >= start_date, - table.created_at < end_date - ).group_by( - table.notification_type, - table.status, - table.service_id - ) - if not include_from_test_key: - subquery = subquery.filter(table.key_type != KEY_TYPE_TEST) - subquery = subquery.subquery() - - query = db.session.query( - Service.id.label('service_id'), - Service.name, - Service.restricted, - Service.research_mode, - Service.active, - Service.created_at, - subquery.c.notification_type, - subquery.c.status, - subquery.c.count - ).outerjoin( - subquery, - subquery.c.service_id == Service.id - ).order_by(Service.id) - if only_active: - query = query.filter(Service.active) - - return query.all() - - @transactional @version_class(Service) @version_class(ApiKey) diff --git a/app/service/rest.py b/app/service/rest.py index f1c4f947e..f62a99967 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -23,8 +23,8 @@ from app.dao.api_key_dao import ( from app.dao.fact_notification_status_dao import ( fetch_notification_status_for_service_by_month, fetch_notification_status_for_service_for_day, - fetch_notification_status_for_service_for_today_and_7_previous_days -) + fetch_notification_status_for_service_for_today_and_7_previous_days, + fetch_stats_for_all_services_by_date_range) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.organisation_dao import dao_get_organisation_by_service_id from app.dao.service_data_retention_dao import ( @@ -56,7 +56,6 @@ from app.dao.services_dao import ( dao_remove_user_from_service, dao_suspend_service, dao_update_service, - fetch_stats_by_date_range_for_all_services ) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, @@ -472,10 +471,10 @@ def get_detailed_services(start_date, end_date, only_active=False, include_from_ only_active=only_active) else: - stats = fetch_stats_by_date_range_for_all_services(start_date=start_date, + stats = fetch_stats_for_all_services_by_date_range(start_date=start_date, end_date=end_date, include_from_test_key=include_from_test_key, - only_active=only_active) + ) results = [] for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): rows = list(rows) diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index db90c6d61..64b6b5bc3 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -11,7 +11,7 @@ from app.dao.fact_notification_status_dao import ( fetch_notification_status_for_service_for_today_and_7_previous_days, fetch_notification_status_totals_for_all_services, fetch_notification_statuses_for_job, -) + fetch_stats_for_all_services_by_date_range) from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE from freezegun import freeze_time from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job @@ -289,6 +289,7 @@ def set_up_data(): create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered') create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered') + return service_1, service_2 def test_fetch_notification_statuses_for_job(sample_template): @@ -304,3 +305,36 @@ def test_fetch_notification_statuses_for_job(sample_template): 'created': 5, 'delivered': 2 } + + +@freeze_time('2018-10-31 14:00') +def test_fetch_stats_for_all_services_by_date_range(notify_db_session): + service_1, service_2 = set_up_data() + results = fetch_stats_for_all_services_by_date_range(start_date=date(2018, 10, 29), + end_date=date(2018, 10, 31)) + assert len(results) == 5 + + assert results[0].service_id == service_1.id + assert results[0].notification_type == 'email' + assert results[0].status == 'delivered' + assert results[0].count == 4 + + assert results[1].service_id == service_1.id + assert results[1].notification_type == 'sms' + assert results[1].status == 'created' + assert results[1].count == 2 + + assert results[2].service_id == service_1.id + assert results[2].notification_type == 'sms' + assert results[2].status == 'delivered' + assert results[2].count == 11 + + assert results[3].service_id == service_2.id + assert results[3].notification_type == 'letter' + assert results[3].status == 'delivered' + assert results[3].count == 10 + + assert results[4].service_id == service_2.id + assert not results[4].notification_type + assert not results[4].status + assert not results[4].count diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 6ea7123e3..024b1c019 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -27,7 +27,6 @@ from app.dao.services_dao import ( dao_fetch_todays_stats_for_service, fetch_todays_total_message_count, dao_fetch_todays_stats_for_all_services, - fetch_stats_by_date_range_for_all_services, dao_suspend_service, dao_resume_service, dao_fetch_active_users_for_service, @@ -775,25 +774,6 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key(notif assert stats[0].count == 2 -def test_fetch_stats_by_date_range_for_all_services(notify_db_session): - template = create_template(service=create_service()) - create_notification(template=template, created_at=datetime.now() - timedelta(days=4)) - create_notification(template=template, created_at=datetime.now() - timedelta(days=3)) - result_one = create_notification(template=template, created_at=datetime.now() - timedelta(days=2)) - create_notification(template=template, created_at=datetime.now() - timedelta(days=1)) - create_notification(template=template, created_at=datetime.now()) - - start_date = (datetime.utcnow() - timedelta(days=2)).date() - end_date = (datetime.utcnow() - timedelta(days=1)).date() - - results = fetch_stats_by_date_range_for_all_services(start_date, end_date) - - assert len(results) == 1 - assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted, - result_one.service.research_mode, result_one.service.active, - result_one.service.created_at, 'sms', 'created', 2) - - @freeze_time('2001-01-01T23:59:00') def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(notify_db_session): service = create_service() @@ -807,64 +787,6 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(noti assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) -@pytest.mark.parametrize("start_delta, end_delta, expected", - [("5", "1", "4"), # a date range less than 7 days ago returns test and normal notifications - ("9", "8", "1"), # a date range older than 9 days does not return test notifications. - ("8", "4", "2")]) # a date range that starts more than 7 days ago -@freeze_time('2017-10-23T00:00:00') -def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(notify_db_session, - start_delta, - end_delta, - expected): - template = create_template(service=create_service()) - result_one = create_notification(template=template, created_at=datetime.now(), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=2), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=3), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='normal') - create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='normal') - - start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date() - end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date() - - results = fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True) - - assert len(results) == 1 - assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted, - result_one.service.research_mode, result_one.service.active, result_one.service.created_at, - 'sms', 'created', int(expected)) - - -@pytest.mark.parametrize("start_delta, end_delta, expected", - [("5", "1", "4"), # a date range less than 7 days ago returns test and normal notifications - ("9", "8", "1"), # a date range older than 9 days does not return test notifications. - ("8", "4", "2")]) # a date range that starts more than 7 days ago -@freeze_time('2017-10-23T23:00:00') -def test_fetch_stats_by_date_range_during_bst_hour_for_all_services_returns_test_notifications( - notify_db_session, start_delta, end_delta, expected -): - template = create_template(service=create_service()) - result_one = create_notification(template=template, created_at=datetime.now(), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=2), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=3), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='normal') - create_notification(template=template, created_at=datetime.now() - timedelta(days=4), key_type='test') - create_notification(template=template, created_at=datetime.now() - timedelta(days=8), key_type='normal') - create_notification(template=template, created_at=datetime.now() - timedelta(days=9), key_type='normal') - create_notification(template=template, created_at=datetime.now() - timedelta(days=9), key_type='test') - - start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date() - end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date() - - results = fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True) - - assert len(results) == 1 - assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted, - result_one.service.research_mode, result_one.service.active, result_one.service.created_at, - 'sms', 'created', int(expected)) - - @freeze_time('2001-01-01T23:59:00') def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revoked(notify_db_session): service = create_service() diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a1a40b3a8..737846aa5 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1642,31 +1642,37 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not } -@pytest.mark.parametrize( - 'set_time', - ['2017-03-28T12:00:00', '2017-01-28T12:00:00', '2017-01-02T12:00:00', '2017-10-31T12:00:00'] -) -def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_time): +@pytest.mark.parametrize("start_date_delta, end_date_delta", + [(2, 1), + (3, 2), + (1, 0) + ]) +@freeze_time('2017-03-28T12:00:00') +def test_get_detailed_services_for_date_range(sample_template, start_date_delta, end_date_delta): from app.service.rest import get_detailed_services - with freeze_time(set_time): - create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=3)) - create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=2)) - create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow() - timedelta(days=1)) - create_sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow()) + create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=3)).date(), + service=sample_template.service, + notification_type='sms') + create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=2)).date(), + service=sample_template.service, + notification_type='sms') + create_ft_notification_status(bst_date=(datetime.utcnow() - timedelta(days=1)).date(), + service=sample_template.service, + notification_type='sms') - start_date = (datetime.utcnow() - timedelta(days=2)).date() - end_date = (datetime.utcnow() - timedelta(days=1)).date() + create_notification(template=sample_template, created_at=datetime.utcnow(), status='delivered') + + start_date = (datetime.utcnow() - timedelta(days=start_date_delta)).date() + end_date = (datetime.utcnow() - timedelta(days=end_date_delta)).date() data = get_detailed_services(only_active=False, include_from_test_key=True, start_date=start_date, end_date=end_date) assert len(data) == 1 - assert data[0]['statistics'] == { - EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, - SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 2}, - LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} - } + assert data[0]['statistics'][EMAIL_TYPE] == {'delivered': 0, 'failed': 0, 'requested': 0} + assert data[0]['statistics'][SMS_TYPE] == {'delivered': 2, 'failed': 0, 'requested': 2} + assert data[0]['statistics'][LETTER_TYPE] == {'delivered': 0, 'failed': 0, 'requested': 0} def test_search_for_notification_by_to_field(client, sample_template, sample_email_template):