Skip to content

Commit

Permalink
Add support to display FR column headings in reports CSVs (#1681)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
whabanks authored Oct 4, 2023
1 parent d1829e6 commit 646c359
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 87 deletions.
7 changes: 6 additions & 1 deletion app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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"
"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"
67 changes: 50 additions & 17 deletions app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand All @@ -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"]:
Expand All @@ -236,21 +269,21 @@ 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"],
]
)
else:
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
Expand Down
2 changes: 1 addition & 1 deletion scripts/test-translations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
150 changes: 82 additions & 68 deletions tests/app/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"[email protected],foo,sms,,[email protected],,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",
"[email protected],foo,sms,Anne Example,[email protected],,Delivered,1943-04-19 12:00:00\r\n",
],
),
Expand All @@ -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="[email protected]",
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="[email protected]",
job_id=None,
job_name=None,
),
)
assert list(generate_notifications_csv(service_id=fake_uuid))[1::] == expected_content


@pytest.mark.parametrize(
Expand All @@ -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",
Expand All @@ -234,18 +236,18 @@ def test_generate_notifications_csv_without_job(
"""
phone_number, a, b, c
07700900123, 🐜,🐝,🦀
""",
""",
[
"Row number",
"phone_number",
"Phone number",
"a",
"b",
"c",
"Template",
"Type",
"Job",
"Status",
"Time",
"Sent Time",
],
[
"1",
Expand All @@ -267,15 +269,15 @@ def test_generate_notifications_csv_without_job(
""",
[
"Row number",
"phone_number",
"Phone number",
"a",
"b",
"c",
"Template",
"Type",
"Job",
"Status",
"Time",
"Sent Time",
],
[
"1",
Expand All @@ -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])
Expand All @@ -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):
Expand Down

0 comments on commit 646c359

Please sign in to comment.