From d7f6bb99762c795ae1a2f2d9fd3e31370bf88ced Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 27 Apr 2016 10:58:08 +0100 Subject: [PATCH] Align date range for template + notification stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were using two different queries to filter template stats to the past 7 days, plus today. Since we’re storing both as short dates, we can now use the same query for both. 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 -------|---------|-----------|----------|--------|----------|--------|------- Monday | Tuesday | Wednesday | Thursday | Friday | Saturday | Sunday | Monday So if we are on Monday, the stats should include today, plus everything back to last Monday. Previously the template stats query was only going back to the Tuesday. This should mean the numbers on the dashboard always line up. --- app/dao/notifications_dao.py | 35 ++++++++++------------ app/notifications_statistics/rest.py | 7 ++--- tests/app/dao/test_notification_dao.py | 11 +++---- tests/app/template_statistics/test_rest.py | 2 +- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index b8c1be710..fddb8ad44 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -39,10 +39,15 @@ def get_sms_message_count(char_count): return 1 if char_count <= 160 else math.ceil(float(char_count) / 153) -def dao_get_notification_statistics_for_service(service_id): - return NotificationStatistics.query.filter_by( - service_id=service_id - ).order_by(desc(NotificationStatistics.day)).all() +def dao_get_notification_statistics_for_service(service_id, limit_days=None): + filter = [NotificationStatistics.service_id == service_id] + if limit_days is not None: + filter.append(NotificationStatistics.day >= days_ago(limit_days)) + return NotificationStatistics.query.filter( + *filter + ).order_by( + desc(NotificationStatistics.day) + ).all() def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -52,24 +57,10 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day): ).order_by(desc(NotificationStatistics.day)).first() -def dao_get_notification_statistics_for_service_and_previous_days(service_id, limit_days): - return NotificationStatistics.query.filter_by( - service_id=service_id - ).filter( - NotificationStatistics.day.in_(( - (date.today() - timedelta(days=days_ago)) - for days_ago in range(0, limit_days + 1) - )) - ).order_by( - desc(NotificationStatistics.day) - ).all() - - def dao_get_template_statistics_for_service(service_id, limit_days=None): filter = [TemplateStatistics.service_id == service_id] - if limit_days: - last_date_to_fetch = date.today() - timedelta(days=limit_days) - filter.append(TemplateStatistics.day > last_date_to_fetch) + if limit_days is not None: + filter.append(TemplateStatistics.day >= days_ago(limit_days)) return TemplateStatistics.query.filter(*filter).order_by( desc(TemplateStatistics.updated_at)).all() @@ -264,3 +255,7 @@ def delete_notifications_created_more_than_a_week_ago(status): ).delete(synchronize_session='fetch') db.session.commit() return deleted + + +def days_ago(number_of_days): + return date.today() - timedelta(days=number_of_days) diff --git a/app/notifications_statistics/rest.py b/app/notifications_statistics/rest.py index 327b1cd04..f4e0cea74 100644 --- a/app/notifications_statistics/rest.py +++ b/app/notifications_statistics/rest.py @@ -4,10 +4,7 @@ from flask import ( request ) -from app.dao.notifications_dao import ( - dao_get_notification_statistics_for_service, - dao_get_notification_statistics_for_service_and_previous_days -) +from app.dao.notifications_dao import dao_get_notification_statistics_for_service from app.schemas import notifications_statistics_schema notifications_statistics = Blueprint( @@ -25,7 +22,7 @@ def get_all_notification_statistics_for_service(service_id): if request.args.get('limit_days'): try: - statistics = dao_get_notification_statistics_for_service_and_previous_days( + statistics = dao_get_notification_statistics_for_service( service_id=service_id, limit_days=int(request.args['limit_days']) ) diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 6054c1569..0da5e8715 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -24,7 +24,6 @@ from app.dao.notifications_dao import ( dao_get_notification_statistics_for_service, delete_notifications_created_more_than_a_week_ago, dao_get_notification_statistics_for_service_and_day, - dao_get_notification_statistics_for_service_and_previous_days, update_notification_status_by_id, update_notification_reference_by_id, update_notification_status_by_reference, @@ -316,7 +315,7 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre dao_create_notification(notification_2, sample_template.template_type, mmg_provider_name) dao_create_notification(notification_3, sample_template.template_type, mmg_provider_name) - stats = dao_get_notification_statistics_for_service_and_previous_days( + stats = dao_get_notification_statistics_for_service( sample_template.service.id, 7 ) assert len(stats) == 2 @@ -1000,7 +999,7 @@ def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_r template_stats = dao_get_template_statistics_for_service(sample_template.service.id) assert len(template_stats) == 0 - # make 9 stats records from 1st to 9th April + # Make 9 stats records from 1st to 9th April for i in range(1, 10): past_date = '2016-04-0{}'.format(i) with freeze_time(past_date): @@ -1011,9 +1010,11 @@ def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_r # Retrieve last week of stats template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7) - assert len(template_stats) == 7 + assert len(template_stats) == 8 assert template_stats[0].day == date(2016, 4, 9) - assert template_stats[6].day == date(2016, 4, 3) + # Final day of stats should be the same as today, eg Monday + assert template_stats[0].day.isoweekday() == template_stats[7].day.isoweekday() + assert template_stats[7].day == date(2016, 4, 2) @freeze_time('2016-04-09') diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index a8d4302e9..7e5ed624c 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -35,7 +35,7 @@ def test_get_template_statistics_for_service_for_last_week(notify_api, sample_te assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 7 + assert len(json_resp['data']) == 8 assert json_resp['data'][0]['day'] == '2016-04-09' assert json_resp['data'][6]['day'] == '2016-04-03'