diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index acd2910b3..02b435990 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -60,7 +60,9 @@ def check_sms_delivery_receipt(self, message_id, notification_id, sent_at): # if status is not success or failure the client raised an exception and this method will retry if status == NOTIFICATION_DELIVERED: - sanitize_successful_notification_by_id(notification_id) + sanitize_successful_notification_by_id( + notification_id, provider_response=provider_response + ) current_app.logger.info( f"Sanitized notification {notification_id} that was successfully delivered" ) diff --git a/app/clients/cloudwatch/aws_cloudwatch.py b/app/clients/cloudwatch/aws_cloudwatch.py index 9d92d65ac..aba9d54e0 100644 --- a/app/clients/cloudwatch/aws_cloudwatch.py +++ b/app/clients/cloudwatch/aws_cloudwatch.py @@ -103,7 +103,7 @@ class AwsCloudwatchClient(Client): return "success", message["delivery"]["providerResponse"] log_group_name = ( - f"sns/{region}/{account_number}/DirectPublishToPhoneNumber/Failure" + f"sns/{region}/{account_number[4]}/DirectPublishToPhoneNumber/Failure" ) # current_app.logger.info(f"Failure log group name: {log_group_name}") all_failed_events = self._get_log(filter_pattern, log_group_name, created_at) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 9cde3d6fa..aa8ebb880 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -139,7 +139,11 @@ def update_notification_status_by_id( notification.provider_response = provider_response if not notification.sent_by and sent_by: notification.sent_by = sent_by - return _update_notification_status(notification=notification, status=status) + return _update_notification_status( + notification=notification, + status=status, + provider_response=notification.provider_response, + ) @autocommit @@ -288,24 +292,15 @@ def _filter_query(query, filter_dict=None): return query -@autocommit -def sanitize_successful_notification_by_id(notification_id): - # TODO what to do for international? - # phone_prefix = '1' - # Notification.query.filter( - # Notification.id.in_([notification_id]), - # ).update( - # {'to': phone_prefix, 'normalised_to': phone_prefix, 'status': 'delivered'} - # ) - # db.session.commit() - +def sanitize_successful_notification_by_id(notification_id, provider_response): update_query = """ - update notifications set notification_status='delivered', "to"='1', normalised_to='1' + update notifications set provider_response=:response, notification_status='delivered', "to"='1', normalised_to='1' where id=:notification_id """ - input_params = {"notification_id": notification_id} + input_params = {"notification_id": notification_id, "response": provider_response} db.session.execute(update_query, input_params) + db.session.commit() @autocommit diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 156aff36a..e854b8e8f 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -109,10 +109,16 @@ def test_should_be_able_to_sanitize_successful_notification( assert Notification.query.get(notification.id).status == "sending" with freeze_time("2000-01-02 12:00:00"): - sanitize_successful_notification_by_id(notification.id) + sanitize_successful_notification_by_id( + notification.id, provider_response="Don't know what happened" + ) assert Notification.query.get(notification.id).status == "delivered" assert Notification.query.get(notification.id).normalised_to == "1" assert Notification.query.get(notification.id).to == "1" + assert ( + Notification.query.get(notification.id).provider_response + == "Don't know what happened" + ) def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(