diff --git a/app/aws/s3.py b/app/aws/s3.py index d7ffcd792..b726276c1 100644 --- a/app/aws/s3.py +++ b/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) diff --git a/app/config.py b/app/config.py index 49cbd89c0..b4a5b8aed 100644 --- a/app/config.py +++ b/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 diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4b3f5d2fb..e2b29922a 100644 --- a/app/dao/notifications_dao.py +++ b/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) diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index f5ac3d2bc..8b903daa6 100644 --- a/tests/app/aws/test_s3.py +++ b/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( diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 87c8da988..2e302476a 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/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), - ("+12028675111", "sms", True), - ("+12028675222", "sms", True), - ("2028675000", "sms", True), - ("2028675111", "sms", True), + ("+14254147755", "sms", True), + ("+14254147167", "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 diff --git a/tests/app/service/send_notification/test_send_notification.py b/tests/app/service/send_notification/test_send_notification.py index c65614e22..3ffbb8e2e 100644 --- a/tests/app/service/send_notification/test_send_notification.py +++ b/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 ): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index f2db925b7..3b97878aa 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/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"), - ("2028675111", "sms"), - ("2028675222", "sms"), + ("+14254147167", "sms"), + ("+14254147755", "sms"), ], ) def test_should_not_persist_or_send_notification_if_simulated_recipient(