diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 01d826ba6..2b3ddee32 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -11,7 +11,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 ( @@ -40,6 +43,10 @@ def check_sms_delivery_receipt(self, message_id, notification_id, sent_at): update_notification_status_by_id(notification_id, status, provider_response=provider_response) current_app.logger.info(f"Updated notification {notification_id} with response '{provider_response}'") + if status == NOTIFICATION_SENT: + insert_notification_history_delete_notifications_by_id(notification_id) + current_app.logger.info(f"Archived notification {notification_id} that was successfully sent") + @notify_celery.task(bind=True, name="deliver_sms", max_retries=48, default_retry_delay=300) def deliver_sms(self, notification_id): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index ae8405440..4f8bdf459 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -271,6 +271,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 f920cb5d8..84382ef42 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -67,11 +67,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 @@ -82,10 +82,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