diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index a274635ce..42a37215e 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -7,7 +7,10 @@ from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException from app.clients.sms import SmsClientResponseException from app.config import QueueNames from app.dao import notifications_dao -from app.dao.notifications_dao import update_notification_status_by_id +from app.dao.notifications_dao import ( + insert_notification_history_delete_notifications_by_id, + update_notification_status_by_id, +) from app.delivery import send_to_providers from app.exceptions import NotificationTechnicalFailureException from app.models import NOTIFICATION_TECHNICAL_FAILURE @@ -37,11 +40,13 @@ def deliver_sms(self, notification_id): self.retry(queue=QueueNames.RETRY, countdown=0) else: self.retry(queue=QueueNames.RETRY) + insert_notification_history_delete_notifications_by_id(notification_id) except self.MaxRetriesExceededError: message = "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. " \ "Notification has been updated to technical-failure".format(notification_id) update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) raise NotificationTechnicalFailureException(message) + insert_notification_history_delete_notifications_by_id(notification_id) @notify_celery.task(bind=True, name="deliver_email", max_retries=48, default_retry_delay=300) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d560e61eb..f602e13ff 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -269,6 +269,37 @@ def _filter_query(query, filter_dict=None): return query +@autocommit +def insert_notification_history_delete_notifications_by_id( + notification_id +): + """ + Deletes one notification after it has run successfully and moves it to the notification_history + table. + """ + input_params = { + "notification_id": notification_id + } + # Insert into NotificationHistory if the row already exists do nothing. + insert_query = """ + insert into notification_history + SELECT id, job_id, job_row_number, service_id, template_id, template_version, api_key_id, + key_type, notification_type, created_at, sent_at, sent_by, updated_at, reference, billable_units, + client_reference, international, phone_prefix, rate_multiplier, notification_status, + created_by_id, document_download_count + from NOTIFICATIONS WHERE id= :notification_id + ON CONFLICT ON CONSTRAINT notification_history_pkey + DO NOTHING + """ + delete_query = """ + DELETE FROM notifications + where id= :notification_id + """ + + db.session.execute(insert_query, input_params) + db.session.execute(delete_query, input_params) + + @autocommit def insert_notification_history_delete_notifications( notification_type, service_id, timestamp_to_delete_backwards_from, qry_limit=50000 diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 2f241bc24..f373c08d8 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -66,11 +66,11 @@ def test_should_retry_and_log_warning_if_SmsClientResponseException_for_deliver_ ) mocker.patch('app.celery.provider_tasks.deliver_sms.retry') mock_logger_warning = mocker.patch('app.celery.tasks.current_app.logger.warning') + assert sample_notification.status == 'created' deliver_sms(sample_notification.id) assert provider_tasks.deliver_sms.retry.called is True - assert sample_notification.status == 'created' assert mock_logger_warning.called @@ -81,10 +81,10 @@ def test_should_retry_and_log_exception_for_non_SmsClientResponseException_excep mocker.patch('app.celery.provider_tasks.deliver_sms.retry') mock_logger_exception = mocker.patch('app.celery.tasks.current_app.logger.exception') + assert sample_notification.status == 'created' deliver_sms(sample_notification.id) assert provider_tasks.deliver_sms.retry.called is True - assert sample_notification.status == 'created' assert mock_logger_exception.called