Count by sms messages not sms parts for daily limit (#2024)

* 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

* Update fetch_todays_total_sms_count no longer count by fragment

- Removed a number of tests involving counting of fragments vs limits
- Fixed a call to check_sms_daily_limit that was still passing fragments into it's parameters

* Bump utils version and update lock file

* Formatting

app/dao/services_dao.py

@@ -443,7 +443,7 @@ def fetch_todays_total_message_count(service_id):
 def fetch_todays_total_sms_count(service_id):
     midnight = get_midnight(datetime.now(tz=pytz.utc))
     result = (
-        db.session.query(func.sum(Notification.billable_units).label("sum_billable_units"))
+        db.session.query(func.count(Notification.id).label("total_sms_notifications"))
         .filter(
             Notification.service_id == service_id,
             Notification.key_type != KEY_TYPE_TEST,
@@ -452,7 +452,7 @@ def fetch_todays_total_sms_count(service_id):
         )
         .first()
     )
-    return 0 if result is None or result.sum_billable_units is None else result.sum_billable_units
+    return 0 if result is None or result.total_sms_notifications is None else result.total_sms_notifications
 
 
 def fetch_service_email_limit(service_id: uuid.UUID) -> int:

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)
 
@@ -296,7 +297,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:
-            check_sms_daily_limit(authenticated_service, template_with_content.fragment_count)
+            check_sms_daily_limit(authenticated_service, 1)
 
     current_app.logger.info(f"Trying to send notification for Template ID: {template.id}")
 
@@ -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

@@ -64,7 +64,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.14" }
 # rsa = "4.9  # awscli 1.22.38 depends on rsa<4.8
 typing-extensions = "4.7.1"
 greenlet = "2.0.2"

tests/app/dao/test_services_dao.py

@@ -1090,13 +1090,6 @@ def test_only_counts_sms(self):
         save_notification(create_notification(template=email_template))
         assert fetch_todays_total_sms_count(service.id) == 2
 
-    def test_sums_billable_units(self):
-        service = create_service()
-        sms_template = create_template(service=service, template_type=SMS_TYPE)
-        save_notification(create_notification(template=sms_template, billable_units=3))
-        save_notification(create_notification(template=sms_template, billable_units=10))
-        assert fetch_todays_total_sms_count(service.id) == 13
-
     def test_returns_0_when_no_messages_for_today(self):
         assert fetch_todays_total_sms_count(uuid.uuid4()) == 0