Skip to content

Commit

Permalink
Reinstate bulk sms limit (#2169)
Browse files Browse the repository at this point in the history
* Reinstate "Add error message for rows_with_combined_variable_content_too_long" (#2164)"

This reverts commit 5b29366.

* Reinstate bulk sms limit validation

- Added an additional check to ensure that we only return an 4xx response if the template type is SMS

* Bump utils version

* Refresh lock file
  • Loading branch information
whabanks authored May 22, 2024
1 parent 12b9571 commit 6919e3c
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 12 deletions.
7 changes: 7 additions & 0 deletions app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import werkzeug
from flask import abort, current_app, jsonify, request
from notifications_utils import SMS_CHAR_COUNT_LIMIT
from notifications_utils.recipients import (
RecipientCSV,
try_validate_and_format_phone_number,
Expand Down Expand Up @@ -705,6 +706,12 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages):
message=f"You cannot send to these recipients {explanation}",
status_code=400,
)
if recipient_csv.template_type == SMS_TYPE and any(recipient_csv.rows_with_combined_variable_content_too_long):
raise BadRequestError(
message=f"Row {next(recipient_csv.rows_with_combined_variable_content_too_long).index + 1} - has a character count greater than {SMS_CHAR_COUNT_LIMIT} characters. Some messages may be too long due to custom content.",
status_code=400,
)

if recipient_csv.rows_with_errors:

def row_error(row):
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 @@ -64,7 +64,7 @@ Werkzeug = "2.3.7"
MarkupSafe = "2.1.5"
# REVIEW: v2 is using sha512 instead of sha1 by default (in v1)
itsdangerous = "2.1.2"
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" }
# rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8
typing-extensions = "4.7.1"
greenlet = "2.0.2"
Expand Down
14 changes: 9 additions & 5 deletions tests/app/celery/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import uuid
from datetime import datetime, timedelta
from unittest import mock
from unittest.mock import Mock, call
from unittest.mock import MagicMock, Mock, call

import pytest
import requests_mock
Expand Down Expand Up @@ -891,9 +891,10 @@ def test_process_rows_sends_save_task(
mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid")
task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function))
signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign")
template = Mock(id="template_id", template_type=template_type, process_type=NORMAL)
template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL)
job = Mock(id="job_id", template_version="temp_vers", notification_count=1, api_key_id=api_key_id, sender_id=sender_id)
service = Mock(id="service_id", research_mode=research_mode)
template.__len__.return_value = 1

process_rows(
[
Expand Down Expand Up @@ -950,10 +951,11 @@ def test_should_redirect_email_job_to_queue_depending_on_csv_threshold(
):
mock_save_email = mocker.patch("app.celery.tasks.save_emails")

template = Mock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type)
template = MagicMock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type)
api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL)
job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key)
service = Mock(id=1, research_mode=False)
template.__len__.return_value = 1

row = next(
RecipientCSV(
Expand Down Expand Up @@ -994,10 +996,11 @@ def test_should_redirect_sms_job_to_queue_depending_on_csv_threshold(
):
mock_save_sms = mocker.patch("app.celery.tasks.save_smss")

template = Mock(id=1, template_type=SMS_TYPE, process_type=template_process_type)
template = MagicMock(id=1, template_type=SMS_TYPE, process_type=template_process_type)
api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL)
job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key)
service = Mock(id=1, research_mode=False)
template.__len__.return_value = 1

row = next(
RecipientCSV(
Expand Down Expand Up @@ -1066,7 +1069,8 @@ def test_process_rows_works_without_key_type(
mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid")
task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function))
signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign")
template = Mock(id="template_id", template_type=template_type, process_type=NORMAL)
template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL)
template.__len__.return_value = 1
api_key = {}
job = Mock(
id="job_id",
Expand Down
4 changes: 2 additions & 2 deletions tests/app/v2/notifications/test_post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -2523,7 +2523,7 @@ def test_post_bulk_with_too_large_sms_fails(self, client, notify_db, notify_db_s
mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4()))

service = create_service(sms_daily_limit=10, message_limit=100)
template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 612)
template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 613)
data = {
"name": "job_name",
"template_id": template.id,
Expand Down Expand Up @@ -2574,7 +2574,7 @@ def test_post_bulk_with_too_large_sms_fail_and_shows_correct_row(
)
assert response.status_code == 400
assert "has a character count greater than" in str(response.data)
assert "row #{}".format(failure_row) in str(response.data)
assert "Row {}".format(failure_row) in str(response.data)


class TestBatchPriorityLanes:
Expand Down

0 comments on commit 6919e3c

Please sign in to comment.