Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reinstate bulk sms limit #1834

Merged
merged 14 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/main/views/send.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
)
recipients = RecipientCSV(
contents,
template=template,
template_type=template.template_type,
placeholders=template.placeholders,
max_initial_rows_shown=int(request.args.get("show") or 10),
Expand Down
2 changes: 2 additions & 0 deletions app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,8 @@
"For other questions about GC Notify, use this form.","Pour toute autre question, utilisez le formulaire ci-dessous."
"We answer many questions on other GC Notify pages:","Nous répondons à de nombreuses questions sur les autres pages de Notification GC :"
"Register for a demo","S’inscrire à une démo"
"added custom content exceeds the {} character limit in 1 row","Le contenu personnalisé ajouté dépasse la limite de {} caractères pour 1 ligne"
"added custom content exceeds the {} character limit in {} rows","Le contenu personnalisé ajouté dépasse la limite de {} caractères pour {} lignes"
"Select another logo","Sélectionner un autre logo"
"Select alternate logo","Sélectionner un autre logo"
"Name of logo","Nom du logo"
Expand Down
14 changes: 14 additions & 0 deletions app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from flask_babel import _
from flask_babel import lazy_gettext as _l
from flask_login import current_user, login_required
from notifications_utils import SMS_CHAR_COUNT_LIMIT
from notifications_utils.field import Field
from notifications_utils.formatters import make_quotes_smart
from notifications_utils.letter_timings import letter_can_be_cancelled
Expand All @@ -45,6 +46,7 @@
from werkzeug.routing import RequestRedirect

from app import cache
from app.models.enum.template_types import TemplateType
from app.notify_client.organisations_api_client import organisations_client
from app.notify_client.service_api_client import service_api_client
from app.types import EmailReplyTo
Expand Down Expand Up @@ -208,6 +210,18 @@ def get_errors_for_csv(recipients, template_type):
else:
errors.append(_("enter missing data in {} rows").format(number_of_rows_with_missing_data))

if recipients.template_type == TemplateType.SMS.value and any(recipients.rows_with_combined_variable_content_too_long):
num_rows_with_combined_content_too_long = len(list(recipients.rows_with_combined_variable_content_too_long))
if num_rows_with_combined_content_too_long == 1:
errors.append(_("added custom content exceeds the {} character limit in 1 row").format(SMS_CHAR_COUNT_LIMIT))
else:
errors.append(
_("added custom content exceeds the {} character limit in {} rows").format(
SMS_CHAR_COUNT_LIMIT, num_rows_with_combined_content_too_long
)
)
# TODO Update the inline cell error messages

return errors


Expand Down
8 changes: 4 additions & 4 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ unidecode = "^1.3.6"

# PaaS
awscli-cwlogs = "^1.4.6"
notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.2.3" }
notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.2.4" }

# Pinned dependencies
rsa = "^4.1" # not directly required, pinned by Snyk to avoid a vulnerability
Expand Down
26 changes: 17 additions & 9 deletions tests/app/main/test_errors_for_csv.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,45 @@

from app.utils import get_errors_for_csv

MockRecipients = namedtuple("MockRecipients", ["rows_with_bad_recipients", "rows_with_missing_data"])
MockRecipients = namedtuple(
"MockRecipients",
["rows_with_bad_recipients", "rows_with_missing_data", "rows_with_combined_variable_content_too_long", "template_type"],
)


@pytest.mark.parametrize(
"rows_with_bad_recipients,rows_with_missing_data,template_type,expected_errors",
"rows_with_bad_recipients, rows_with_missing_data, rows_with_combined_variable_content_too_long, template_type, expected_errors",
[
([], [], "sms", []),
({2}, [], "sms", ["fix 1 phone number"]),
({2, 4, 6}, [], "sms", ["fix 3 phone numbers"]),
({1}, [], "email", ["fix 1 email address"]),
({2, 4, 6}, [], "email", ["fix 3 email addresses"]),
({2}, {3}, "sms", ["fix 1 phone number", "enter missing data in 1 row"]),
([], [], [], "sms", []),
({2}, [], [], "sms", ["fix 1 phone number"]),
({2, 4, 6}, [], [], "sms", ["fix 3 phone numbers"]),
({1}, [], [], "email", ["fix 1 email address"]),
({2, 4, 6}, [], [], "email", ["fix 3 email addresses"]),
({2}, {3}, [], "sms", ["fix 1 phone number", "enter missing data in 1 row"]),
(
{2, 4, 6, 8},
{3, 6, 9, 12},
[],
"sms",
["fix 4 phone numbers", "enter missing data in 4 rows"],
),
([], [], {2, 5, 8}, "sms", ["added custom content exceeds the 612 character limit in 3 rows"]),
],
)
def test_get_errors_for_csv(
app_,
rows_with_bad_recipients,
rows_with_missing_data,
rows_with_combined_variable_content_too_long,
template_type,
expected_errors,
):
with app_.test_request_context():
assert (
get_errors_for_csv(
MockRecipients(rows_with_bad_recipients, rows_with_missing_data),
MockRecipients(
rows_with_bad_recipients, rows_with_missing_data, rows_with_combined_variable_content_too_long, template_type
),
template_type,
)
== expected_errors
Expand Down
63 changes: 63 additions & 0 deletions tests/app/main/views/test_send.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,69 @@ def test_upload_csv_file_with_missing_columns_shows_error(
assert normalize_spaces(page.select_one("h1").text) == expected_heading


@pytest.mark.parametrize(
"file_contents, expected_error, expected_heading, num_cells_errors",
[
(
f"""
phone number, one, two, three
+16502532222, {'a' * 155}, {'a' * 155}, {'a' * 155}
+16502532222, {'a' * 613}, {'a' * 613}, {'a' * 613}
+16502532222, {'a' * 50}, {'a' * 50}, {'a' * 50}
""",
"Maximum 612 characters. Some messages may be too long due to custom content.",
"Added custom content exceeds the 612 character limit in 1 row",
3,
),
(
f"""
phone number, one, two, three
+16502532222, {'a' * 613}, {'a' * 613}, {'a' * 613}
+16502532222, {'a' * 619}, {'a' * 619}, {'a' * 619}
+16502532222, {'a' * 700}, {'a' * 700}, {'a' * 700}
""",
"Maximum 612 characters. Some messages may be too long due to custom content.",
"Added custom content exceeds the 612 character limit in 3 rows",
9,
),
],
)
def test_upload_csv_variables_too_long_shows_banner_and_inline_cell_errors(
client_request,
mocker,
mock_get_service_template_with_multiple_placeholders,
mock_s3_upload,
mock_get_users_by_service,
mock_get_service_statistics,
mock_get_template_statistics,
mock_get_job_doesnt_exist,
mock_get_jobs,
service_one,
fake_uuid,
file_contents,
expected_error,
expected_heading,
num_cells_errors,
):
mocker.patch("app.main.views.send.s3download", return_value=file_contents)
page = client_request.post(
"main.send_messages",
service_id=service_one["id"],
template_id=fake_uuid,
_data={"file": (BytesIO("".encode("utf-8")), "invalid.csv")},
_follow_redirects=True,
)
cell_errors = page.find_all("span", class_="table-field-error-label")

with client_request.session_transaction() as session:
assert "file_uploads" not in session

assert normalize_spaces(page.find(role="alert").text) == expected_heading
assert normalize_spaces(page.select_one("h1").text) == "Edit your spreadsheet"
assert all(cell.text == expected_error for cell in cell_errors)
assert len(cell_errors) == num_cells_errors


def test_upload_csv_invalid_extension(
logged_in_client,
mock_login,
Expand Down
Loading