diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 356b74ed2..3a21fd714 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -103,13 +103,26 @@ def create_notification_statistics_dict(service_id, day): @statsd(namespace="dao") def dao_get_template_usage(service_id, limit_days=None): + query_filter = [] + table = NotificationHistory - if limit_days and limit_days <= 7: # can get this data from notifications table + if limit_days is not None and limit_days <= 7: 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 != 7: + query_filter.append(table.created_at >= days_ago(limit_days)) + + elif limit_days is not None: + # case where not under 7 days, so using NotificationsHistory so limit allowed + query_filter.append(table.created_at >= days_ago(limit_days)) + + query_filter.append(table.service_id == service_id) + query_filter.append(table.key_type != KEY_TYPE_TEST) + + # 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/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 086a24c57..52dd115c6 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -141,7 +141,7 @@ def test_should_be_able_to_get_no_template_usage_history_if_no_notifications_usi assert not results -def test_should_by_able_to_get_template_count_from_notifications_history(notify_db, notify_db_session, sample_service): +def test_should_by_able_to_get_template_count(notify_db, notify_db_session, sample_service): sms = sample_template(notify_db, notify_db_session) email = sample_email_template(notify_db, notify_db_session) sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) @@ -184,7 +184,7 @@ def test_template_history_should_ignore_test_keys( assert results[0].count == 3 -def test_should_by_able_to_get_template_count_from_notifications_history_for_service( +def test_should_by_able_to_get_template_count_limited_for_service( notify_db, notify_db_session): service_1 = sample_service(notify_db, notify_db_session, service_name="test1", email_from="test1") @@ -212,7 +212,7 @@ def test_should_by_able_to_get_zero_count_from_notifications_history_if_no_servi assert len(results) == 0 -def test_should_by_able_to_get_template_count_from_notifications_history_across_days( +def test_should_by_able_to_get_template_count_across_days( notify_db, notify_db_session, sample_service): @@ -246,6 +246,100 @@ def test_should_by_able_to_get_template_count_from_notifications_history_across_ ] +def test_should_by_able_to_get_template_count_for_under_seven_days( + notify_db, + notify_db_session, + sample_service, + sample_template): + + yesterday = datetime.now() - timedelta(days=1) + six_days_ago = datetime.now() - timedelta(days=6) + seven_days_ago = datetime.now() - timedelta(days=7) + eight_days_ago = datetime.now() - timedelta(days=8) + + sample_notification( + notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=six_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=seven_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=eight_days_ago, service=sample_service, template=sample_template + ) + + results = dao_get_template_usage(sample_service.id, limit_days=6) + assert len(results) == 1 + assert [(row.name, row.template_type, row.count) for row in results] == [ + ('Template Name', 'sms', 2) + ] + + +def test_should_by_able_to_get_template_count_for_whole_of_notifications_table_if_seven_days_exactly( + notify_db, + notify_db_session, + sample_service, + sample_template): + yesterday = datetime.now() - timedelta(days=1) + six_days_ago = datetime.now() - timedelta(days=6) + seven_days_ago = datetime.now() - timedelta(days=7) + eight_days_ago = datetime.now() - timedelta(days=8) + + sample_notification( + notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=six_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=seven_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=eight_days_ago, service=sample_service, template=sample_template + ) + + results = dao_get_template_usage(sample_service.id, limit_days=7) + assert len(results) == 1 + # note as we haven't run the delete task they'll ALL be in the notifications table. + assert [(row.name, row.template_type, row.count) for row in results] == [ + ('Template Name', 'sms', 4) + ] + + +def test_should_by_able_to_get_all_template_count_for_more_than_seven_days( + notify_db, + notify_db_session, + sample_service, + sample_template): + yesterday = datetime.now() - timedelta(days=1) + six_days_ago = datetime.now() - timedelta(days=6) + seven_days_ago = datetime.now() - timedelta(days=7) + eight_days_ago = datetime.now() - timedelta(days=8) + + sample_notification( + notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=six_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=seven_days_ago, service=sample_service, template=sample_template + ) + sample_notification( + notify_db, notify_db_session, created_at=eight_days_ago, service=sample_service, template=sample_template + ) + + Notification.query.delete() + # gets all from history table + results = dao_get_template_usage(sample_service.id, limit_days=10) + assert len(results) == 1 + assert [(row.name, row.template_type, row.count) for row in results] == [ + ('Template Name', 'sms', 4) + ] + + def test_should_by_able_to_get_template_count_from_notifications_history_with_day_limit( notify_db, notify_db_session, 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')