Skip to content

Commit

Permalink
Handle invalid numbers when sending to emergency contact lists
Browse files Browse the repository at this point in the history
legacy emergency contact lists are known to contain invalid numbers. We do not want this to cause sending to the entire list to fail, so this will bypass validation against libphonenumber entirely for emergency contact lists while ensuring new uploads of all CSVs are validated properly
  • Loading branch information
rparke committed Jan 31, 2025
1 parent 7d756c5 commit 5c45eb0
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
7 changes: 1 addition & 6 deletions app/main/views/send.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,6 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
except HTTPError as e:
if e.status_code != 404:
raise

contents = s3download(service_id, upload_id)

template = current_service.get_template_with_user_permission_or_403(
Expand All @@ -625,7 +624,6 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
elif template.template_type == "sms":
template.sender = get_sms_sender_from_session()
template.show_sender = bool(template.sender)

recipients = RecipientCSV(
contents,
template=template,
Expand All @@ -642,8 +640,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
allow_international_sms=current_service.has_permission("international_sms"),
allow_sms_to_uk_landline=current_service.has_permission("sms_to_uk_landlines"),
allow_international_letters=current_service.has_permission("international_letters"),
should_validate_phone_number=False,
)

if request.args.get("from_test"):
# only happens if generating a letter preview test
back_link = url_for("main.send_one_off", service_id=service_id, template_id=template.id)
Expand All @@ -659,9 +657,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
template.values = recipients[preview_row - 2].recipient_and_personalisation
elif preview_row > 2:
abort(404)

original_file_name = get_csv_metadata(service_id, upload_id).get("original_file_name", "")

return {
"recipients": recipients,
"template": template,
Expand Down Expand Up @@ -699,7 +695,6 @@ def _check_messages(service_id, template_id, upload_id, preview_row):
def check_messages(service_id, template_id, upload_id, row_index=2):
data = _check_messages(service_id, template_id, upload_id, row_index)
data["allowed_file_extensions"] = Spreadsheet.ALLOWED_FILE_EXTENSIONS

if (
data["recipients"].too_many_rows
or not data["count_of_recipients"]
Expand Down
2 changes: 1 addition & 1 deletion tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ def inbound_sms_json():
(4, 5, "33(0)1 12345678"), # France
(5, 7, "1-202-555-0104"), # USA in one format
(6, 9, "+12025550104"), # USA in another format
(7, 9, "+68212345"), # Cook Islands
(7, 9, "+68223001"), # Cook Islands
)
],
}
Expand Down
35 changes: 35 additions & 0 deletions tests/app/main/views/test_send.py
Original file line number Diff line number Diff line change
Expand Up @@ -4519,3 +4519,38 @@ def test_send_to_myself_404s_for_letter(
template_id=fake_uuid,
_expected_status=404,
)


def test_can_send_from_emergency_contact_list_with_error_rows(
client_request,
mock_get_service_template,
mock_s3_download,
service_one,
mock_get_job_doesnt_exist,
fake_uuid,
mocker,
):
service_one["restricted"] = False
mocker.patch(
"app.main.views.send.s3download",
return_value="""
phone number
+1 800 555 5555
""",
)
mocker.patch(
"app.main.views.send.get_csv_metadata",
return_value={"original_file_name": "example.csv"},
)
mocker.patch("app.main.views.send.job_api_client.has_sent_previously", return_value=False)
mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
page = client_request.get(
"main.check_messages",
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
upload_id=fake_uuid,
_test_page_title=False,
_follow_redirects=True,
)
assert not page.select_one(".banner-dangerous")
assert page.select_one(".govuk-button").text.strip() == "Send 1 text message"

0 comments on commit 5c45eb0

Please sign in to comment.