Merge pull request #1485 from GSA/notify-api-1465

start writing message ids to the notifications table

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)

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

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}"

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)
 

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)