Count by sms msg not sms parts for daily limit

- For SMS jobs, calls to check_sms_daily_limit and increment_sms_daily_count_send_warnings_if_needed now use the row count instead of fragment_count
- Refactored check_for_csv_errors, more_sms_rows_than_can_send was
  removed as redundant. We perform the ceck with more_rows_than_can_send
and determine the message to send by checking the template type
associated with the CSV send

app/job/rest.py

@@ -169,7 +169,7 @@ def create_job(service_id):
         is_test_notification = len(list(recipient_csv.get_rows())) == numberOfSimulated
 
         if not is_test_notification:
-            check_sms_daily_limit(service, recipient_csv.sms_fragment_count)
+            check_sms_daily_limit(service, len(recipient_csv))
 
     if template.template_type == EMAIL_TYPE:
         check_email_daily_limit(service, len(list(recipient_csv.get_rows())))
@@ -183,7 +183,7 @@ def create_job(service_id):
         raise InvalidRequest(errors, status_code=400)
 
     if template.template_type == SMS_TYPE and not is_test_notification:
-        increment_sms_daily_count_send_warnings_if_needed(service, recipient_csv.sms_fragment_count)
+        increment_sms_daily_count_send_warnings_if_needed(service, len(recipient_csv))
     elif template.template_type == EMAIL_TYPE:
         increment_email_daily_count_send_warnings_if_needed(service, len(list(recipient_csv.get_rows())))
 

app/service/send_notification.py

@@ -72,7 +72,7 @@ def send_one_off_notification(service_id, post_data):
     if template.template_type == SMS_TYPE:
         is_test_notification = simulated_recipient(post_data["to"], template.template_type)
         if not is_test_notification:
-            check_sms_daily_limit(service, template_with_content.fragment_count)
+            check_sms_daily_limit(service, 1)
     elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]:
         check_email_daily_limit(service, 1)  # 1 email
 
@@ -91,7 +91,7 @@ def send_one_off_notification(service_id, post_data):
     if template.template_type == SMS_TYPE:
         is_test_notification = simulated_recipient(post_data["to"], template.template_type)
         if not is_test_notification:
-            increment_sms_daily_count_send_warnings_if_needed(service, template_with_content.fragment_count)
+            increment_sms_daily_count_send_warnings_if_needed(service, 1)
     elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]:
         increment_email_daily_count_send_warnings_if_needed(service, 1)  # 1 email
 

app/sms_fragment_utils.py

@@ -13,11 +13,11 @@ def fetch_todays_requested_sms_count(service_id: UUID) -> int:
         return fetch_todays_total_sms_count(service_id)
 
     cache_key = sms_daily_count_cache_key(service_id)
-    fragment_count = redis_store.get(cache_key)
-    if fragment_count is None:
-        fragment_count = fetch_todays_total_sms_count(service_id)
-        redis_store.set(cache_key, fragment_count, ex=int(timedelta(hours=2).total_seconds()))
-    return int(fragment_count)
+    sms_count = redis_store.get(cache_key)
+    if sms_count is None:
+        sms_count = fetch_todays_total_sms_count(service_id)
+        redis_store.set(cache_key, sms_count, ex=int(timedelta(hours=2).total_seconds()))
+    return int(sms_count)
 
 
 def increment_todays_requested_sms_count(service_id: UUID, increment_by: int):

app/v2/notifications/post_notifications.py

@@ -61,6 +61,7 @@
     Notification,
     NotificationType,
     Service,
