From 6b24be4dc32984d77926b5d9702d198be040c459 Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:25:43 -0400 Subject: [PATCH] Add error message for rows_with_combined_variable_content_too_long (#2160) * Add error message for rows_with_combined_variable_content_too_long * Fix tests - Bump Waffles version - Bump Utils version --- .github/workflows/test.yaml | 2 +- app/v2/notifications/post_notifications.py | 7 +++++++ poetry.lock | 8 ++++---- pyproject.toml | 2 +- tests/app/celery/test_tasks.py | 14 +++++++++----- .../v2/notifications/test_post_notifications.py | 4 ++-- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 8d5c023e27..d5d607856c 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -67,7 +67,7 @@ jobs: run: | cp -f .env.example .env - name: Checks for new endpoints against AWS WAF rules - uses: cds-snc/notification-utils/.github/actions/waffles@06a40db6286f525fe3551e029418458d33342592 # 52.1.0 + uses: cds-snc/notification-utils/.github/actions/waffles@4e204b1ec688961ae32c3a6646246c58a1c1bc4f # 52.1.10 with: app-loc: '/github/workspace' app-libs: '/github/workspace/env/site-packages' diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 9a1640b9a2..ee69366f9d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -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, @@ -699,6 +700,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 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): diff --git a/poetry.lock b/poetry.lock index 1ab0346e40..3417ce4eb3 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2444,7 +2444,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "52.1.5" +version = "52.1.10" description = "Shared python code for Notification - Provides logging utils etc." optional = false python-versions = "~3.10.9" @@ -2479,8 +2479,8 @@ werkzeug = "2.3.7" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "52.1.5" -resolved_reference = "9d9e8c7c32e3608f4dd8f320eaba4bb67edfcbf5" +reference = "52.1.10" +resolved_reference = "4e204b1ec688961ae32c3a6646246c58a1c1bc4f" [[package]] name = "ordered-set" @@ -4255,4 +4255,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "f00992b7f47d8434a76d0be08135eace31315c696e20a222a10e2bf926e8a561" +content-hash = "c4c46eaf3c731be2eb3e27d29405e0c79ab17d53eaf3e7728f80145abc39c88e" diff --git a/pyproject.toml b/pyproject.toml index db3ae6fff3..4b73ba0a7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,7 +64,7 @@ Werkzeug = "2.3.7" MarkupSafe = "2.1.4" # 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.1.5" } +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.1.10" } # rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8 typing-extensions = "4.7.1" greenlet = "2.0.2" diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 437d59e7ec..0bbaff06ba 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -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 @@ -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( [ @@ -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( @@ -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( @@ -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", diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 2e3d1d36b2..a5b3da2fc0 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -2495,7 +2495,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, @@ -2546,7 +2546,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: