From 3702ebdd93a3a81c19ecbc371b902d4f9327f40d Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 8 Jun 2017 13:42:41 +0100 Subject: [PATCH] This is a bit gnarly. When querying the notifications table for template usage, we don't apply the days limit if we're trying to get exactly 7 days. This is because 7days should be the amount of data in that table so don't restrict. Note not all queries do this in the same way and a pivotal bug has been raised to align this. This is a bug fix as right now the numbers are out in prod. --- app/dao/notifications_dao.py | 4 +++- tests/app/template_statistics/test_rest.py | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 22da11966..ccfb280b0 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -109,7 +109,9 @@ def dao_get_template_usage(service_id, limit_days=None): table = Notification query_filter = [table.service_id == service_id, table.key_type != KEY_TYPE_TEST] - if limit_days is not None: + + # only limit days if it's not seven days, as 7 days == the whole of Notifications table. + if limit_days is not None and limit_days != 7: query_filter.append(table.created_at >= days_ago(limit_days)) notifications_aggregate_query = db.session.query( diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index a5c9bd0a4..4f79541ec 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -87,8 +87,8 @@ def test_get_template_statistics_for_service_limit_7_days(notify_db, notify_db_s mocker, cache_values): email, sms = set_up_notifications(notify_db, notify_db_session) - mock_cache_values = {str.encode(str(sms.id)): str.encode('2'), - str.encode(str(email.id)): str.encode('2')} if cache_values else None + mock_cache_values = {str.encode(str(sms.id)): str.encode('3'), + str.encode(str(email.id)): str.encode('3')} if cache_values else None mocked_redis_get = mocker.patch('app.redis_store.get_all_from_hash', return_value=mock_cache_values) mocked_redis_set = mocker.patch('app.redis_store.set_hash_and_expire') @@ -102,9 +102,9 @@ def test_get_template_statistics_for_service_limit_7_days(notify_db, notify_db_s assert response_for_a_week.status_code == 200 json_resp = json.loads(response_for_a_week.get_data(as_text=True)) assert len(json_resp['data']) == 2 - assert json_resp['data'][0]['count'] == 2 + assert json_resp['data'][0]['count'] == 3 assert json_resp['data'][0]['template_name'] == 'Email Template Name' - assert json_resp['data'][1]['count'] == 2 + assert json_resp['data'][1]['count'] == 3 assert json_resp['data'][1]['template_name'] == 'Template Name' mocked_redis_get.assert_called_once_with("{}-template-counter-limit-7-days".format(email.service_id)) @@ -112,7 +112,7 @@ def test_get_template_statistics_for_service_limit_7_days(notify_db, notify_db_s mocked_redis_set.assert_not_called() else: mocked_redis_set.assert_called_once_with("{}-template-counter-limit-7-days".format(email.service_id), - {sms.id: 2, email.id: 2}, 600) + {sms.id: 3, email.id: 3}, 600) @freeze_time('2016-08-18')