+    TemplateType,
 )
 from app.notifications.process_letter_notifications import create_letter_notification
 from app.notifications.process_notifications import (
@@ -238,8 +239,8 @@ def post_bulk():
         is_test_notification = api_user.key_type == KEY_TYPE_TEST or len(list(recipient_csv.get_rows())) == numberOfSimulated
 
         if not is_test_notification:
-            check_sms_daily_limit(authenticated_service, recipient_csv.sms_fragment_count)
-            increment_sms_daily_count_send_warnings_if_needed(authenticated_service, recipient_csv.sms_fragment_count)
+            check_sms_daily_limit(authenticated_service, len(recipient_csv))
+            increment_sms_daily_count_send_warnings_if_needed(authenticated_service, len(recipient_csv))
 
     job = create_bulk_job(authenticated_service, api_user, template, form, recipient_csv)
 
@@ -327,7 +328,7 @@ def post_notification(notification_type: NotificationType):
     if template.template_type == SMS_TYPE:
         is_test_notification = api_user.key_type == KEY_TYPE_TEST or simulated_recipient(form["phone_number"], notification_type)
         if not is_test_notification:
-            increment_sms_daily_count_send_warnings_if_needed(authenticated_service, template_with_content.fragment_count)
+            increment_sms_daily_count_send_warnings_if_needed(authenticated_service, 1)
 
     if notification_type == SMS_TYPE:
         create_resp_partial = functools.partial(create_post_sms_response_from_notification, from_number=reply_to)
@@ -667,16 +668,17 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages):
                 message=f"Duplicate column headers: {', '.join(sorted(recipient_csv.duplicate_recipient_column_headers))}",
                 status_code=400,
             )
-        if recipient_csv.more_sms_rows_than_can_send:
-            raise BadRequestError(
-                message=f"You only have {remaining_messages} remaining sms message parts before you reach your daily limit. You've tried to send {recipient_csv.sms_fragment_count} message parts.",
-                status_code=400,
-            )
         if recipient_csv.more_rows_than_can_send:
-            raise BadRequestError(
-                message=f"You only have {remaining_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.",
-                status_code=400,
-            )
+            if recipient_csv.template_type == SMS_TYPE:
+                raise BadRequestError(
+                    message=f"You only have {remaining_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.",
+                    status_code=400,
+                )
+            else:
+                raise BadRequestError(
+                    message=f"You only have {remaining_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.",
+                    status_code=400,
+                )
 
         if recipient_csv.too_many_rows:
             raise BadRequestError(

pyproject.toml

@@ -65,7 +65,7 @@ Werkzeug = "2.3.7"
 MarkupSafe = "2.1.3"
 # 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", rev = "52.0.12" }
+notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", rev = "52.0.13" }
 # rsa = "4.9  # awscli 1.22.38 depends on rsa<4.8
 typing-extensions = "4.7.1"
 greenlet = "2.0.2"

tests/app/v2/notifications/test_post_notifications.py

@@ -1797,12 +1797,13 @@ def __send_sms():
             "template_id": str(template.id),
             "personalisation": {" Name": "Jo"},
         }
-        for x in range(5):
+        for x in range(7):
             create_sample_notification(notify_db, notify_db_session, service=service)
 
         __send_sms()  # send 1 sms (3 fragments) should be at 80%
         assert send_warning_email.called
 
+        __send_sms()
         response = __send_sms()  # send one more, puts us at 11/10 fragments
         assert response.status_code == 429  # Ensure send is blocked
 
@@ -1895,48 +1896,6 @@ def __send_sms(number_to_send=1):
         response = __send_sms(1)  # attempt to send over limit (11 with max 10)1
         assert response.status_code == 400  # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else
 
-    def test_API_BULK_cant_hop_over_limit_2_fragment(self, notify_api, client, notify_db, notify_db_session, mocker):
-        # test setup
-        mocker.patch("app.sms_normal_publish.publish")
-        mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4()))
-        send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email")
-
-        def __send_sms(number_to_send=1):
-            with set_config_values(notify_api, {"REDIS_ENABLED": True}):
-                numbers = [["9025551234"]] * number_to_send
-                data = {
-                    "name": "job_name",
-                    "template_id": str(template.id),
-                    "rows": [["phone number"], *numbers],
-                }
-
-                response = client.post(
-                    "/v2/notifications/bulk",
-                    data=json.dumps(data),
-                    headers=[
-                        ("Content-Type", "application/json"),
-                        create_authorization_header(service_id=template.service_id),
-                    ],
-                )
-            return response
-
-        # Create a service, Set limit to 10 fragments
-        service = create_service(sms_daily_limit=10, message_limit=100)
-
-        # Create notifications in the db
-        template = create_sample_template(notify_db, notify_db_session, content="A" * 400, service=service, template_type="sms")
-        for x in range(5):
-            create_sample_notification(notify_db, notify_db_session, service=service)
-
-        __send_sms(1)  # 8/10 fragments used
-        assert send_warning_email.called
-
-        response = __send_sms(3)  # attempt to send over limit
-        assert response.status_code == 400
-
-        response = __send_sms(2)  # attempt to send over limit
-        assert response.status_code == 400
-
     # ADMIN
     def test_ADMIN_ONEOFF_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker):
         # test setup
