From 8ad078b663d53ea4f26250de330e695990c92c02 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 28 Dec 2016 15:39:55 +0000 Subject: [PATCH 1/3] Added a date range filter for the get all services end point. When the start_date and end_date query argruments exists in the request, the query will return the results from the NotificationHistory table for the given date range. We will need to check the performance of this query, but this will only be used by the platform admin page. --- app/dao/services_dao.py | 28 ++++++++++++++++++++++++++++ app/service/rest.py | 20 ++++++++++++++------ tests/app/dao/test_services_dao.py | 21 +++++++++++++++++++-- tests/app/service/test_rest.py | 21 ++++++++++++++++++++- 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 71896a5df..8e982c9da 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -263,3 +263,31 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): query = query.filter(Notification.key_type != KEY_TYPE_TEST) return query + + +@statsd(namespace='dao') +def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True): + query = db.session.query( + NotificationHistory.notification_type, + NotificationHistory.status, + NotificationHistory.service_id, + func.count(NotificationHistory.id).label('count') + ).select_from( + Service + ).join( + NotificationHistory + ).filter( + func.date(NotificationHistory.created_at) >= start_date, + func.date(NotificationHistory.created_at) <= end_date + ).group_by( + NotificationHistory.notification_type, + NotificationHistory.status, + NotificationHistory.service_id + ).order_by( + NotificationHistory.service_id + ) + + if not include_from_test_key: + query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) + + return query diff --git a/app/service/rest.py b/app/service/rest.py index 1c1e9b77c..29738141f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -26,7 +26,8 @@ from app.dao.services_dao import ( dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, dao_fetch_todays_stats_for_all_services, - dao_deactive_service + dao_deactive_service, + fetch_stats_by_date_range_for_all_services ) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, @@ -63,11 +64,14 @@ def get_services(): detailed = request.args.get('detailed') == 'True' user_id = request.args.get('user_id', None) include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' + start_date = request.args.get('start_date', None) + end_date = request.args.get('end_date', None) if user_id: services = dao_fetch_all_services_by_user(user_id, only_active) elif detailed: - return jsonify(data=get_detailed_services(only_active, include_from_test_key=include_from_test_key)) + return jsonify(data=get_detailed_services(only_active, include_from_test_key=include_from_test_key, + start_date=start_date, end_date=end_date)) else: services = dao_fetch_all_services(only_active) data = service_schema.dump(services, many=True).data @@ -269,18 +273,22 @@ def get_detailed_service(service_id, today_only=False): return detailed_service_schema.dump(service).data -def get_detailed_services(only_active=False, include_from_test_key=True): +def get_detailed_services(only_active=False, include_from_test_key=True, start_date=None, end_date=None): services = {service.id: service for service in dao_fetch_all_services(only_active)} - stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) + if start_date: + stats = fetch_stats_by_date_range_for_all_services(start_date=start_date, + end_date=end_date, + include_from_test_key=include_from_test_key) + else: + stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): - services[service_id].statistics = statistics.format_statistics(rows) + services[service_id].statistics = statistics.format_statistics(rows) # if service has not sent anything, query will not have set statistics correctly for service in services.values(): if not hasattr(service, 'statistics'): service.statistics = statistics.create_zeroed_stats_dicts() - return detailed_service_schema.dump(services.values(), many=True).data diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index eacca3db3..896089a24 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 +from datetime import datetime, timedelta import uuid import functools @@ -21,7 +21,8 @@ from app.dao.services_dao import ( dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, fetch_todays_total_message_count, - dao_fetch_todays_stats_for_all_services + dao_fetch_todays_stats_for_all_services, + fetch_stats_by_date_range_for_all_services ) from app.dao.users_dao import save_model_user from app.models import ( @@ -639,3 +640,19 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key(notif assert len(stats) == 1 assert stats[0].count == 2 + + +def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session): + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=4)) + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=3)) + result_one = create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=2)) + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=1)) + create_notification(notify_db, notify_db_session, 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).all() + + assert len(results) == 1 + assert results[0] == ('sms', 'created', result_one.service_id, 2) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3641982d1..cf5f458ca 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timedelta import json import uuid @@ -1363,6 +1363,25 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not } +def test_get_detailed_services_for_date_range(notify_db, notify_db_session): + from app.service.rest import get_detailed_services + create_sample_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=3)) + create_sample_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=2)) + create_sample_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=1)) + create_sample_notification(notify_db, notify_db_session, created_at=datetime.now()) + + start_date = (datetime.utcnow() - timedelta(days=2)).date() + end_date = (datetime.utcnow() - timedelta(days=1)).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': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + } + + @freeze_time('2012-12-12T12:00:01') def test_get_notification_billable_unit_count(client, notify_db, notify_db_session): notification = create_sample_notification(notify_db, notify_db_session) From 1de022f0054cee8476134dc73be91451ffed96a8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 29 Dec 2016 13:28:55 +0000 Subject: [PATCH 2/3] Update the query to execute immediately. Fix indent. Left a comment as to why start and end date are not set. --- app/dao/services_dao.py | 4 ++-- app/service/rest.py | 3 ++- tests/app/dao/test_services_dao.py | 12 ++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 8e982c9da..aaefec145 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -262,7 +262,7 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): if not include_from_test_key: query = query.filter(Notification.key_type != KEY_TYPE_TEST) - return query + return query.all() @statsd(namespace='dao') @@ -290,4 +290,4 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro if not include_from_test_key: query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) - return query + return query.all() diff --git a/app/service/rest.py b/app/service/rest.py index 29738141f..02eb65830 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -64,6 +64,7 @@ def get_services(): detailed = request.args.get('detailed') == 'True' user_id = request.args.get('user_id', None) include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' + # If start and end date are not set in the request.args, we are expecting today's stats. start_date = request.args.get('start_date', None) end_date = request.args.get('end_date', None) @@ -283,7 +284,7 @@ def get_detailed_services(only_active=False, include_from_test_key=True, start_d stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): - services[service_id].statistics = statistics.format_statistics(rows) + services[service_id].statistics = statistics.format_statistics(rows) # if service has not sent anything, query will not have set statistics correctly for service in services.values(): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 896089a24..a3f5d1de7 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -576,7 +576,7 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_services(notify_db notify_db, notify_db_session, service=service2, template=create_email_template(notify_db, notify_db_session, service=service2)) - stats = dao_fetch_todays_stats_for_all_services().all() + stats = dao_fetch_todays_stats_for_all_services() assert len(stats) == 4 # services are ordered by service id; not explicit on email/sms or status @@ -591,7 +591,7 @@ def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db): just_after_midnight_today = create_notification(notify_db, None, to_field='2', status='failed') with freeze_time('2001-01-02T12:00:00'): - stats = dao_fetch_todays_stats_for_all_services().all() + stats = dao_fetch_todays_stats_for_all_services() stats = {row.status: row.count for row in stats} assert 'delivered' not in stats @@ -611,7 +611,7 @@ def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, not # service2: 1 sms "created" create_notification(notify_db, notify_db_session, service=service2) - stats = dao_fetch_todays_stats_for_all_services().all() + stats = dao_fetch_todays_stats_for_all_services() assert len(stats) == 4 assert ('sms', 'created', service1.id, 2) in stats @@ -625,7 +625,7 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default(no create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEAM) create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) - stats = dao_fetch_todays_stats_for_all_services().all() + stats = dao_fetch_todays_stats_for_all_services() assert len(stats) == 1 assert stats[0].count == 3 @@ -636,7 +636,7 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key(notif create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEAM) create_notification(notify_db, notify_db_session, key_type=KEY_TYPE_TEST) - stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=False).all() + stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=False) assert len(stats) == 1 assert stats[0].count == 2 @@ -652,7 +652,7 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session 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).all() + results = fetch_stats_by_date_range_for_all_services(start_date, end_date) assert len(results) == 1 assert results[0] == ('sms', 'created', result_one.service_id, 2) From 0ec84ff5e84a782bdf5b920dcd15e596ee7f678c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 29 Dec 2016 13:50:41 +0000 Subject: [PATCH 3/3] Refactor the get_detailed_services so that the start and end date are not defaulted to None. Set the start and end date to today's date if they are not set in the request.args --- app/service/rest.py | 20 +++++++++++--------- tests/app/service/test_rest.py | 7 ++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 02eb65830..cdca5045b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,4 +1,5 @@ import itertools +from datetime import datetime from flask import ( jsonify, @@ -64,15 +65,16 @@ def get_services(): detailed = request.args.get('detailed') == 'True' user_id = request.args.get('user_id', None) include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' - # If start and end date are not set in the request.args, we are expecting today's stats. - start_date = request.args.get('start_date', None) - end_date = request.args.get('end_date', None) + # If start and end date are not set, we are expecting today's stats. + start_date = request.args.get('start_date', datetime.utcnow().date()) + end_date = request.args.get('end_date', datetime.utcnow().date()) if user_id: services = dao_fetch_all_services_by_user(user_id, only_active) elif detailed: - return jsonify(data=get_detailed_services(only_active, include_from_test_key=include_from_test_key, - start_date=start_date, end_date=end_date)) + return jsonify(data=get_detailed_services(start_date=start_date, end_date=end_date, + only_active=only_active, include_from_test_key=include_from_test_key + )) else: services = dao_fetch_all_services(only_active) data = service_schema.dump(services, many=True).data @@ -274,14 +276,14 @@ def get_detailed_service(service_id, today_only=False): return detailed_service_schema.dump(service).data -def get_detailed_services(only_active=False, include_from_test_key=True, start_date=None, end_date=None): +def get_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True): services = {service.id: service for service in dao_fetch_all_services(only_active)} - if start_date: + if start_date == datetime.utcnow().date(): + stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) + else: stats = fetch_stats_by_date_range_for_all_services(start_date=start_date, end_date=end_date, include_from_test_key=include_from_test_key) - else: - stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index cf5f458ca..8a291a1a6 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1305,7 +1305,7 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): create_sample_notification(notify_db, notify_db_session, service=service_1, status='delivered') create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') - data = get_detailed_services() + data = get_detailed_services(start_date=datetime.utcnow().date(), end_date=datetime.utcnow().date()) data = sorted(data, key=lambda x: x['name']) assert len(data) == 2 @@ -1329,7 +1329,8 @@ def test_get_detailed_services_includes_services_with_no_notifications(notify_db create_sample_notification(notify_db, notify_db_session, service=service_1) - data = get_detailed_services() + data = get_detailed_services(start_date=datetime.utcnow().date(), + end_date=datetime.utcnow().date()) data = sorted(data, key=lambda x: x['name']) assert len(data) == 2 @@ -1353,7 +1354,7 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 12, 0)) with freeze_time('2015-10-10T12:00:00'): - data = get_detailed_services() + data = get_detailed_services(start_date=datetime.utcnow().date(), end_date=datetime.utcnow().date()) data = sorted(data, key=lambda x: x['id']) assert len(data) == 1