diff --git a/app/config.py b/app/config.py index 7188616208..d016e989c2 100644 --- a/app/config.py +++ b/app/config.py @@ -258,6 +258,8 @@ class Config(object): NEAR_DAILY_SMS_LIMIT_TEMPLATE_ID = "a796568f-a89b-468e-b635-8105554301b9" REACHED_DAILY_SMS_LIMIT_TEMPLATE_ID = "a646e614-c527-4f94-a955-ed7185d577f4" DAILY_SMS_LIMIT_UPDATED_TEMPLATE_ID = "6ec12dd0-680a-4073-8d58-91d17cc8442f" + BOUNCE_RATE_EXCEEDED_ID = "2dd32fab-0dd5-411b-9fa2-d23469eabbaf" + BOUNCE_RATE_WARNING_ID = "60d67125-e088-4793-a260-70d43982ec5a" # Allowed service IDs able to send HTML through their templates. ALLOW_HTML_SERVICE_IDS: List[str] = [id.strip() for id in os.getenv("ALLOW_HTML_SERVICE_IDS", "").split(",")] diff --git a/app/service/rest.py b/app/service/rest.py index 4295fad361..4be64f3383 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -17,7 +17,7 @@ from app import redis_store from app.clients.zendesk_sell import ZenDeskSell -from app.config import QueueNames +from app.config import Config, QueueNames from app.dao import fact_notification_status_dao, notifications_dao from app.dao.api_key_dao import ( expire_api_key, @@ -710,6 +710,10 @@ def suspend_service(service_id): if service.active: dao_suspend_service(service.id) + # TODO: Check if the service's bounce rate has been exceeded + # will depend on agreed upon method for fetching / storing + # a services bounce rate. + notify_bounce_rate_exceeded(service) return "", 204 @@ -1123,3 +1127,54 @@ def check_if_reply_to_address_already_in_use(service_id, email_address): }, status_code=400, ) + + +def notify_bounce_rate_exceeded(user_service): + service = Service.query.get(current_app.config["NOTIFY_SERVICE_ID"]) + recipient = dao_fetch_service_creator(user_service.id).email_address + template = dao_get_template_by_id(current_app.config["BOUNCE_RATE_EXCEEDED_ID"]) + reply_to = template.service.get_default_reply_to_email_address() + + saved_notification = persist_notification( + template_id=template.id, + template_version=template.version, + recipient=recipient, + service=service, + personalisation={ + "service_name": user_service.name, + "contact_us_url": f"{Config.ADMIN_BASE_URL}/contact", + }, + notification_type=template.template_type, + api_key_id=None, + key_type=KEY_TYPE_NORMAL, + reply_to_text=reply_to, + ) + + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) + + +def notify_bounce_rate_warning(user_service): + if user_service is None: + raise TypeError("user_service: must not be None") + service = Service.query.get(current_app.config["NOTIFY_SERVICE_ID"]) + user = dao_fetch_service_creator(user_service.id) + template = dao_get_template_by_id(current_app.config["BOUNCE_RATE_LIMIT_WARNING_ID"]) + reply_to = template.service.get_default_reply_to_email_address() + + saved_notification = persist_notification( + template_id=template.id, + template_version=template.version, + recipient=user.email_address, + service=service, + personalisation={ + "name": user.name, + "service_name": user_service.name, + "contact_us_url": f"{Config.ADMIN_BASE_URL}/contact", + }, + notification_type=template.template_type, + api_key_id=None, + key_type=KEY_TYPE_NORMAL, + reply_to_text=reply_to, + ) + + send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) diff --git a/migrations/versions/0425_bounce_rate_limits.py b/migrations/versions/0425_bounce_rate_limits.py new file mode 100644 index 0000000000..183c4f6937 --- /dev/null +++ b/migrations/versions/0425_bounce_rate_limits.py @@ -0,0 +1,89 @@ +""" +Revision ID: 0425_bounce_rate_limits + +Revises: 0424_sms_templates_in_redacted + +Create Date: 2022-12-28 00:00:00 +""" + +from datetime import datetime +from alembic import op +from flask import current_app + + +revision = "0425_bounce_rate_limites" +down_revision = "0424_sms_templates_in_redacted" + +bounce_rate_exceeded = current_app.config["BOUNCE_RATE_EXCEEDED_ID"] +bounce_rate_warning = current_app.config["BOUNCE_RATE_WARNING_ID"] + +templates = [ + { + "id": bounce_rate_exceeded, + "name": "Bounce Rate Exceeded", + "type": "email", + "subject": "Notification service bounce rate exceeded", + "content_lines": [ + 'The bounce rate for your service, "((service_name))" has been exceeded. ', + "", + "To ensure we can provide reliable, uninterrupted service to all users of Notify, we temporarily suspended your service: ((service_name))", + "", + "To resume your service, please [contact us](((contact_us_url)))", + ], + }, + { + "id": bounce_rate_warning, + "name": "Bounce Rate Warning", + "type": "email", + "subject": "Notification service bounce rate warning", + "content_lines": [ + "Hello ((name))" "" 'Your service, "((service_name))" is approaching the bounce rate limit.', + "", + "To ensure that your service is not suspended, please ensure that your recipient lists are up to date and contain valid email addresses", + "", + "To learn more about managing the bounce rate for your services [contact us](((contact_us_url)))", + ], + }, +] + + +def upgrade(): + + insert = """ + INSERT INTO {} (id, name, template_type, created_at, content, archived, service_id, subject, + created_by_id, version, process_type, hidden) + VALUES ('{}', '{}', '{}', current_timestamp, '{}', False, '{}', '{}', '{}', 1, '{}', false) + """ + + for template in templates: + for table_name in "templates", "templates_history": + op.execute( + insert.format( + table_name, + template["id"], + template["name"], + template["type"], + "\n".join(template["content_lines"]), + current_app.config["NOTIFY_SERVICE_ID"], + template.get("subject"), + current_app.config["NOTIFY_USER_ID"], + "normal", + ) + ) + op.execute( + f""" + INSERT INTO template_redacted( + template_id, + redact_personalisation, + updated_at, + updated_by_id + ) VALUES ('{template["id"]}', false, current_timestamp, '{current_app.config["NOTIFY_USER_ID"]}') + """ + ) + + +def downgrade(): + for template in templates: + for table in "templates", "template_history": + op.execute(f"DELETE FROM {table} where template_id = '{template['id']}'") + op.execute(f"DELETE FROM template_redacted WHERE template_id = '{template['id']}'") diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 027631f61c..56b8a6c4c9 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1200,6 +1200,30 @@ def no_reply_template(notify_db, notify_db_session): ) +@pytest.fixture(scope="function") +def bounce_rate_templates(notify_db, notify_db_session): + service, user = notify_service(notify_db, notify_db_session) + import importlib + + bounce_exceeded = importlib.import_module("migrations.versions.0425_bounce_rate_limits") + + return { + config_name: create_custom_template( + service, + user, + config_name, + "email", + content="\n".join( + next(x for x in bounce_exceeded.templates if x["id"] == current_app.config[config_name])["content_lines"] + ), + ) + for config_name in [ + "BOUNCE_RATE_EXCEEDED_ID", + "BOUNCE_RATE_WARNING_ID", + ] + } + + @pytest.fixture(scope="function") def mou_signed_templates(notify_db, notify_db_session): service, user = notify_service(notify_db, notify_db_session) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 8f05d0fb0b..70e568acad 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2851,38 +2851,23 @@ def test_delete_service_reply_to_email_address_archives_an_email_reply_to(sample assert reply_to.archived is True -def test_delete_service_reply_to_email_address_archives_default_reply_to_if_no_others_exist( +def test_delete_service_reply_to_email_address_returns_400_if_archiving_default_reply_to( admin_request, notify_db_session, sample_service ): reply_to = create_reply_to_email(service=sample_service, email_address="some@email.com") - admin_request.post( - "service.delete_service_reply_to_email_address", - service_id=sample_service.id, - reply_to_email_id=reply_to.id, - ) - - assert reply_to.archived is True - - -def test_delete_service_reply_to_email_address_returns_400_if_archiving_default_reply_to_and_others_exist( - admin_request, notify_db_session, sample_service -): - reply_to_1 = create_reply_to_email(service=sample_service, email_address="some_1@email.com") - create_reply_to_email(service=sample_service, email_address="some_2@email.com") - response = admin_request.post( "service.delete_service_reply_to_email_address", service_id=sample_service.id, - reply_to_email_id=reply_to_1.id, + reply_to_email_id=reply_to.id, _expected_status=400, ) assert response == { - "message": "You cannot delete a default email reply to address if other reply to addresses exist", + "message": "You cannot delete a default email reply to address", "result": "error", } - assert reply_to_1.archived is False + assert reply_to.archived is False def test_get_email_reply_to_address(client, notify_db, notify_db_session): @@ -3405,6 +3390,7 @@ def test_cancel_notification_for_service_raises_invalid_request_when_notificatio admin_request, sample_notification, ): + response = admin_request.post( "service.cancel_notification_for_service", service_id=sample_notification.service_id, @@ -3517,3 +3503,43 @@ def test_get_monthly_notification_data_by_service(mocker, admin_request): dao_mock.assert_called_once_with(start_date, end_date) assert response == [] + + +def test_suspend_service_bounce_rate_exceeded_email_sent(mocker, sample_service, admin_request, bounce_rate_templates): + mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") + + admin_request.post( + "service.suspend_service", + service_id=sample_service.id, + _expected_status=204, + ) + + notification = Notification.query.first() + # TODO: Mock a service with an exceeded bounce rate if we end up storing + # that data as a status indicator in the DB + service = Service.query.get(sample_service.id) + + mocked.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks") + assert notification.personalisation["service_name"] == sample_service.name + assert service.active is False + + for api_key in service.api_keys: + assert api_key.expiry_date <= datetime.utcnow() + + +@pytest.mark.skip( + reason="Depends on completion of ADR for bounce rate. How we will fetch and store whether a service has hit or is approaching the bounce rate needs to be decided and implemented" +) +def test_suspend_service_bounce_rate_not_exceeded_no_email_sent(mocker, sample_service, admin_request, bounce_rate_templates): + mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") + + response = admin_request.post( + "service.suspend_service", + service_id=sample_service.id, + _expected_status=204, + ) + + notification = Notification.query.first() + service = Service.query.get(sample_service.id) + + assert True