diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 1e6c7e96f..a75a68c96 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -128,6 +128,8 @@ def deliver_sms(self, notification_id): ) # Code branches off to send_to_providers.py message_id = send_to_providers.send_sms_to_provider(notification) + + # DEPRECATED # We have to put it in UTC. For other timezones, the delay # will be ignored and it will fire immediately (although this probably only affects developer testing) my_eta = utc_now() + timedelta(seconds=DELIVERY_RECEIPT_DELAY_IN_SECONDS) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 8371aaa85..052242644 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -110,6 +110,16 @@ def _update_notification_status( return notification +def update_notification_message_id(notification_id, message_id): + stmt = ( + update(Notification) + .where(Notification.id == notification_id) + .values(message_id=message_id) + ) + db.session.execute(stmt) + db.session.commit() + + @autocommit def update_notification_status_by_id( notification_id, status, sent_by=None, provider_response=None, carrier=None diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 07763823f..e41062b41 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -16,7 +16,10 @@ from app.aws.s3 import get_personalisation_from_s3, get_phone_number_from_s3 from app.celery.test_key_tasks import send_email_response, send_sms_response from app.dao.email_branding_dao import dao_get_email_branding_by_id -from app.dao.notifications_dao import dao_update_notification +from app.dao.notifications_dao import ( + dao_update_notification, + update_notification_message_id, +) from app.dao.provider_details_dao import get_provider_details_by_notification_type from app.dao.service_sms_sender_dao import dao_get_sms_senders_by_service_id from app.enums import BrandType, KeyType, NotificationStatus, NotificationType @@ -117,6 +120,7 @@ def send_sms_to_provider(notification): message_id = provider.send_sms(**send_sms_kwargs) current_app.logger.info(f"got message_id {message_id}") + update_notification_message_id(notification.id, message_id) except Exception as e: n = notification msg = f"FAILED send to sms, job_id: {n.job_id} row_number {n.job_row_number} message_id {message_id}" diff --git a/app/models.py b/app/models.py index 6b008f64b..ec6eac335 100644 --- a/app/models.py +++ b/app/models.py @@ -1532,6 +1532,7 @@ class Notification(db.Model): provider_response = db.Column(db.Text, nullable=True) carrier = db.Column(db.Text, nullable=True) + message_id = db.Column(db.Text, nullable=True) # queue_name = db.Column(db.Text, nullable=True) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 20b0f7186..7a6259551 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -93,6 +93,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") mock_s3.return_value = "2028675309" + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_personalisation = mocker.patch( "app.delivery.send_to_providers.get_personalisation_from_s3" ) @@ -233,6 +234,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") mock_s3.return_value = "2028675309" + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_s3_p = mocker.patch( "app.delivery.send_to_providers.get_personalisation_from_s3" ) @@ -327,6 +329,7 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): # ī, grapes, tabs, zero width space and ellipsis are not # ó isn't in GSM, but it is in the welsh alphabet so will still be sent + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] @@ -365,6 +368,7 @@ def test_send_sms_should_use_service_sms_sender( mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch("app.aws_sns_client.send_sms") + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") sms_sender = create_service_sms_sender( service=sample_service, sms_sender="123456", is_default=False @@ -405,6 +409,8 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c ) mocker.patch("app.aws_ses_client.send_email") mocker.patch("app.delivery.send_to_providers.send_email_response") + + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_phone = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") mock_phone.return_value = "15555555555" @@ -627,6 +633,10 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_ ): mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) @@ -637,6 +647,11 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_ key_type=key_type, reply_to_text="testing", ) + + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) mocker.patch("app.aws_sns_client.send_sms") mocker.patch( "app.delivery.send_to_providers.send_sms_response", @@ -647,6 +662,8 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_ sample_template.service.research_mode = True mock_phone = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") + + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_phone.return_value = "15555555555" mock_personalisation = mocker.patch( @@ -670,6 +687,8 @@ def test_should_set_notification_billable_units_and_reduces_provider_priority_if assert sample_notification.sent_by is None mock_phone = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") + + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_phone.return_value = "15555555555" mock_personalisation = mocker.patch( @@ -705,8 +724,14 @@ def test_should_send_sms_to_international_providers( ) mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") + + mocker.patch("app.delivery.send_to_providers.update_notification_message_id") mock_s3.return_value = "601117224412" + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) mock_personalisation = mocker.patch( "app.delivery.send_to_providers.get_personalisation_from_s3" ) @@ -744,6 +769,11 @@ def test_should_handle_sms_sender_and_prefix_message( mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch("app.aws_sns_client.send_sms") + + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) service = create_service_with_defined_sms_sender( sms_sender_value=sms_sender, prefix_sms=prefix_sms ) @@ -803,6 +833,11 @@ def test_send_sms_to_provider_should_use_normalised_to(mocker, client, sample_te mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) + + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) send_mock = mocker.patch("app.aws_sns_client.send_sms") notification = create_notification( template=sample_template, @@ -866,6 +901,11 @@ def test_send_sms_to_provider_should_return_template_if_found_in_redis( mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) + + mocker.patch( + "app.delivery.send_to_providers.update_notification_message_id", + return_value=None, + ) from app.schemas import service_schema, template_schema service_dict = service_schema.dump(sample_template.service)