diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 229552ee8c..2564e7c2cd 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -198,8 +198,8 @@ def check_email_annual_limit(service: Service, requested_emails=0): if not send_exceeds_annual_limit: # Will this send put the service right at their limit? - if send_reaches_annual_limit and not annual_limit_client.check_has_over_limit_been_sent(service.id, SMS_TYPE): - annual_limit_client.set_over_sms_limit(service.id) + if send_reaches_annual_limit and not annual_limit_client.check_has_over_limit_been_sent(service.id, EMAIL_TYPE): + annual_limit_client.set_over_email_limit(service.id) current_app.logger.info( f"Service {service.id} reached their annual email limit of {service.email_annual_limit} when sending {requested_emails} messages. Sending reached annual limit email." ) @@ -257,16 +257,16 @@ def check_sms_annual_limit(service: Service, requested_sms=0): current_app.logger.info( f"Service {service.id} reached their annual SMS limit of {service.sms_annual_limit} messages. Sending reached annual limit email." ) - send_annual_limit_reached_email(service, "email", current_fiscal_year + 1) + send_annual_limit_reached_email(service, "sms", current_fiscal_year + 1) # Will this send put annual usage within 80% of the limit? - if is_near_annual_limit and not annual_limit_client.check_has_warning_been_sent(service.id, EMAIL_TYPE): - annual_limit_client.set_nearing_email_limit(service.id) + if is_near_annual_limit and not annual_limit_client.check_has_warning_been_sent(service.id, SMS_TYPE): + annual_limit_client.set_nearing_sms_limit(service.id) current_app.logger.info( f"Service {service.id} reached 80% of their annual SMS limit of {service.sms_annual_limit} messages. Sending annual limit usage warning email." ) send_near_annual_limit_warning_email( - service, "email", int(sms_sent_today + sms_sent_this_fiscal), current_fiscal_year + 1 + service, "sms", int(sms_sent_today + sms_sent_this_fiscal), current_fiscal_year + 1 ) return @@ -497,6 +497,7 @@ def send_annual_limit_reached_email(service: Service, notification_type: Notific service_id=service.id, template_id=current_app.config["REACHED_ANNUAL_LIMIT_TEMPLATE_ID"], personalisation={ + "service_name": service.name, "message_type_en": notification_type, "message_type_fr": "Courriel" if notification_type == EMAIL_TYPE else "SMS", "fiscal_end": fiscal_end, diff --git a/app/service/rest.py b/app/service/rest.py index 5e5d4dc4e2..02c846a87f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -133,6 +133,10 @@ register_errors(service_blueprint) +# TODO: FF_ANNUAL_LIMIT - Remove once logic is consolidated in the annual_limit_client +ANNUAL_LIMIT_SMS_OVER_NEAR_STATUS_FIELDS = ["near_sms_limit", "near_email_limit"] +ANNUAL_LIMIT_EMAIL_OVER_NEAR_STATUS_FIELDS = ["over_email_limit", "near_email_limit"] + @service_blueprint.errorhandler(IntegrityError) def handle_integrity_error(exc): @@ -317,13 +321,11 @@ def update_service(service_id): if sms_annual_limit_changed: _warn_service_users_about_annual_limit_change(service, SMS_TYPE) # TODO: abstract this in the annual_limits_client - redis_store.delete(f"annual-limit:{service_id}:status:near_sms_limit") - redis_store.delete(f"annual-limit:{service_id}:status:over_sms_limit") + redis_store.delete_hash_fields(f"annual-limit:{service_id}:status", ANNUAL_LIMIT_SMS_OVER_NEAR_STATUS_FIELDS) if email_annual_limit_changed: _warn_service_users_about_annual_limit_change(service, EMAIL_TYPE) # TODO: abstract this in the annual_limits_client - redis_store.delete(f"annual-limit:{service_id}:status:near_email_limit") - redis_store.delete(f"annual-limit:{service_id}:status:over_email_limit") + redis_store.delete_hash_fields(f"annual-limit:{service_id}:status", ANNUAL_LIMIT_EMAIL_OVER_NEAR_STATUS_FIELDS) if service_going_live: _warn_services_users_about_going_live(service_id, current_data) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 20fd0a0949..15ff30c206 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -701,27 +701,27 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_daily_messages, rema message=f"Duplicate column headers: {', '.join(sorted(recipient_csv.duplicate_recipient_column_headers))}", status_code=400, ) - ## TODO: FF_ANNUAL_LIMIT - remove this if block in favour of more_rows_than_can_send_today found below - if recipient_csv.more_rows_than_can_send: + if recipient_csv.more_rows_than_can_send_this_year: if recipient_csv.template_type == SMS_TYPE: raise BadRequestError( - message=f"You only have {remaining_daily_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.", + message=f"You only have {remaining_annual_messages} remaining sms messages before you reach your annual limit. You've tried to send {len(recipient_csv)} sms messages.", status_code=400, ) else: raise BadRequestError( - message=f"You only have {remaining_daily_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.", + message=f"You only have {remaining_annual_messages} remaining messages before you reach your annual limit. You've tried to send {nb_rows} messages.", status_code=400, ) - if recipient_csv.more_rows_than_can_send_this_year: + ## TODO: FF_ANNUAL_LIMIT - remove this if block in favour of more_rows_than_can_send_today found below + if recipient_csv.more_rows_than_can_send: if recipient_csv.template_type == SMS_TYPE: raise BadRequestError( - message=f"You only have {remaining_annual_messages} remaining sms messages before you reach your annual limit. You've tried to send {len(recipient_csv)} sms messages.", + message=f"You only have {remaining_daily_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_annual_messages} remaining messages before you reach your annual limit. You've tried to send {nb_rows} messages.", + message=f"You only have {remaining_daily_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.", status_code=400, ) if recipient_csv.more_rows_than_can_send_today: diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index c217e431aa..7c861cf5a3 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -893,3 +893,40 @@ def test_check_sms_annual_limit( mock_redis_set.assert_called_with(service.id) if log_msg: assert log_msg in mock_logger.call_args[0][0] + + def test_check_sms_annual_limit_only_sends_warning_email_once( + self, + notify_api, + notify_db, + notify_db_session, + mocker, + ): + mocker.patch("app.redis_store.set_hash_value") + mocker.patch("app.redis_store.get", return_value=45) + mocker.patch("app.annual_limit_client.check_has_warning_been_sent", return_value=True) + mock_send_email = mocker.patch("app.notifications.validators.send_notification_to_service_users") + + service = create_sample_service(notify_db, notify_db_session, sms_annual_limit=49) + + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + check_sms_annual_limit(service, 2) + mock_send_email.assert_not_called() + + def test_check_sms_annual_limit_only_sends_reached_limit_email_once( + self, + notify_api, + notify_db, + notify_db_session, + mocker, + ): + mocker.patch("app.redis_store.set_hash_value") + mocker.patch("app.redis_store.get", return_value=45) + mocker.patch("app.annual_limit_client.check_has_over_limit_been_sent", return_value=True) + mocker.patch("app.annual_limit_client.check_has_warning_been_sent", return_value=True) + mock_send_email = mocker.patch("app.notifications.validators.send_notification_to_service_users") + + service = create_sample_service(notify_db, notify_db_session, sms_annual_limit=49) + + with set_config(notify_api, "FF_ANNUAL_LIMIT", True): + check_sms_annual_limit(service, 4) + mock_send_email.assert_not_called() diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 32860da226..80a061cbe9 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1028,7 +1028,7 @@ def test_update_service_annual_limits( mocker, ): mocker.patch("app.service.rest.send_notification_to_service_users") - redis_delete = mocker.patch("app.service.rest.redis_store.delete") + redis_delete = mocker.patch("app.service.rest.redis_store.delete_hash_fields") admin_request.post( "service.update_service", service_id=sample_service.id, @@ -1040,13 +1040,11 @@ def test_update_service_annual_limits( assert getattr(sample_service, limit_field) == limit_value if limit_field == "sms_annual_limit": assert redis_delete.call_args_list == [ - call(f"annual-limit:{sample_service.id}:status:near_sms_limit"), - call(f"annual-limit:{sample_service.id}:status:over_sms_limit"), + call(f"annual-limit:{sample_service.id}:status", ["near_sms_limit", "near_email_limit"]), ] if limit_field == "email_annual_limit": assert redis_delete.call_args_list == [ - call(f"annual-limit:{sample_service.id}:status:near_email_limit"), - call(f"annual-limit:{sample_service.id}:status:over_email_limit"), + call(f"annual-limit:{sample_service.id}:status", ["over_email_limit", "near_email_limit"]), ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index a41129cd06..b6a76d8c5f 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -2773,6 +2773,7 @@ def __send_notification(): def test_post_bulk_validates_annual_limit( notify_api, notify_db, notify_db_session, client, template_type, data_type, expected_msg, mocker ): + mock_check_daily_limit = mocker.patch(f"app.v2.notifications.post_notifications.check_{template_type}_daily_limit") service = create_service(email_annual_limit=1, sms_annual_limit=1) template = create_template(service=service, template_type=template_type) data = { @@ -2799,6 +2800,8 @@ def test_post_bulk_validates_annual_limit( assert response.status_code == 400 assert resp_json["errors"][0]["message"] == expected_msg + mock_check_daily_limit.assert_not_called() + class TestSeedingBounceRateData: @freeze_time("2019-01-01 12:00:00.000000")