merge from main again

app/aws/s3.py

@@ -105,24 +105,23 @@ def extract_phones(job):
     current_app.logger.info(f"HEADERS {first_row}")
     phone_index = 0
     for item in first_row:
-        if item.lower() == "phone number":
+        # Note: may contain a BOM and look like \ufeffphone number
+        if "phone number" in item.lower():
             break
         phone_index = phone_index + 1
+
     phones = {}
     job_row = 0
     for row in job:
         row = row.split(",")
-        # TODO WHY ARE WE CALCULATING PHONE INDEX IN THE LOOP?
-        phone_index = 0
-        for item in first_row:
-            if item.lower() == "phone number":
-                break
-            phone_index = phone_index + 1
         current_app.logger.info(f"PHONE INDEX IS NOW {phone_index}")
         current_app.logger.info(f"LENGTH OF ROW IS {len(row)}")
         if phone_index >= len(row):
             phones[job_row] = "Error: can't retrieve phone number"
-            current_app.logger.error("Corrupt csv file, missing columns job_id {job_id} service_id {service_id}")
+            current_app.logger.error(
+                "Corrupt csv file, missing columns or possibly a byte order mark in the file"
+            )
+
         else:
             my_phone = row[phone_index]
             my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone)

app/config.py

@@ -283,7 +283,8 @@ class Config(object):
         "simulate-delivered-2@notifications.service.gov.uk",
         "simulate-delivered-3@notifications.service.gov.uk",
     )
-    SIMULATED_SMS_NUMBERS = ("+12028675000", "+12028675111", "+12028675222")
+    # 7755 is success, 7167 is failure
+    SIMULATED_SMS_NUMBERS = ("+14254147755", "+14254147167")
 
     FREE_SMS_TIER_FRAGMENT_COUNT = 250000
 

app/dao/notifications_dao.py

@@ -108,6 +108,7 @@ def _update_notification_status(
         current_status=notification.status, status=status
     )
     notification.status = status
+    notification.sent_at = datetime.utcnow()
     if provider_response:
         notification.provider_response = provider_response
     if carrier:
@@ -326,13 +327,15 @@ def _filter_query(query, filter_dict=None):
 def sanitize_successful_notification_by_id(notification_id, carrier, provider_response):
     update_query = """
     update notifications set provider_response=:response, carrier=:carrier,
-    notification_status='delivered', "to"='1', normalised_to='1'
+    notification_status='delivered', sent_at=:sent_at, "to"='1', normalised_to='1'
     where id=:notification_id
     """
+
     input_params = {
         "notification_id": notification_id,
         "carrier": carrier,
         "response": provider_response,
+        "sent_at": datetime.utcnow(),
     }
 
     db.session.execute(update_query, input_params)

tests/app/aws/test_s3.py

@@ -68,6 +68,13 @@ def test_get_s3_file_makes_correct_call(notify_api, mocker):
             0,
             "15553333333",
         ),
+        (
+            # simulate file saved with utf8withbom
+            "\\ufeffPHONE NUMBER,Name\r\n5555555550,T 1\r\n5555555551,T 5,3/31/2024\r\n5555555552,T 2",
+            "eee",
+            2,
+            "5555555552",
+        ),
     ],
 )
 def test_get_phone_number_from_s3(

tests/app/notifications/test_process_notification.py

@@ -254,11 +254,8 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(
 @pytest.mark.parametrize(
     "to_address, notification_type, expected",
     [
-        ("+12028675000", "sms", True),
+        ("+14254147755", "sms", True),
-        ("+12028675111", "sms", True),
+        ("+14254147167", "sms", True),
-        ("+12028675222", "sms", True),
-        ("2028675000", "sms", True),
-        ("2028675111", "sms", True),
         ("simulate-delivered@notifications.service.gov.uk", "email", True),
         ("simulate-delivered-2@notifications.service.gov.uk", "email", True),
         ("simulate-delivered-3@notifications.service.gov.uk", "email", True),
@@ -275,7 +272,7 @@ def test_simulated_recipient(notify_api, to_address, notification_type, expected
         'simulate-delivered-2@notifications.service.gov.uk',
         'simulate-delivered-2@notifications.service.gov.uk'
     )
-    SIMULATED_SMS_NUMBERS = ('+12028675000', '+12028675111', '+12028675222')
+    SIMULATED_SMS_NUMBERS = ("+14254147755", "+14254147167")
     """
     formatted_address = None
 

tests/app/service/send_notification/test_send_notification.py

@@ -881,7 +881,7 @@ def test_should_not_persist_notification_or_send_email_if_simulated_email(
     assert Notification.query.count() == 0
 
 
-@pytest.mark.parametrize("to_sms", ["2028675000", "2028675111", "+12028675222"])
+@pytest.mark.parametrize("to_sms", ["+14254147755", "+14254147167"])
 def test_should_not_persist_notification_or_send_sms_if_simulated_number(
     client, to_sms, sample_template, mocker
 ):

tests/app/v2/notifications/test_post_notifications.py

@@ -532,9 +532,8 @@ def test_post_email_notification_returns_201(
         ("simulate-delivered@notifications.service.gov.uk", EMAIL_TYPE),
         ("simulate-delivered-2@notifications.service.gov.uk", EMAIL_TYPE),
         ("simulate-delivered-3@notifications.service.gov.uk", EMAIL_TYPE),
-        ("2028675000", "sms"),
+        ("+14254147167", "sms"),
-        ("2028675111", "sms"),
+        ("+14254147755", "sms"),
-        ("2028675222", "sms"),
     ],
 )
 def test_should_not_persist_or_send_notification_if_simulated_recipient(