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

Author: whabanks

.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'

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):

poetry.lock

@@ -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"

pyproject.toml

@@ -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",

tests/app/celery/test_tasks.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: