Skip to content

Commit

Permalink
Fix/handle api exception bulk send (#2022)
Browse files Browse the repository at this point in the history
* fix(start_job): add exception handling in case API throws an annual limits error at this point; update the expected error message strings

* fix(check.html): don't render the template preview in the case of an API exception

* chore: add tests

* fix: pass time_to_reset into the view
  • Loading branch information
andrewleith authored Dec 18, 2024
1 parent 7863163 commit 2454078
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 5 deletions.
16 changes: 12 additions & 4 deletions app/main/views/send.py
Original file line number Diff line number Diff line change
Expand Up @@ -888,8 +888,15 @@ def check_notification_preview(service_id, template_id, filetype):
@main.route("/services/<service_id>/start-job/<upload_id>", methods=["POST"])
@user_has_permissions("send_messages", restrict_admin_usage=True)
def start_job(service_id, upload_id):
job_api_client.create_job(upload_id, service_id, scheduled_for=request.form.get("scheduled_for", ""))

try:
job_api_client.create_job(upload_id, service_id, scheduled_for=request.form.get("scheduled_for", ""))
except HTTPError as exception:
return render_template(
"views/notifications/check.html",
time_to_reset=get_limit_reset_time_et(),
**(get_template_error_dict(exception) if exception else {}),
template=None,
)
session.pop("sender_id", None)

return redirect(
Expand Down Expand Up @@ -1107,11 +1114,12 @@ def get_template_error_dict(exception):
error = "too-many-sms-messages"
elif "Content for template has a character count greater than the limit of" in exception.message:
error = "message-too-long"
elif "Exceeded annual email sending limit" in exception.message:
elif "Exceeded annual email sending" in exception.message:
error = "too-many-email-annual"
elif "Exceeded annual SMS sending limit" in exception.message:
elif "Exceeded annual SMS sending" in exception.message:
error = "too-many-sms-annual"
else:
current_app.logger.error("Unhandled exception from API: {}".format(exception))
raise exception

return {
Expand Down
4 changes: 3 additions & 1 deletion app/templates/views/notifications/check.html
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ <h2 class="heading-medium">{{_('You cannot send this email message today') }}</h
) }}
{% endif %}

{{ template|string|translate_preview_template }}
{% if template %}
{{ template|string|translate_preview_template }}
{% endif %}

<div class="js-stick-at-bottom-when-scrolling">
<form method="post" enctype="multipart/form-data" action="{{url_for(
Expand Down
98 changes: 98 additions & 0 deletions tests/app/main/views/test_send.py
Original file line number Diff line number Diff line change
Expand Up @@ -3679,3 +3679,101 @@ def test_correct_error_displayed(
elif error_shown == "none":
assert page.find(attrs={"data-testid": "exceeds-annual"}) is None
assert page.find(attrs={"data-testid": "exceeds-daily"}) is None

@pytest.mark.parametrize(
"notification_type, exception_msg_api, expected_error_msg_admin",
[
("email", "Exceeded annual email sending", "These messages exceed the annual limit"),
("sms", "Exceeded annual SMS sending", "These messages exceed the annual limit"),
],
)
def test_error_msgs_from_api_for_one_off(
self,
client_request,
service_one,
fake_uuid,
mocker,
mock_get_service_template_with_placeholders,
mock_get_template_statistics,
notification_type,
exception_msg_api,
expected_error_msg_admin,
):
class MockHTTPError(HTTPError):
message = exception_msg_api

mocker.patch(
"app.notification_api_client.send_notification",
side_effect=MockHTTPError(),
)

if notification_type == "sms":
with client_request.session_transaction() as session:
session["recipient"] = "6502532223"
session["placeholders"] = {"name": "a" * 600}
elif notification_type == "email":
with client_request.session_transaction() as session:
session["recipient"] = "[email protected]"
session["placeholders"] = {"name": "a" * 600}

page = client_request.post(
"main.send_notification",
service_id=service_one["id"],
template_id=fake_uuid,
_expected_status=200,
)

assert normalize_spaces(page.select("h1")[0].text) == expected_error_msg_admin

@pytest.mark.parametrize(
"exception_msg_api, expected_error_msg_admin",
[
# ("email","Exceeded annual email sending", "These messages exceed the annual limit"),
("Exceeded annual SMS sending", "These messages exceed the annual limit")
],
)
def test_error_msgs_from_api_for_bulk(
self,
client_request,
mock_create_job,
mock_get_job,
mock_get_notifications,
mock_get_service_template,
mock_get_service_data_retention,
mocker,
fake_uuid,
exception_msg_api,
expected_error_msg_admin,
):
class MockHTTPError(HTTPError):
message = exception_msg_api

data = mock_get_job(SERVICE_ONE_ID, fake_uuid)["data"]
job_id = data["id"]
original_file_name = data["original_file_name"]
template_id = data["template"]
notification_count = data["notification_count"]
with client_request.session_transaction() as session:
session["file_uploads"] = {
fake_uuid: {
"template_id": template_id,
"notification_count": notification_count,
"valid": True,
}
}

mocker.patch(
"app.job_api_client.create_job",
side_effect=MockHTTPError(),
)
page = client_request.post(
"main.start_job",
service_id=SERVICE_ONE_ID,
upload_id=job_id,
original_file_name=original_file_name,
_data={},
_follow_redirects=True,
_expected_status=200,
)

assert normalize_spaces(page.select("h1")[0].text) == expected_error_msg_admin

0 comments on commit 2454078

Please sign in to comment.