@@ -2012,11 +1971,12 @@ def __send_sms():
 
         # Create 5 notifications in the db
         template = create_sample_template(notify_db, notify_db_session, content="a" * 400, service=service, template_type="sms")
-        for x in range(5):
+        for x in range(8):
             create_sample_notification(notify_db, notify_db_session, service=service)
 
-        __send_sms()  # 8/10 fragments
+        __send_sms()  # 9/10 fragments
         assert send_warning_email.called
+        __send_sms()
 
         response = __send_sms()  # 11/10 fragments
         assert response.status_code == 429  # Ensure send is blocked
@@ -2176,13 +2136,13 @@ def __send_sms(number_to_send=1):
         for x in range(6):
             create_sample_notification(notify_db, notify_db_session, service=service)
 
-        __send_sms(1)  # 8/10 fragments
+        __send_sms(2)  # 8/10 fragments
         assert send_warning_email.called
 
-        response = __send_sms(2)  # 12/10 fragments
+        response = __send_sms(4)  # 12/10 fragments
         assert response.status_code == 429
 
-        __send_sms(1)  # 10/10 fragments
+        __send_sms(2)  # 10/10 fragments
         assert send_limit_reached_email.called
 
         response = __send_sms(1)  # 11/10 fragments
@@ -2623,38 +2583,11 @@ def test_post_bulk_flags_not_enough_remaining_sms_messages(self, notify_api, cli
         assert error_json["errors"] == [
             {
                 "error": "BadRequestError",
-                "message": "You only have 1 remaining sms message parts before you reach your daily limit. You've tried to send 2 message parts.",
+                "message": "You only have 1 remaining sms messages before you reach your daily limit. You've tried to send 2 sms messages.",
             }
         ]
         messages_count_mock.assert_called_once()
 
-    def test_post_bulk_flags_not_enough_remaining_sms_message_parts(
-        self, notify_api, client, notify_db, notify_db_session, mocker
-    ):
-        service = create_service(sms_daily_limit=10, message_limit=100)
-        template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms")
-        mocker.patch("app.v2.notifications.post_notifications.fetch_todays_total_message_count", return_value=9)
-        mocker.patch("app.v2.notifications.post_notifications.fetch_todays_requested_sms_count", return_value=9)
-        data = {
-            "name": "job_name",
-            "template_id": template.id,
-            "csv": rows_to_csv([["phone number"], ["6135551234"]]),
-        }
-        response = client.post(
-            "/v2/notifications/bulk",
-            data=json.dumps(data),
-            headers=[("Content-Type", "application/json"), create_authorization_header(service_id=template.service_id)],
-        )
-
-        assert response.status_code == 400
-        error_json = json.loads(response.get_data(as_text=True))
-        assert error_json["errors"] == [
-            {
-                "error": "BadRequestError",
-                "message": "You only have 1 remaining sms message parts before you reach your daily limit. You've tried to send 4 message parts.",
-            }
-        ]
-
     @pytest.mark.parametrize("data_type", ["rows", "csv"])
     def test_post_bulk_flags_rows_with_errors(self, client, notify_db, notify_db_session, data_type):
         template = create_sample_template(notify_db, notify_db_session, template_type="email", content="Hello ((name))")