From a1760044c75683b6d2c6988a696cc7f510f5b782 Mon Sep 17 00:00:00 2001 From: Jumana B Date: Fri, 20 Sep 2024 09:35:05 -0400 Subject: [PATCH] Fix Notification History to have the full day of data (#2293) * Set NH table to have full days history vs partial day * fix tests * add log --- app/dao/notifications_dao.py | 23 ++++++++------ ...t_notification_dao_delete_notifications.py | 30 +++++++++---------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index e88df7cbee..b95ad178e1 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,6 +1,6 @@ import functools import string -from datetime import datetime, timedelta +from datetime import datetime, time, timedelta, timezone from flask import current_app from itsdangerous import BadSignature @@ -49,7 +49,7 @@ Service, ServiceDataRetention, ) -from app.utils import escape_special_characters, get_local_timezone_midnight_in_utc +from app.utils import escape_special_characters @transactional @@ -384,26 +384,31 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim flexible_data_retention = ServiceDataRetention.query.filter(ServiceDataRetention.notification_type == notification_type).all() deleted = 0 for f in flexible_data_retention: - days_of_retention = get_local_timezone_midnight_in_utc( - convert_utc_to_local_timezone(datetime.utcnow()).date() - ) - timedelta(days=f.days_of_retention) + days_of_retention = datetime.combine(datetime.now(timezone.utc) - timedelta(days=f.days_of_retention), time.max) insert_update_notification_history(notification_type, days_of_retention, f.service_id) - current_app.logger.info("Deleting {} notifications for service id: {}".format(notification_type, f.service_id)) + current_app.logger.info( + "Deleting {} notifications for service id: {} uptil {} retention_days {}".format( + notification_type, f.service_id, days_of_retention, f.days_of_retention + ) + ) deleted += _delete_notifications(notification_type, days_of_retention, f.service_id, qry_limit) current_app.logger.info("Deleting {} notifications for services without flexible data retention".format(notification_type)) - seven_days_ago = get_local_timezone_midnight_in_utc(convert_utc_to_local_timezone(datetime.utcnow()).date()) - timedelta( - days=7 - ) + seven_days_ago = datetime.combine(datetime.now(timezone.utc) - timedelta(days=7), time.max) services_with_data_retention = [x.service_id for x in flexible_data_retention] service_ids_to_purge = db.session.query(Service.id).filter(Service.id.notin_(services_with_data_retention)).all() for row in service_ids_to_purge: service_id = row._mapping["id"] insert_update_notification_history(notification_type, seven_days_ago, service_id) + current_app.logger.info( + "Deleting {} notifications for service id: {} uptil {} for 7days".format( + notification_type, service_id, seven_days_ago + ) + ) deleted += _delete_notifications(notification_type, seven_days_ago, service_id, qry_limit) current_app.logger.info("Finished deleting {} notifications".format(notification_type)) diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 2601522739..0b409ad67f 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -1,4 +1,4 @@ -from datetime import date, datetime, timedelta +from datetime import date, datetime, timedelta, timezone import pytest from freezegun import freeze_time @@ -95,10 +95,14 @@ def _create_templates(sample_service): return email_template, letter_template, sms_template -@pytest.mark.parametrize("month, delete_run_time", [(4, "2016-04-10 23:40"), (1, "2016-01-11 00:40")]) @pytest.mark.parametrize( - "notification_type, expected_sms_count, expected_email_count, expected_letter_count", - [("sms", 7, 10, 10), ("email", 10, 7, 10)], + "month, delete_run_time, notification_type, expected_sms_count, expected_email_count", + [ + (4, "2016-04-10 23:40", "sms", 7, 10), + (4, "2016-04-10 23:40", "email", 10, 7), + (1, "2016-01-11 00:40", "sms", 6, 10), + (1, "2016-01-11 00:40", "email", 10, 6), + ], ) def test_should_delete_notifications_by_type_after_seven_days( sample_service, @@ -108,9 +112,8 @@ def test_should_delete_notifications_by_type_after_seven_days( notification_type, expected_sms_count, expected_email_count, - expected_letter_count, ): - email_template, letter_template, sms_template = _create_templates(sample_service) + email_template, _, sms_template = _create_templates(sample_service) # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type for i in range(1, 11): past_date = "2016-0{0}-{1:02d} {1:02d}:00:00.000000".format(month, i) @@ -118,37 +121,32 @@ def test_should_delete_notifications_by_type_after_seven_days( save_notification( create_notification( template=email_template, - created_at=datetime.utcnow(), + created_at=datetime.now(timezone.utc), status="permanent-failure", ) ) - save_notification(create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered")) save_notification( create_notification( - template=letter_template, - created_at=datetime.utcnow(), - status="temporary-failure", + template=sms_template, + created_at=datetime.now(timezone.utc), + status="delivered", ) ) - assert Notification.query.count() == 30 + assert Notification.query.count() == 20 # Records from before 3rd should be deleted with freeze_time(delete_run_time): delete_notifications_older_than_retention_by_type(notification_type) remaining_sms_notifications = Notification.query.filter_by(notification_type="sms").all() - remaining_letter_notifications = Notification.query.filter_by(notification_type="letter").all() remaining_email_notifications = Notification.query.filter_by(notification_type="email").all() assert len(remaining_sms_notifications) == expected_sms_count assert len(remaining_email_notifications) == expected_email_count - assert len(remaining_letter_notifications) == expected_letter_count if notification_type == "sms": notifications_to_check = remaining_sms_notifications if notification_type == "email": notifications_to_check = remaining_email_notifications - if notification_type == "letter": - notifications_to_check = remaining_letter_notifications for notification in notifications_to_check: assert notification.created_at.date() >= date(2016, month, 3)