From 646c359348c29cf3b193ce9c5b919d73e370505d Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:11:29 -0400 Subject: [PATCH] Add support to display FR column headings in reports CSVs (#1681) * Add support to display FR column headings in reports csvs - Converts email_address to Email address - Converts phone_number to Phone number * Translate template_type and status values to FR * Fix incorrect translation, fix failing tests * Add proper French translations - Fetch translations using fr.csv * Add UTF-8 BOM to CSV for compatability with MS excel * Fix tests that broke when adding the UTF-8 BOM to CSVs * Remove 'name' from the translation mapping dict --- app/translations/csv/fr.csv | 7 +- app/utils.py | 67 ++++++++++++---- scripts/test-translations.py | 2 +- tests/app/test_utils.py | 150 +++++++++++++++++++---------------- 4 files changed, 139 insertions(+), 87 deletions(-) diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 2f972a46b2..f93c90bdec 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1760,4 +1760,9 @@ "Message limits reset each night at 7pm Eastern Time","Les limites d’envoi sont réinitialisées chaque soir à 19 h, heure de l’Est" "Maximum 612 characters. Some messages may be too long due to custom content.","612 caractères au maximum. Certains messages peuvent être trop longs en raison de leur contenu personnalisé." "Too many characters","Limite de caractère atteinte" -"New features","Nouvelles fonctionnalités" \ No newline at end of file +"New features","Nouvelles fonctionnalités" +"Row number","Numéro de ligne" +"Type","Type" +"Sent by email","Envoyé par courriel" +"Sent Time","Heure d’envoi" +"Job","Tâche" \ No newline at end of file diff --git a/app/utils.py b/app/utils.py index e907ded6fc..5994761a6b 100644 --- a/app/utils.py +++ b/app/utils.py @@ -59,6 +59,21 @@ "virus-scan-failed", "validation-failed", ] +CSV_COLUMN_HEADER_MAPPINGS = [ + {"original_heading": "email address", "en": "Email address", "fr": _l("Email address")}, + {"original_heading": "email_address", "en": "Email address", "fr": _l("Email address")}, + {"original_heading": "phone number", "en": "Phone number", "fr": _l("Phone number")}, + {"original_heading": "phone_number", "en": "Phone number", "fr": _l("Phone number")}, + {"original_heading": "Row number", "en": "Row number", "fr": _l("Row number")}, + {"original_heading": "Recipient", "en": "Recipient", "fr": _l("Recipient")}, + {"original_heading": "Template", "en": "Template", "fr": _l("Template")}, + {"original_heading": "Type", "en": "Type", "fr": _l("Type")}, + {"original_heading": "Sent by", "en": "Sent by", "fr": _l("Sent by")}, + {"original_heading": "Sent by email", "en": "Sent by email", "fr": _l("Sent by email")}, + {"original_heading": "Job", "en": "Job", "fr": _l("Job")}, + {"original_heading": "Status", "en": "Status", "fr": _l("Status")}, + {"original_heading": "Time", "en": "Sent Time", "fr": _l("Sent Time")}, +] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES with open("{}/email_domains.txt".format(os.path.dirname(os.path.realpath(__file__)))) as email_domains: @@ -196,10 +211,23 @@ def get_errors_for_csv(recipients, template_type): return errors +def localize_and_format_csv_headers(column_headers: list) -> list: + from app import get_current_locale + + lang = get_current_locale(current_app) + localized_headers = [] + for header in column_headers: + localized_header = [mapped for mapped in CSV_COLUMN_HEADER_MAPPINGS if mapped["original_heading"] == header] + localized_headers.append(str(localized_header[0][lang]) if len(localized_header) >= 1 else header) + return localized_headers + + def generate_notifications_csv(**kwargs): - from app import notification_api_client + from app import get_current_locale, notification_api_client from app.s3_client.s3_csv_client import s3download + lang = get_current_locale(current_app) + if "page" not in kwargs: kwargs["page"] = 1 @@ -210,19 +238,24 @@ def generate_notifications_csv(**kwargs): template_type=kwargs["template_type"], ) original_column_headers = original_upload.column_headers - fieldnames = ["Row number"] + original_column_headers + ["Template", "Type", "Job", "Status", "Time"] + fieldnames = localize_and_format_csv_headers( + ["Row number"] + original_column_headers + ["Template", "Type", "Job", "Status", "Time"] + ) else: - fieldnames = [ - "Recipient", - "Template", - "Type", - "Sent by", - "Sent by email", - "Job", - "Status", - "Time", - ] - + fieldnames = localize_and_format_csv_headers( + [ + "Recipient", + "Template", + "Type", + "Sent by", + "Sent by email", + "Job", + "Status", + "Time", + ] + ) + # Add encoded Byte Order Mark to the csv so MS Excel treats it as UTF-8 and properly renders accented FR characters. + yield "\uFEFF".encode("utf-8") yield ",".join(fieldnames) + "\n" while kwargs["page"]: @@ -236,9 +269,9 @@ def generate_notifications_csv(**kwargs): + [original_upload[notification["row_number"] - 1].get(header).data for header in original_column_headers] + [ notification["template_name"], - notification["template_type"], + notification["template_type"] if lang == "en" else _l(notification["template_type"]), notification["job_name"], - notification["status"], + notification["status"] if lang == "en" else _l(notification["status"]), notification["created_at"], ] ) @@ -246,11 +279,11 @@ def generate_notifications_csv(**kwargs): values = [ notification["recipient"], notification["template_name"], - notification["template_type"], + notification["template_type"] if lang == "en" else _l(notification["template_type"]), notification["created_by_name"] or "", notification["created_by_email_address"] or "", notification["job_name"] or "", - notification["status"], + notification["status"] if lang == "en" else _l(notification["status"]), notification["created_at"], ] yield Spreadsheet.from_rows([map(str, values)]).as_csv_data diff --git a/scripts/test-translations.py b/scripts/test-translations.py index 36f84e283f..dbbed2f461 100644 --- a/scripts/test-translations.py +++ b/scripts/test-translations.py @@ -25,7 +25,7 @@ ] ) -keys_wrongly_detected = set(["header", "Send {}", "Not a valid phone number"]) +keys_wrongly_detected = set(["header", "Send {}", "Not a valid phone number", "template_type", "status"]) def csv_to_dict(filename): diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index ddc95cd992..5931f6a198 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -180,14 +180,14 @@ def test_spreadsheet_checks_for_bad_arguments(args, kwargs): ( None, [ - "Recipient,Template,Type,Sent by,Sent by email,Job,Status,Time\n", + "Recipient,Template,Type,Sent by,Sent by email,Job,Status,Sent Time\n", "foo@bar.com,foo,sms,,sender@email.canada.ca,,Delivered,1943-04-19 12:00:00\r\n", ], ), ( "Anne Example", [ - "Recipient,Template,Type,Sent by,Sent by email,Job,Status,Time\n", + "Recipient,Template,Type,Sent by,Sent by email,Job,Status,Sent Time\n", "foo@bar.com,foo,sms,Anne Example,sender@email.canada.ca,,Delivered,1943-04-19 12:00:00\r\n", ], ), @@ -199,16 +199,18 @@ def test_generate_notifications_csv_without_job( created_by_name, expected_content, ): - mocker.patch( - "app.notification_api_client.get_notifications_for_service", - side_effect=_get_notifications_csv( - created_by_name=created_by_name, - created_by_email_address="sender@email.canada.ca", - job_id=None, - job_name=None, - ), - ) - assert list(generate_notifications_csv(service_id=fake_uuid)) == expected_content + with app_.test_request_context(): + mocker.patch.dict("app.current_app.config", values={"LANGUAGES": ["en", "fr"]}) + mocker.patch( + "app.notification_api_client.get_notifications_for_service", + side_effect=_get_notifications_csv( + created_by_name=created_by_name, + created_by_email_address="sender@email.canada.ca", + job_id=None, + job_name=None, + ), + ) + assert list(generate_notifications_csv(service_id=fake_uuid))[1::] == expected_content @pytest.mark.parametrize( @@ -219,7 +221,7 @@ def test_generate_notifications_csv_without_job( phone_number 07700900123 """, - ["Row number", "phone_number", "Template", "Type", "Job", "Status", "Time"], + ["Row number", "Phone number", "Template", "Type", "Job", "Status", "Sent Time"], [ "1", "07700900123", @@ -234,10 +236,10 @@ def test_generate_notifications_csv_without_job( """ phone_number, a, b, c 07700900123, 🐜,🐝,🦀 - """, + """, [ "Row number", - "phone_number", + "Phone number", "a", "b", "c", @@ -245,7 +247,7 @@ def test_generate_notifications_csv_without_job( "Type", "Job", "Status", - "Time", + "Sent Time", ], [ "1", @@ -267,7 +269,7 @@ def test_generate_notifications_csv_without_job( """, [ "Row number", - "phone_number", + "Phone number", "a", "b", "c", @@ -275,7 +277,7 @@ def test_generate_notifications_csv_without_job( "Type", "Job", "Status", - "Time", + "Sent Time", ], [ "1", @@ -300,23 +302,29 @@ def test_generate_notifications_csv_returns_correct_csv_file( expected_column_headers, expected_1st_row, ): - mocker.patch( - "app.s3_client.s3_csv_client.s3download", - return_value=original_file_contents, - ) - csv_content = generate_notifications_csv(service_id="1234", job_id=fake_uuid, template_type="sms") - csv_file = DictReader(StringIO("\n".join(csv_content))) - assert csv_file.fieldnames == expected_column_headers - assert next(csv_file) == dict(zip(expected_column_headers, expected_1st_row)) + with app_.test_request_context(): + mocker.patch.dict("app.current_app.config", values={"LANGUAGES": ["en", "fr"]}) + mocker.patch( + "app.s3_client.s3_csv_client.s3download", + return_value=original_file_contents, + ) + # Remove encoded BOM bytes before parsing + csv_content = list(generate_notifications_csv(service_id="1234", job_id=fake_uuid, template_type="sms"))[1::] + csv_file = DictReader(StringIO("\n".join(csv_content))) + assert csv_file.fieldnames == expected_column_headers + assert next(csv_file) == dict(zip(expected_column_headers, expected_1st_row)) def test_generate_notifications_csv_only_calls_once_if_no_next_link( app_, + mocker, _get_notifications_csv_mock, ): - list(generate_notifications_csv(service_id="1234")) + with app_.test_request_context(): + mocker.patch.dict("app.current_app.config", values={"LANGUAGES": ["en", "fr"]}) + list(generate_notifications_csv(service_id="1234")) - assert _get_notifications_csv_mock.call_count == 1 + assert _get_notifications_csv_mock.call_count == 1 @pytest.mark.parametrize("job_id", ["some", None]) @@ -325,49 +333,55 @@ def test_generate_notifications_csv_calls_twice_if_next_link( mocker, job_id, ): - mocker.patch( - "app.s3_client.s3_csv_client.s3download", - return_value=""" - phone_number - 07700900000 - 07700900001 - 07700900002 - 07700900003 - 07700900004 - 07700900005 - 07700900006 - 07700900007 - 07700900008 - 07700900009 - """, - ) + with app_.test_request_context(): + mocker.patch.dict("app.current_app.config", values={"LANGUAGES": ["en", "fr"]}) + + mocker.patch( + "app.s3_client.s3_csv_client.s3download", + return_value=""" + phone_number + 07700900000 + 07700900001 + 07700900002 + 07700900003 + 07700900004 + 07700900005 + 07700900006 + 07700900007 + 07700900008 + 07700900009 + """, + ) - service_id = "1234" - response_with_links = _get_notifications_csv(rows=7, with_links=True) - response_with_no_links = _get_notifications_csv(rows=3, row_number=8, with_links=False) + service_id = "1234" + response_with_links = _get_notifications_csv(rows=7, with_links=True) + response_with_no_links = _get_notifications_csv(rows=3, row_number=8, with_links=False) - mock_get_notifications = mocker.patch( - "app.notification_api_client.get_notifications_for_service", - side_effect=[ - response_with_links(service_id), - response_with_no_links(service_id), - ], - ) + mock_get_notifications = mocker.patch( + "app.notification_api_client.get_notifications_for_service", + side_effect=[ + response_with_links(service_id), + response_with_no_links(service_id), + ], + ) - csv_content = generate_notifications_csv( - service_id=service_id, - job_id=job_id or fake_uuid, - template_type="sms", - ) - csv = list(DictReader(StringIO("\n".join(csv_content)))) - - assert len(csv) == 10 - assert csv[0]["phone_number"] == "07700900000" - assert csv[9]["phone_number"] == "07700900009" - assert mock_get_notifications.call_count == 2 - # mock_calls[0][2] is the kwargs from first call - assert mock_get_notifications.mock_calls[0][2]["page"] == 1 - assert mock_get_notifications.mock_calls[1][2]["page"] == 2 + # Remove encoded BOM bytes before parsing + csv_content = list( + generate_notifications_csv( + service_id=service_id, + job_id=job_id or fake_uuid, + template_type="sms", + ) + )[1::] + csv = list(DictReader(StringIO("\n".join(csv_content)))) + + assert len(csv) == 10 + assert csv[0]["Phone number"] == "07700900000" + assert csv[9]["Phone number"] == "07700900009" + assert mock_get_notifications.call_count == 2 + # mock_calls[0][2] is the kwargs from first call + assert mock_get_notifications.mock_calls[0][2]["page"] == 1 + assert mock_get_notifications.mock_calls[1][2]["page"] == 2 def test_get_cdn_domain_on_localhost(client, mocker):