From 6613cad2ae67da31ee76a9b3689f5ceaf4af020f Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 3 Jan 2023 15:53:06 +0000 Subject: [PATCH 1/3] Implement email notification when a service is suspended from the admin panel --- app/config.py | 2 + app/service/rest.py | 57 +++++++++++- .../versions/0425_bounce_rate_limits.py | 89 +++++++++++++++++++ tests/app/conftest.py | 24 +++++ tests/app/service/test_rest.py | 64 +++++++++---- 5 files changed, 216 insertions(+), 20 deletions(-) create mode 100644 migrations/versions/0425_bounce_rate_limits.py 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 From 522e1f264bd84d5f3655e665ffec5ce576d4aea1 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 10 Jan 2023 18:23:45 -0400 Subject: [PATCH 2/3] Add notification when a user's service is resumed - Update db migrations - Add test for service resume notification - Add new draft email content --- app/config.py | 1 + app/service/rest.py | 31 ++++- .../versions/0425_bounce_rate_limits.py | 89 ------------- .../versions/0425_service_suspend_resume.py | 118 ++++++++++++++++++ tests/app/conftest.py | 9 +- tests/app/service/test_rest.py | 26 +++- 6 files changed, 176 insertions(+), 98 deletions(-) delete mode 100644 migrations/versions/0425_bounce_rate_limits.py create mode 100644 migrations/versions/0425_service_suspend_resume.py diff --git a/app/config.py b/app/config.py index d016e989c2..589855ea49 100644 --- a/app/config.py +++ b/app/config.py @@ -260,6 +260,7 @@ class Config(object): 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" + SERVICE_RESUMED_ID = "b393b19f-957f-4b4a-84b7-494dc1c615c3" # 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 4be64f3383..5c5cd78a82 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -706,6 +706,7 @@ def suspend_service(service_id): :param service_id: :return: """ + service = dao_fetch_service_by_id(service_id) if service.active: @@ -713,7 +714,7 @@ def 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) + notify_bounce_rate_exceeded() return "", 204 @@ -730,6 +731,7 @@ def resume_service(service_id): if not service.active: dao_resume_service(service.id) + notify_service_resumed(dao_fetch_service_by_id(service_id)) return "", 204 @@ -1154,8 +1156,6 @@ def notify_bounce_rate_exceeded(user_service): 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"]) @@ -1169,7 +1169,30 @@ def notify_bounce_rate_warning(user_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) + + +def notify_service_resumed(user_service): + 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["SERVICE_RESUMED_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, }, notification_type=template.template_type, api_key_id=None, diff --git a/migrations/versions/0425_bounce_rate_limits.py b/migrations/versions/0425_bounce_rate_limits.py deleted file mode 100644 index 183c4f6937..0000000000 --- a/migrations/versions/0425_bounce_rate_limits.py +++ /dev/null @@ -1,89 +0,0 @@ -""" -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/migrations/versions/0425_service_suspend_resume.py b/migrations/versions/0425_service_suspend_resume.py new file mode 100644 index 0000000000..b79321bd0b --- /dev/null +++ b/migrations/versions/0425_service_suspend_resume.py @@ -0,0 +1,118 @@ +""" +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_service_suspend_resume" +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"] +resume_service = current_app.config["SERVICE_RESUMED_ID"] + +templates = [ + { + "id": bounce_rate_exceeded, + "name": "Bounce Rate Exceeded", + "type": "email", + "subject": "We’ve suspended your service", + "content_lines": [ + "The email bounce rate has reached 10% for “((service_name))”. We’ve suspended the service to maintain our operations. While the service is under suspension, it will not be able to send email or text messages.", + "", + "You need to update and verify your list of recipient’s email addresses. Once you’ve taken these steps, you can request to resume service by [contacting us]((contact_us_url)). ", + "", + "The bounce rate for each service on GC Notify contributes to our total bounce rate. A high bounce rate suggests we’re emailing recipients without their consent. Then email providers send messages from GC Notify to the spam folder.", + "", + "An email may bounce if:", + "", + "(1) The recipient or their email provider has blocked sends from your service.", + "(2) You send to an email address that does not exist." + "", + "The GC Notify team" + ], + }, + { + "id": bounce_rate_warning, + "name": "Bounce Rate Warning", + "type": "email", + "subject": "Your bounce rate has exceeded 5%", + "content_lines": [ + "Hello ((name))," + "", + 'The bounce rate has exceeded 5% for “((service_name))”. You should update your list of recipient’s email addresses.' + "", + "An email may bounce if:", + "(1) The recipient or their email provider has blocked sends from your service.", + "(2) You send to an email address that does not exist.", + "", + "The bounce rate for each service on GC Notify contributes to our total bounce rate. A high bounce rate suggests we’re emailing recipients without their consent. Then email providers send messages from GC Notify to the spam folder.", + "", + "To maintain our operations, we’ll suspend the service if its bounce rate reaches 10%. While the service is under suspension, it will not be able to send email or text messages." + "", + "The GC Notify team" + ], + }, + { + "id": resume_service, + "name": "Resume Service", + "type": "email", + "subject": "We’ve resumed your service", + "content_lines": [ + "Hello ((name)),", + "", + "“((service_name))” can send messages again. We’ve removed the suspension." + "", + "The GC Notify Team", + ] + } +] + + +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 56b8a6c4c9..55eb82ba46 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -212,6 +212,10 @@ def sample_service( research_mode=None, ) +@pytest.fixture(scope="function") +def sample_inactive_service(sample_service): + sample_service.active = False + return sample_service @pytest.fixture(scope="function", name="sample_service_full_permissions") def _sample_service_full_permissions(notify_db_session): @@ -1201,11 +1205,11 @@ def no_reply_template(notify_db, notify_db_session): @pytest.fixture(scope="function") -def bounce_rate_templates(notify_db, notify_db_session): +def bounce_rate_suspend_resume_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") + bounce_exceeded = importlib.import_module("migrations.versions.0425_service_suspend_resume") return { config_name: create_custom_template( @@ -1220,6 +1224,7 @@ def bounce_rate_templates(notify_db, notify_db_session): for config_name in [ "BOUNCE_RATE_EXCEEDED_ID", "BOUNCE_RATE_WARNING_ID", + "SERVICE_RESUMED_ID", ] } diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 70e568acad..f1bb07f7d8 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -3505,7 +3505,7 @@ def test_get_monthly_notification_data_by_service(mocker, admin_request): assert response == [] -def test_suspend_service_bounce_rate_exceeded_email_sent(mocker, sample_service, admin_request, bounce_rate_templates): +def test_suspend_service_bounce_rate_exceeded_email_sent(mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates): mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") admin_request.post( @@ -3527,10 +3527,30 @@ def test_suspend_service_bounce_rate_exceeded_email_sent(mocker, sample_service, assert api_key.expiry_date <= datetime.utcnow() +def test_resume_service_user_notification_email_sent(mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates): + mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") + sample_service.active = False + + admin_request.post( + "service.resume_service", + service_id=sample_service.id, + _expected_status=204, + ) + + notification = Notification.query.first() + 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 True + + + @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" + reason="Depends on completion of ADR for bounce rate. The decision regarding how we will fetch and store whether a service has hit or is approaching the bounce rate needs to be made." ) -def test_suspend_service_bounce_rate_not_exceeded_no_email_sent(mocker, sample_service, admin_request, bounce_rate_templates): +def test_suspend_service_bounce_rate_not_exceeded_no_email_sent(mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates): + # TODO mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") response = admin_request.post( From 7100aa627b0a1b61a63d7038d7fe1ff85f5a02e4 Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 2 Feb 2023 19:25:19 +0000 Subject: [PATCH 3/3] Formatting --- .../versions/0425_service_suspend_resume.py | 21 ++++---- tests/app/conftest.py | 2 + tests/app/dao/test_permissions_dao.py | 27 +++++----- tests/app/service/test_rest.py | 9 ++-- tests/app/test_config.py | 49 +++++++++---------- 5 files changed, 52 insertions(+), 56 deletions(-) diff --git a/migrations/versions/0425_service_suspend_resume.py b/migrations/versions/0425_service_suspend_resume.py index b79321bd0b..956e45e94c 100644 --- a/migrations/versions/0425_service_suspend_resume.py +++ b/migrations/versions/0425_service_suspend_resume.py @@ -7,10 +7,10 @@ """ from datetime import datetime + from alembic import op from flask import current_app - revision = "0425_service_suspend_resume" down_revision = "0424_sms_templates_in_redacted" @@ -34,9 +34,8 @@ "An email may bounce if:", "", "(1) The recipient or their email provider has blocked sends from your service.", - "(2) You send to an email address that does not exist." - "", - "The GC Notify team" + "(2) You send to an email address that does not exist." "", + "The GC Notify team", ], }, { @@ -45,9 +44,8 @@ "type": "email", "subject": "Your bounce rate has exceeded 5%", "content_lines": [ - "Hello ((name))," - "", - 'The bounce rate has exceeded 5% for “((service_name))”. You should update your list of recipient’s email addresses.' + "Hello ((name))," "", + "The bounce rate has exceeded 5% for “((service_name))”. You should update your list of recipient’s email addresses." "", "An email may bounce if:", "(1) The recipient or their email provider has blocked sends from your service.", @@ -57,7 +55,7 @@ "", "To maintain our operations, we’ll suspend the service if its bounce rate reaches 10%. While the service is under suspension, it will not be able to send email or text messages." "", - "The GC Notify team" + "The GC Notify team", ], }, { @@ -68,11 +66,10 @@ "content_lines": [ "Hello ((name)),", "", - "“((service_name))” can send messages again. We’ve removed the suspension." - "", + "“((service_name))” can send messages again. We’ve removed the suspension." "", "The GC Notify Team", - ] - } + ], + }, ] diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 55eb82ba46..e6db3320fc 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -212,11 +212,13 @@ def sample_service( research_mode=None, ) + @pytest.fixture(scope="function") def sample_inactive_service(sample_service): sample_service.active = False return sample_service + @pytest.fixture(scope="function", name="sample_service_full_permissions") def _sample_service_full_permissions(notify_db_session): service = create_service( diff --git a/tests/app/dao/test_permissions_dao.py b/tests/app/dao/test_permissions_dao.py index 635686ed2c..02bbfbf567 100644 --- a/tests/app/dao/test_permissions_dao.py +++ b/tests/app/dao/test_permissions_dao.py @@ -5,21 +5,18 @@ def test_get_permissions_by_user_id_returns_all_permissions(sample_service): permissions = permission_dao.get_permissions_by_user_id(user_id=sample_service.users[0].id) assert len(permissions) == 8 - assert ( - sorted( - [ - "manage_users", - "manage_templates", - "manage_settings", - "send_texts", - "send_emails", - "send_letters", - "manage_api_keys", - "view_activity", - ] - ) - == sorted([i.permission for i in permissions]) - ) + assert sorted( + [ + "manage_users", + "manage_templates", + "manage_settings", + "send_texts", + "send_emails", + "send_letters", + "manage_api_keys", + "view_activity", + ] + ) == sorted([i.permission for i in permissions]) def test_get_permissions_by_user_id_returns_only_active_service(notify_db, notify_db_session, sample_user): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index f1bb07f7d8..b210a8dc1a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -3505,7 +3505,9 @@ def test_get_monthly_notification_data_by_service(mocker, admin_request): assert response == [] -def test_suspend_service_bounce_rate_exceeded_email_sent(mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates): +def test_suspend_service_bounce_rate_exceeded_email_sent( + mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates +): mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") admin_request.post( @@ -3545,11 +3547,12 @@ def test_resume_service_user_notification_email_sent(mocker, sample_service, adm assert service.active is True - @pytest.mark.skip( reason="Depends on completion of ADR for bounce rate. The decision regarding how we will fetch and store whether a service has hit or is approaching the bounce rate needs to be made." ) -def test_suspend_service_bounce_rate_not_exceeded_no_email_sent(mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates): +def test_suspend_service_bounce_rate_not_exceeded_no_email_sent( + mocker, sample_service, admin_request, bounce_rate_suspend_resume_templates +): # TODO mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async") diff --git a/tests/app/test_config.py b/tests/app/test_config.py index d47985dcee..9a1ecd8b9e 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -28,32 +28,29 @@ def test_queue_names_all_queues_correct(): # Need to ensure that all_queues() only returns queue names used in API queues = QueueNames.all_queues() assert len(queues) == 17 - assert ( - set( - [ - QueueNames.PRIORITY, - QueueNames.BULK, - QueueNames.PERIODIC, - QueueNames.DATABASE, - QueueNames.PRIORITY_DATABASE, - QueueNames.NORMAL_DATABASE, - QueueNames.BULK_DATABASE, - QueueNames.SEND_SMS, - QueueNames.SEND_THROTTLED_SMS, - QueueNames.SEND_EMAIL, - QueueNames.RESEARCH_MODE, - QueueNames.REPORTING, - QueueNames.JOBS, - QueueNames.RETRY, - QueueNames.NOTIFY, - # QueueNames.CREATE_LETTERS_PDF, - QueueNames.CALLBACKS, - # QueueNames.LETTERS, - QueueNames.DELIVERY_RECEIPTS, - ] - ) - == set(queues) - ) + assert set( + [ + QueueNames.PRIORITY, + QueueNames.BULK, + QueueNames.PERIODIC, + QueueNames.DATABASE, + QueueNames.PRIORITY_DATABASE, + QueueNames.NORMAL_DATABASE, + QueueNames.BULK_DATABASE, + QueueNames.SEND_SMS, + QueueNames.SEND_THROTTLED_SMS, + QueueNames.SEND_EMAIL, + QueueNames.RESEARCH_MODE, + QueueNames.REPORTING, + QueueNames.JOBS, + QueueNames.RETRY, + QueueNames.NOTIFY, + # QueueNames.CREATE_LETTERS_PDF, + QueueNames.CALLBACKS, + # QueueNames.LETTERS, + QueueNames.DELIVERY_RECEIPTS, + ] + ) == set(queues) def test_get_safe_config(mocker, reload_config):