diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 1a6e1ad3d..d142fb595 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -311,11 +311,9 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim day_to_delete_backwards_from = get_london_midnight_in_utc( convert_utc_to_bst(datetime.utcnow()).date()) - timedelta(days=f.days_of_retention) - delete_count_per_call = 1 - while delete_count_per_call > 0: - delete_count_per_call = _move_notifications_to_notification_history( - notification_type, f.service_id, day_to_delete_backwards_from, qry_limit) - deleted += delete_count_per_call + + deleted += _move_notifications_to_notification_history( + notification_type, f.service_id, day_to_delete_backwards_from, qry_limit) current_app.logger.info( 'Deleting {} notifications for services without flexible data retention'.format(notification_type)) @@ -399,23 +397,15 @@ def _move_notifications_to_notification_history(notification_type, service_id, d _delete_letters_from_s3( notification_type, service_id, day_to_delete_backwards_from, qry_limit ) - - stop = -1 # exclusive, we want to include 0 - step = -1 - for hour_delta in range(23, stop, step): - # We find the timestamp we want to delete all notifications backwards from - # We then start 23 hours ago, and do an insert notification history before deleting all notifications older - # We then look 22 hours ago, do an insert notifications history before deleting all notifications older - # We continue this until we reach the original timestamp we wanted to delete notifications backwardsfrom - # This enables us to break this into smaller database queries - timestamp_to_delete_backwards_from = day_to_delete_backwards_from - timedelta(hours=hour_delta) - - deleted += insert_notification_history_delete_notifications( + delete_count_per_call = 1 + while delete_count_per_call > 0: + delete_count_per_call = insert_notification_history_delete_notifications( notification_type=notification_type, service_id=service_id, - timestamp_to_delete_backwards_from=timestamp_to_delete_backwards_from, + timestamp_to_delete_backwards_from=day_to_delete_backwards_from, qry_limit=qry_limit ) + deleted += delete_count_per_call # Deleting test Notifications, test notifications are not persisted to NotificationHistory Notification.query.filter( 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 78e559e96..88a517a1a 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 @@ -222,10 +222,14 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee mock_get_s3.assert_not_called() +@freeze_time('2020-03-25 00:01') def test_delete_notifications_calls_subquery_multiple_times(sample_template): - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered') - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered') - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered') + create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3), + status='delivered') + create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3), + status='delivered') + create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3), + status='delivered') assert Notification.query.count() == 3 delete_notifications_older_than_retention_by_type('sms', qry_limit=1)