Refactor code to make it clearer

- Added code in `delete_notifications_created_more_than_a_week_ago_by_type` to remove notifications to email_reply_to older than 7 days
- Added transactional to `delete_notifications_created_more_than_a_week_ago_by_type`

app/celery/scheduled_tasks.py

@@ -27,7 +27,6 @@ from app.dao.notifications_dao import (
     dao_timeout_notifications,
     is_delivery_slow_for_provider,
     delete_notifications_created_more_than_a_week_ago_by_type,
-    delete_notification_to_email_reply_to_more_than_a_week_ago,
     dao_get_scheduled_notifications,
     set_scheduled_notification_to_processed,
     dao_set_created_live_letter_api_notifications_to_pending,
@@ -119,17 +118,6 @@ def delete_sms_notifications_older_than_seven_days():
 @statsd(namespace="tasks")
 def delete_email_notifications_older_than_seven_days():
     try:
-        start = datetime.utcnow()
-        deleted_noti_to_reply_to = delete_notification_to_email_reply_to_more_than_a_week_ago()
-        current_app.logger.info(
-            "Delete {} job started {} finished {} deleted {} notification to email reply to mappings".format(
-                'email',
-                start,
-                datetime.utcnow(),
-                deleted_noti_to_reply_to
-            )
-        )
-
         start = datetime.utcnow()
         deleted = delete_notifications_created_more_than_a_week_ago_by_type('email')
         current_app.logger.info(

app/dao/notifications_dao.py

@@ -32,9 +32,9 @@ from app.models import (
     ScheduledNotification,
     ServiceEmailReplyTo,
     Template,
+    EMAIL_TYPE,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEST,
-    EMAIL_TYPE,
     LETTER_TYPE,
     NOTIFICATION_CREATED,
     NOTIFICATION_DELIVERED,
@@ -367,29 +367,26 @@ def _filter_query(query, filter_dict=None):
 
 
 @statsd(namespace="dao")
+@transactional
 def delete_notifications_created_more_than_a_week_ago_by_type(notification_type):
     seven_days_ago = date.today() - timedelta(days=7)
+
+    # Following could be refactored when NotificationSmsReplyTo and NotificationLetterContact in models.py
+    if notification_type == EMAIL_TYPE:
+        subq = db.session.query(Notification.id).filter(
+            func.date(Notification.created_at) < seven_days_ago,
+            Notification.notification_type == notification_type
+        ).subquery()
+        deleted = db.session.query(
+            NotificationEmailReplyTo
+        ).filter(
+            NotificationEmailReplyTo.notification_id.in_(subq)
+        ).delete(synchronize_session='fetch')
+
     deleted = db.session.query(Notification).filter(
         func.date(Notification.created_at) < seven_days_ago,
         Notification.notification_type == notification_type,
     ).delete(synchronize_session='fetch')
-    db.session.commit()
-    return deleted
-
-
-@statsd(namespace="dao")
-def delete_notification_to_email_reply_to_more_than_a_week_ago():
-    seven_days_ago = date.today() - timedelta(days=7)
-
-    subq = db.session.query(Notification.id).filter(
-        func.date(Notification.created_at) < seven_days_ago
-    ).subquery()
-    deleted = db.session.query(
-        NotificationEmailReplyTo
-    ).filter(
-        NotificationEmailReplyTo.notification_id.in_(subq)
-    ).delete(synchronize_session='fetch')
-    db.session.commit()
     return deleted
 
 

tests/app/celery/test_scheduled_tasks.py

@@ -126,12 +126,9 @@ def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api,
 
 
 def test_should_call_delete_email_notifications_more_than_week_in_task(notify_api, mocker):
-    mocked_delete_noti_to_reply_to = mocker.patch(
-        'app.celery.scheduled_tasks.delete_notification_to_email_reply_to_more_than_a_week_ago')
     mocked_notifications = mocker.patch(
         'app.celery.scheduled_tasks.delete_notifications_created_more_than_a_week_ago_by_type')
     delete_email_notifications_older_than_seven_days()
-    mocked_delete_noti_to_reply_to.assert_called_once_with()
     mocked_notifications.assert_called_once_with('email')
 
 

tests/app/dao/notification_dao/test_notification_dao.py

@@ -41,7 +41,6 @@ from app.dao.notifications_dao import (
     dao_update_notification,
     dao_update_notifications_for_job_to_sent_to_dvla,
     delete_notifications_created_more_than_a_week_ago_by_type,
-    delete_notification_to_email_reply_to_more_than_a_week_ago,
     get_notification_by_id,
     get_notification_for_job,
     get_notification_with_personalisation,
@@ -1027,7 +1026,6 @@ def test_should_delete_notification_to_email_reply_to_after_seven_days(
     assert len(all_notification_email_reply_to) == 10
 
     # Records before 3rd should be deleted
-    delete_notification_to_email_reply_to_more_than_a_week_ago()
     delete_notifications_created_more_than_a_week_ago_by_type(EMAIL_TYPE)
     remaining_email_notifications = Notification.query.filter_by(notification_type=EMAIL_TYPE).all()
     remaining_notification_to_email_reply_to = NotificationEmailReplyTo.query.filter_by().all()