From eee662e5bea867e5db2ca1cb26500c0cf4fb5cea Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 3 Apr 2017 15:52:52 +0100 Subject: [PATCH 1/3] Make the code neater for the query. --- app/dao/services_dao.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index ff75a3c93..0f9a234dc 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -325,8 +325,7 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): @statsd(namespace='dao') def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True): start_date = get_london_midnight_in_utc(start_date) - end_date = get_london_midnight_in_utc(end_date) - end_date += timedelta(hours=23, minutes=59, seconds=59) + end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) query = db.session.query( NotificationHistory.notification_type, @@ -335,7 +334,7 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro func.count(NotificationHistory.id).label('count') ).filter( NotificationHistory.created_at >= start_date, - NotificationHistory.created_at <= end_date + NotificationHistory.created_at < end_date ).group_by( NotificationHistory.notification_type, NotificationHistory.status, From 0506bc3b0c9ce3071abbb03352873aa81e636b72 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 4 Apr 2017 16:34:20 +0100 Subject: [PATCH 2/3] Updated the fetch_stats_by_date_range to return test notifications if the start and end date are with in the last 7 days. If the date range is with the last 7 days we query Notifcations. If the date range is outside of the last 7 days we query NotificationHistory. NotificationHistory does not persist notifications created with a test api key. --- app/dao/services_dao.py | 56 +++++++++++++++++++++--------- tests/app/dao/test_services_dao.py | 34 ++++++++++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 0f9a234dc..092708767 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -327,24 +327,46 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro start_date = get_london_midnight_in_utc(start_date) end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) - query = db.session.query( - NotificationHistory.notification_type, - NotificationHistory.status, - NotificationHistory.service_id, - func.count(NotificationHistory.id).label('count') - ).filter( - NotificationHistory.created_at >= start_date, - NotificationHistory.created_at < end_date - ).group_by( - NotificationHistory.notification_type, - NotificationHistory.status, - NotificationHistory.service_id - ).order_by( - NotificationHistory.service_id - ) + if start_date >= datetime.utcnow() - timedelta(days=7): + # Use notifications table. + # This should improve performance for the default query and allow us to see test messages for the last week. + query = db.session.query( + Notification.notification_type, + Notification.status, + Notification.service_id, + func.count(Notification.id).label('count') + ).filter( + Notification.created_at >= start_date, + Notification.created_at < end_date + ).group_by( + Notification.notification_type, + Notification.status, + Notification.service_id + ).order_by( + Notification.service_id + ) + if not include_from_test_key: + query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) - if not include_from_test_key: - query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) + else: + query = db.session.query( + NotificationHistory.notification_type, + NotificationHistory.status, + NotificationHistory.service_id, + func.count(NotificationHistory.id).label('count') + ).filter( + NotificationHistory.created_at >= start_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.all() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f3195ce52..1b90a8539 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -647,6 +647,40 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(samp 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 +def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(notify_db, + notify_db_session, + sample_api_key, + start_delta, + end_delta, + expected): + result_one = create_notification(notify_db, notify_db_session, created_at=datetime.now(), + api_key_id=sample_api_key.id, key_type='test') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=2), + api_key_id=sample_api_key.id, key_type='test') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=3), + api_key_id=sample_api_key.id, key_type='test') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=4), + api_key_id=sample_api_key.id, key_type='normal') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=4), + api_key_id=sample_api_key.id, key_type='test') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=8), + api_key_id=sample_api_key.id, key_type='test') + create_notification(notify_db, notify_db_session, created_at=datetime.now() - timedelta(days=8), + api_key_id=sample_api_key.id, 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] == ('sms', 'created', result_one.service_id, int(expected)) + + @freeze_time('2001-01-01T23:59:00') def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revoked(sample_service, sample_api_key): dao_suspend_service(sample_service.id) From 9e175f42e8f0ba5e63b32a435c3687e42b6e7c01 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 6 Apr 2017 11:18:21 +0100 Subject: [PATCH 3/3] Refactor to make code more readible. --- app/dao/services_dao.py | 56 ++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 092708767..928b3188b 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -326,47 +326,29 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True): def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=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): - # Use notifications table. - # This should improve performance for the default query and allow us to see test messages for the last week. - query = db.session.query( - Notification.notification_type, - Notification.status, - Notification.service_id, - func.count(Notification.id).label('count') - ).filter( - Notification.created_at >= start_date, - Notification.created_at < end_date - ).group_by( - Notification.notification_type, - Notification.status, - Notification.service_id - ).order_by( - Notification.service_id - ) - if not include_from_test_key: - query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) + table = Notification - else: - query = db.session.query( - NotificationHistory.notification_type, - NotificationHistory.status, - NotificationHistory.service_id, - func.count(NotificationHistory.id).label('count') - ).filter( - NotificationHistory.created_at >= start_date, - NotificationHistory.created_at < end_date - ).group_by( - NotificationHistory.notification_type, - NotificationHistory.status, - NotificationHistory.service_id - ).order_by( - NotificationHistory.service_id - ) + query = 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 + ).order_by( + table.service_id + ) - if not include_from_test_key: - query = query.filter(NotificationHistory.key_type != KEY_TYPE_TEST) + if not include_from_test_key: + query = query.filter(table.key_type != KEY_TYPE_TEST) return query.all()