diff --git a/app/job/rest.py b/app/job/rest.py index 45f07840c4..3c443ec4ba 100644 --- a/app/job/rest.py +++ b/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()))) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index cfae1129a4..813ce2f138 100644 --- a/app/service/send_notification.py +++ b/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 diff --git a/app/sms_fragment_utils.py b/app/sms_fragment_utils.py index 1398a98d97..0b7bdb152f 100644 --- a/app/sms_fragment_utils.py +++ b/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): diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 0280df058b..cf979b1923 100644 --- a/app/v2/notifications/post_notifications.py +++ b/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( diff --git a/pyproject.toml b/pyproject.toml index a5e80de922..fe18053b2f 100644 --- a/pyproject.toml +++ b/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" diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index e86c405f64..5bb2831012 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/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))")