From cbb9f71e610d30398bd9113bb82c0df609e9cd3b Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 28 Jun 2018 15:02:19 +0100 Subject: [PATCH] Fix bug with test notifications not showing Test notifications are only stored in the notification table, not the notification_history table. We were deciding which table to query for the platform admin stats based on the start date passed to the DAO function. This means that if the start date given was more than 7 days ago and the end date was within the last 7 days, test notifications were not being returned. We now use the end date to choose which table to query which means that test notifications always get returned when they should. --- app/dao/notifications_dao.py | 2 +- .../notification_dao/test_notification_dao.py | 35 ++++++++----------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 88319de34..e0d39e87f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -617,7 +617,7 @@ def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date): end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) table = NotificationHistory - if start_date >= datetime.utcnow() - timedelta(days=7): + if end_date >= datetime.utcnow() - timedelta(days=7): table = Notification query = db.session.query( diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index d7f1401f5..1e05a509c 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1936,27 +1936,6 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_ assert len(results) == 0 -@pytest.mark.parametrize('table_name, days_ago', [ - ('Notification', 8), - ('NotificationHistory', 3), -]) -@freeze_time('2018-01-08') -def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_the_correct_table( - mocker, - notify_db_session, - table_name, - days_ago -): - start_date = datetime.now().date() - timedelta(days=days_ago) - end_date = datetime.now().date() - - # mock the table that should not be used, then check it is not being called - unused_table_mock = mocker.patch('app.dao.services_dao.{}'.format(table_name)) - fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date) - - unused_table_mock.assert_not_called() - - def test_fetch_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session): start_date = date(2018, 1, 1) end_date = date(2018, 1, 5) @@ -2003,3 +1982,17 @@ def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_bst_date(samp assert len(result) == 1 assert result[0].status == 'sent' + + +@freeze_time('2018-01-08T12:00:00') +def test_fetch_aggregate_stats_by_date_range_for_all_services_gets_test_notifications_when_start_date_over_7_days_ago( + sample_template +): + ten_days_ago = datetime.utcnow().date() - timedelta(days=10) + today = datetime.utcnow().date() + + create_notification(sample_template, key_type=KEY_TYPE_TEST, created_at=datetime.utcnow()) + + result = fetch_aggregate_stats_by_date_range_for_all_services(ten_days_ago, today) + + assert len(result) == 1