From 4728ee20e890a9e479961968242ecf10dcfd9f8c Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Tue, 13 May 2025 15:35:31 -0700 Subject: [PATCH 1/4] reverting pr 2484 because it breaks personalization --- app/s3_client/s3_csv_client.py | 9 --------- tests/app/s3_client/test_s3_csv_client.py | 10 +--------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index 4d8f33a07..195ea3032 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -28,17 +28,8 @@ def get_csv_upload(service_id, upload_id): return get_s3_object(*get_csv_location(service_id, upload_id)) -def remove_blank_lines(filedata): - # sometimes people upload files with hundreds of blank lines at the end - data = filedata["data"] - cleaned_data = "\n".join(line for line in data.splitlines() if line.strip()) - filedata["data"] = cleaned_data - return filedata - - def s3upload(service_id, filedata): - filedata = remove_blank_lines(filedata) upload_id = str(uuid.uuid4()) bucket_name, file_location, access_key, secret_key, region = get_csv_location( service_id, upload_id diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index 2ea3c43db..dbf26ea47 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -1,6 +1,6 @@ from unittest.mock import Mock -from app.s3_client.s3_csv_client import remove_blank_lines, set_metadata_on_csv_upload +from app.s3_client.s3_csv_client import set_metadata_on_csv_upload def test_sets_metadata(client_request, mocker): @@ -21,11 +21,3 @@ def test_sets_metadata(client_request, mocker): MetadataDirective="REPLACE", ServerSideEncryption="AES256", ) - - -def test_removes_blank_lines(): - filedata = { - "data": "phone number\r\n15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n" - } - file_data = remove_blank_lines(filedata) - assert file_data == {"data": "phone number\n15555555555"} From bd884012d4ba6d27c307db9652110b4a2ec4ba26 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Wed, 14 May 2025 09:58:52 -0700 Subject: [PATCH 2/4] added a second test to preserve the \r\n formatting for line endings --- app/s3_client/s3_csv_client.py | 9 +++++++++ tests/app/s3_client/test_s3_csv_client.py | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index 195ea3032..426191aec 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -28,8 +28,17 @@ def get_csv_upload(service_id, upload_id): return get_s3_object(*get_csv_location(service_id, upload_id)) +def remove_blank_lines(filedata): + # sometimes people upload files with hundreds of blank lines at the end + data = filedata["data"] + cleaned_data = "\r\n".join(line for line in data.splitlines() if line.strip()) + filedata["data"] = cleaned_data + return filedata + + def s3upload(service_id, filedata): + filedata = remove_blank_lines(filedata) upload_id = str(uuid.uuid4()) bucket_name, file_location, access_key, secret_key, region = get_csv_location( service_id, upload_id diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index dbf26ea47..28186cec4 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -1,6 +1,6 @@ from unittest.mock import Mock -from app.s3_client.s3_csv_client import set_metadata_on_csv_upload +from app.s3_client.s3_csv_client import remove_blank_lines, set_metadata_on_csv_upload def test_sets_metadata(client_request, mocker): @@ -21,3 +21,9 @@ def test_sets_metadata(client_request, mocker): MetadataDirective="REPLACE", ServerSideEncryption="AES256", ) + + +def test_removes_blank_lines(): + filedata = { "data": "variable,phone number\r\ntest,+15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n" } + file_data = remove_blank_lines(filedata) + assert file_data == {"data": "variable,phone number\r\ntest,+15555555555"} From 3f944e244733fa85a2156e2b1e294168e37f440c Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Wed, 14 May 2025 10:07:29 -0700 Subject: [PATCH 3/4] flake8 --- tests/app/s3_client/test_s3_csv_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index 28186cec4..b2671f738 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -24,6 +24,6 @@ def test_sets_metadata(client_request, mocker): def test_removes_blank_lines(): - filedata = { "data": "variable,phone number\r\ntest,+15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n" } + filedata = {"data": "variable,phone number\r\ntest,+15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n"} file_data = remove_blank_lines(filedata) assert file_data == {"data": "variable,phone number\r\ntest,+15555555555"} From 8aa7a63f552c5f5e2296fc23e0383d387180d9b0 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Wed, 14 May 2025 10:08:02 -0700 Subject: [PATCH 4/4] reformatted --- app/main/views/send.py | 36 ++++++++++++----------- app/main/views/tour.py | 12 ++++---- tests/app/main/views/test_send.py | 6 ++-- tests/app/s3_client/test_s3_csv_client.py | 4 ++- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 61e0c0083..b1b393a90 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -566,18 +566,18 @@ def _check_messages(service_id, template_id, upload_id, preview_row, **kwargs): "url": url_for( "main.send_one_off", service_id=service_id, template_id=template.id ), - "text": "Back to message personalization" + "text": "Back to message personalization", }, - "html": "Back to message personalization" + "html": "Back to message personalization", } back_link_from_preview = { "href": { "url": url_for( "main.send_one_off", service_id=service_id, template_id=template.id ), - "text": "Back to message personalization" + "text": "Back to message personalization", }, - "html": "Back to message personalization" + "html": "Back to message personalization", } choose_time_form = None else: @@ -586,9 +586,9 @@ def _check_messages(service_id, template_id, upload_id, preview_row, **kwargs): "url": url_for( "main.send_messages", service_id=service_id, template_id=template.id ), - "text": "Back to upload a file" + "text": "Back to upload a file", }, - "html": "Back to upload a file" + "html": "Back to upload a file", } back_link_from_preview = { "href": { @@ -598,9 +598,9 @@ def _check_messages(service_id, template_id, upload_id, preview_row, **kwargs): template_id=template.id, upload_id=upload_id, ), - "text": "Back to check messages" + "text": "Back to check messages", }, - "html": "Back to check messages" + "html": "Back to check messages", } choose_time_form = ChooseTimeForm() @@ -786,9 +786,9 @@ def get_back_link( service_id=service_id, template_id=template.id, ), - "text": "Back to select delivery time" + "text": "Back to select delivery time", }, - "html": "Back to select delivery time" + "html": "Back to select delivery time", } if step_index == 0: @@ -799,9 +799,9 @@ def get_back_link( ".choose_template", service_id=service_id, ), - "text": "Back to all templates" + "text": "Back to all templates", }, - "html": "Back to all templates" + "html": "Back to all templates", } else: return { @@ -811,14 +811,16 @@ def get_back_link( service_id=service_id, template_id=template.id, ), - "text": "Back to confirm your template" + "text": "Back to confirm your template", }, - "html": "Back to confirm your template" + "html": "Back to confirm your template", } # fallback for other steps back_to_text = ( - "Back to select recipients" if step_index == 1 else "Back to message personalization" + "Back to select recipients" + if step_index == 1 + else "Back to message personalization" ) return { @@ -829,9 +831,9 @@ def get_back_link( template_id=template.id, step_index=step_index - 1, ), - "text": back_to_text + "text": back_to_text, }, - "html": back_to_text + "html": back_to_text, } diff --git a/app/main/views/tour.py b/app/main/views/tour.py index e253167c8..d42ff1edf 100644 --- a/app/main/views/tour.py +++ b/app/main/views/tour.py @@ -142,13 +142,11 @@ def _get_tour_step_back_link(service_id, template_id, step_index): return { "href": { "url": url_for( - 'main.begin_tour', - service_id=service_id, - template_id=template_id + "main.begin_tour", service_id=service_id, template_id=template_id ), - "text": "Back to tour start" + "text": "Back to tour start", }, - "html": "Back to tour start" + "html": "Back to tour start", } else: return { @@ -207,9 +205,9 @@ def check_tour_notification(service_id, template_id): template_id=template_id, step_index=len(placeholders), ), - "text": "Back to previous step" + "text": "Back to previous step", }, - "html": "Back to previous step" + "html": "Back to previous step", } template.values = get_recipient_and_placeholders_from_session( diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index a18909070..11104325a 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1451,7 +1451,7 @@ def test_send_one_off_offers_link_to_upload( assert back_link.text.strip() in { "Back to all templates", - "Back to confirm your template" + "Back to confirm your template", } assert link.text.strip() == "Upload a list of phone numbers" @@ -2288,7 +2288,9 @@ def test_check_messages_back_link( actual_href = page.find_all("a", {"class": "usa-back-link"})[0]["href"] expected_href = expected_url(service_id=SERVICE_ONE_ID, template_id=fake_uuid) - assert actual_href != "#", "Back link href fell back to '#' — missing correct back_link in view" + assert ( + actual_href != "#" + ), "Back link href fell back to '#' — missing correct back_link in view" assert actual_href == expected_href diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index b2671f738..f5a89bee4 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -24,6 +24,8 @@ def test_sets_metadata(client_request, mocker): def test_removes_blank_lines(): - filedata = {"data": "variable,phone number\r\ntest,+15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n"} + filedata = { + "data": "variable,phone number\r\ntest,+15555555555\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n" + } file_data = remove_blank_lines(filedata) assert file_data == {"data": "variable,phone number\r\ntest,+15555555555"}