From 3ac7507f3b10437c0195c67f49a019d0619c61ce Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 21 Nov 2017 16:29:20 +0000 Subject: [PATCH 1/2] Template usage always aggregating today's stats Added a check to ensure that the current date falls in between the financial year for the year supplied by the method, so that the todays stats will only be appended in that situation. --- app/dao/services_dao.py | 79 ++++++++++++++++-------------- tests/app/dao/test_services_dao.py | 7 +++ 2 files changed, 48 insertions(+), 38 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 58772b0b4..416ed87a8 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -562,47 +562,50 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) stats.append(stat) month = get_london_month_from_utc_column(Notification.created_at) - year = func.date_trunc("year", Notification.created_at) + year_func = func.date_trunc("year", Notification.created_at) start_date = datetime.combine(date.today(), time.min) - today_results = db.session.query( - Notification.template_id, - Template.name, - Template.template_type, - extract('month', month).label('month'), - extract('year', year).label('year'), - func.count().label('count') - ).join( - Template, Notification.template_id == Template.id, - ).filter( - Notification.created_at >= start_date, - Notification.service_id == service_id - ).group_by( - Notification.template_id, - Template.name, - Template.template_type, - month, - year - ).order_by( - Notification.template_id - ).all() + fy_start, fy_end = get_financial_year(year) - for today_result in today_results: - add_to_stats = True - for stat in stats: - if today_result.template_id == stat.template_id and today_result.month == stat.month \ - and today_result.year == stat.year: - stat.count = stat.count + today_result.count - add_to_stats = False + if fy_start < datetime.now() < fy_end: + today_results = db.session.query( + Notification.template_id, + Template.name, + Template.template_type, + extract('month', month).label('month'), + extract('year', year_func).label('year'), + func.count().label('count') + ).join( + Template, Notification.template_id == Template.id, + ).filter( + Notification.created_at >= start_date, + Notification.service_id == service_id + ).group_by( + Notification.template_id, + Template.name, + Template.template_type, + month, + year_func + ).order_by( + Notification.template_id + ).all() - if add_to_stats: - new_stat = type("", (), {})() - new_stat.template_id = today_result.template_id - new_stat.template_type = today_result.template_type - new_stat.name = today_result.name - new_stat.month = int(today_result.month) - new_stat.year = int(today_result.year) - new_stat.count = today_result.count - stats.append(new_stat) + for today_result in today_results: + add_to_stats = True + for stat in stats: + if today_result.template_id == stat.template_id and today_result.month == stat.month \ + and today_result.year == stat.year: + stat.count = stat.count + today_result.count + add_to_stats = False + + if add_to_stats: + new_stat = type("", (), {})() + new_stat.template_id = today_result.template_id + new_stat.template_type = today_result.template_type + new_stat.name = today_result.name + new_stat.month = int(today_result.month) + new_stat.year = int(today_result.year) + new_stat.count = today_result.count + stats.append(new_stat) return stats diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f172769e7..beefc6b86 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1471,6 +1471,13 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_returns_fina assert result[4].month == 3 assert result[4].year == 2018 + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id, 2014), + key=lambda x: (x.year, x.month) + ) + + assert len(result) == 0 + @freeze_time("2018-03-10 11:09:00.000000") def test_dao_fetch_monthly_historical_usage_by_template_for_service_only_returns_for_service( From b453669a7cf503aa1f712fc731f19d7425cd3a3b Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 21 Nov 2017 16:46:29 +0000 Subject: [PATCH 2/2] Added test to ensure todays data isn't aggregated to template usage The integration tests did test for zero return. Added a method to test for a year which has no data and also tweaked the existing tests to ensure they are testing the year more fully. --- tests/app/service/test_rest.py | 83 ++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 13 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 79756c461..f7450cce7 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1862,7 +1862,7 @@ def test_get_template_usage_by_month_returns_correct_data( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='sending' ) @@ -1870,7 +1870,7 @@ def test_get_template_usage_by_month_returns_correct_data( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='permanent-failure' ) @@ -1878,7 +1878,7 @@ def test_get_template_usage_by_month_returns_correct_data( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='temporary-failure' ) @@ -1890,7 +1890,7 @@ def test_get_template_usage_by_month_returns_correct_data( ) resp = client.get( - '/service/{}/notifications/templates_usage/monthly?year=2016'.format(not1.service_id), + '/service/{}/notifications/templates_usage/monthly?year=2017'.format(not1.service_id), headers=[create_authorization_header()] ) resp_json = json.loads(resp.get_data(as_text=True)).get('stats') @@ -1902,8 +1902,8 @@ def test_get_template_usage_by_month_returns_correct_data( assert resp_json[0]["name"] == sample_template.name assert resp_json[0]["type"] == sample_template.template_type assert resp_json[0]["month"] == 4 - assert resp_json[0]["year"] == 2016 - assert resp_json[0]["count"] == 4 + assert resp_json[0]["year"] == 2017 + assert resp_json[0]["count"] == 3 assert resp_json[1]["template_id"] == str(sample_template.id) assert resp_json[1]["name"] == sample_template.name @@ -1913,6 +1913,63 @@ def test_get_template_usage_by_month_returns_correct_data( assert resp_json[1]["count"] == 1 +@freeze_time('2017-11-11 02:00') +def test_get_template_usage_by_month_returns_no_data( + notify_db, + notify_db_session, + client, + sample_template +): + + # add a historical notification for template + not1 = create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2017, 4, 1), + status='sending' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2017, 4, 1), + status='permanent-failure' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2017, 4, 1), + status='temporary-failure' + ) + + daily_stats_template_usage_by_month() + + create_notification( + sample_template, + created_at=datetime.utcnow() + ) + + resp = client.get( + '/service/{}/notifications/templates_usage/monthly?year=2015'.format(not1.service_id), + headers=[create_authorization_header()] + ) + resp_json = json.loads(resp.get_data(as_text=True)).get('stats') + + assert resp.status_code == 200 + assert len(resp_json) == 0 + + @freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_two_templates( notify_db, @@ -1929,14 +1986,14 @@ def test_get_template_usage_by_month_returns_two_templates( notify_db, notify_db_session, template_one, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), ) create_notification_history( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='sending' ) @@ -1944,7 +2001,7 @@ def test_get_template_usage_by_month_returns_two_templates( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='permanent-failure' ) @@ -1952,7 +2009,7 @@ def test_get_template_usage_by_month_returns_two_templates( notify_db, notify_db_session, sample_template, - created_at=datetime(2016, 4, 1), + created_at=datetime(2017, 4, 1), status='temporary-failure' ) @@ -1964,7 +2021,7 @@ def test_get_template_usage_by_month_returns_two_templates( ) resp = client.get( - '/service/{}/notifications/templates_usage/monthly?year=2016'.format(not1.service_id), + '/service/{}/notifications/templates_usage/monthly?year=2017'.format(not1.service_id), headers=[create_authorization_header()] ) resp_json = json.loads(resp.get_data(as_text=True)).get('stats') @@ -1978,14 +2035,14 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[0]["name"] == template_one.name assert resp_json[0]["type"] == template_one.template_type assert resp_json[0]["month"] == 4 - assert resp_json[0]["year"] == 2016 + assert resp_json[0]["year"] == 2017 assert resp_json[0]["count"] == 1 assert resp_json[1]["template_id"] == str(sample_template.id) assert resp_json[1]["name"] == sample_template.name assert resp_json[1]["type"] == sample_template.template_type assert resp_json[1]["month"] == 4 - assert resp_json[1]["year"] == 2016 + assert resp_json[1]["year"] == 2017 assert resp_json[1]["count"] == 3 assert resp_json[2]["template_id"] == str(sample_template.id)