From eb12d390120a1db06adfde0c32062d1875bdacad Mon Sep 17 00:00:00 2001 From: Jumana B Date: Wed, 2 Oct 2024 11:02:49 -0400 Subject: [PATCH] Removed the FF_TEMPLATE_CATEGORY flag (#2314) * Removed the FF_TEMPLATE_CATEGORY flag * fix test * fix test * fix tests * Remove irrelevant letter code that doesn't work with only two process_types --- app/clients/sms/aws_pinpoint.py | 13 +- app/config.py | 1 - app/delivery/send_to_providers.py | 2 +- app/models.py | 11 +- tests/app/clients/test_aws_pinpoint.py | 45 +- tests/app/conftest.py | 26 +- tests/app/dao/test_templates_dao.py | 7 +- tests/app/db.py | 15 + .../test_process_letter_notifications.py | 61 -- .../test_process_notification.py | 26 +- .../service/test_send_one_off_notification.py | 80 --- tests/app/template/test_rest.py | 318 ++++------- .../test_post_letter_notifications.py | 519 ------------------ 13 files changed, 166 insertions(+), 958 deletions(-) delete mode 100644 tests/app/notifications/test_process_letter_notifications.py delete mode 100644 tests/app/v2/notifications/test_post_letter_notifications.py diff --git a/app/clients/sms/aws_pinpoint.py b/app/clients/sms/aws_pinpoint.py index 57c58c9f13..bcd9ac114b 100644 --- a/app/clients/sms/aws_pinpoint.py +++ b/app/clients/sms/aws_pinpoint.py @@ -27,16 +27,9 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id= opted_out = False response = {} - if self.current_app.config["FF_TEMPLATE_CATEGORY"]: - use_shortcode_pool = ( - sending_vehicle == SmsSendingVehicles.SHORT_CODE - or str(service_id) == self.current_app.config["NOTIFY_SERVICE_ID"] - ) - else: - use_shortcode_pool = ( - str(template_id) in self.current_app.config["AWS_PINPOINT_SC_TEMPLATE_IDS"] - or str(service_id) == self.current_app.config["NOTIFY_SERVICE_ID"] - ) + use_shortcode_pool = ( + sending_vehicle == SmsSendingVehicles.SHORT_CODE or str(service_id) == self.current_app.config["NOTIFY_SERVICE_ID"] + ) if use_shortcode_pool: pool_id = self.current_app.config["AWS_PINPOINT_SC_POOL_ID"] else: diff --git a/app/config.py b/app/config.py index f69f9fdeb9..07382c0e60 100644 --- a/app/config.py +++ b/app/config.py @@ -566,7 +566,6 @@ class Config(object): FF_CELERY_CUSTOM_TASK_PARAMS = env.bool("FF_CELERY_CUSTOM_TASK_PARAMS", True) FF_CLOUDWATCH_METRICS_ENABLED = env.bool("FF_CLOUDWATCH_METRICS_ENABLED", False) FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False) - FF_TEMPLATE_CATEGORY = env.bool("FF_TEMPLATE_CATEGORY", False) # SRE Tools auth keys SRE_USER_NAME = "SRE_CLIENT_USER" diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index dfac94cb69..453aceb57d 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -113,7 +113,7 @@ def send_sms_to_provider(notification): else: try: template_category_id = template_dict.get("template_category_id") - if current_app.config["FF_TEMPLATE_CATEGORY"] and template_category_id is not None: + if template_category_id is not None: sending_vehicle = SmsSendingVehicles( dao_get_template_category_by_id(template_category_id).sms_sending_vehicle ) diff --git a/app/models.py b/app/models.py index e0cba92019..8f06e860b8 100644 --- a/app/models.py +++ b/app/models.py @@ -1146,13 +1146,10 @@ def process_type_column(cls): @hybrid_property def process_type(self): - # The if statement is required as a way to check if FF_TEMPLATE_CATEGORY is enabled - if self.template_category_id: - if self.template_type == SMS_TYPE: - return self.process_type_column if self.process_type_column else self.template_category.sms_process_type - elif self.template_type == EMAIL_TYPE: - return self.process_type_column if self.process_type_column else self.template_category.email_process_type - return self.process_type_column if self.process_type_column else NORMAL + if self.template_type == SMS_TYPE: + return self.process_type_column if self.process_type_column else self.template_category.sms_process_type + elif self.template_type == EMAIL_TYPE: + return self.process_type_column if self.process_type_column else self.template_category.email_process_type @process_type.setter # type: ignore def process_type(self, value): diff --git a/tests/app/clients/test_aws_pinpoint.py b/tests/app/clients/test_aws_pinpoint.py index b913b1c39b..a13d7f63d6 100644 --- a/tests/app/clients/test_aws_pinpoint.py +++ b/tests/app/clients/test_aws_pinpoint.py @@ -34,35 +34,6 @@ def test_send_sms_sends_to_default_pool(notify_api, mocker, sample_template, tem ) -@pytest.mark.serial -def test_send_sms_sends_sc_template_to_shortcode_pool_with_ff_false(notify_api, mocker, sample_template): - boto_mock = mocker.patch.object(aws_pinpoint_client, "_client", create=True) - mocker.patch.object(aws_pinpoint_client, "statsd_client", create=True) - to = "6135555555" - content = "foo" - reference = "ref" - - with set_config_values( - notify_api, - { - "AWS_PINPOINT_SC_POOL_ID": "sc_pool_id", - "AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id", - "AWS_PINPOINT_CONFIGURATION_SET_NAME": "config_set_name", - "AWS_PINPOINT_SC_TEMPLATE_IDS": [str(sample_template.id)], - "FF_TEMPLATE_CATEGORY": False, - }, - ): - aws_pinpoint_client.send_sms(to, content, reference=reference, template_id=sample_template.id) - - boto_mock.send_text_message.assert_called_once_with( - DestinationPhoneNumber="+16135555555", - OriginationIdentity="sc_pool_id", - MessageBody=content, - MessageType="TRANSACTIONAL", - ConfigurationSetName="config_set_name", - ) - - @pytest.mark.serial def test_send_sms_sends_notify_sms_to_shortcode_pool(notify_api, mocker, sample_template): boto_mock = mocker.patch.object(aws_pinpoint_client, "_client", create=True) @@ -118,19 +89,14 @@ def test_handles_opted_out_numbers(notify_api, mocker, sample_template): @pytest.mark.serial @pytest.mark.parametrize( - "FF_TEMPLATE_CATEGORY, sending_vehicle, expected_pool", + "sending_vehicle, expected_pool", [ - (False, None, "default_pool_id"), - (False, "long_code", "default_pool_id"), - (False, "short_code", "default_pool_id"), - (True, None, "default_pool_id"), - (True, "long_code", "default_pool_id"), - (True, "short_code", "sc_pool_id"), + (None, "default_pool_id"), + ("long_code", "default_pool_id"), + ("short_code", "sc_pool_id"), ], ) -def test_respects_sending_vehicle_if_FF_enabled( - notify_api, mocker, sample_template, FF_TEMPLATE_CATEGORY, sending_vehicle, expected_pool -): +def test_respects_sending_vehicle_if_FF_enabled(notify_api, mocker, sample_template, sending_vehicle, expected_pool): boto_mock = mocker.patch.object(aws_pinpoint_client, "_client", create=True) mocker.patch.object(aws_pinpoint_client, "statsd_client", create=True) to = "6135555555" @@ -145,7 +111,6 @@ def test_respects_sending_vehicle_if_FF_enabled( "AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id", "AWS_PINPOINT_CONFIGURATION_SET_NAME": "config_set_name", "AWS_PINPOINT_SC_TEMPLATE_IDS": [], - "FF_TEMPLATE_CATEGORY": FF_TEMPLATE_CATEGORY, }, ): aws_pinpoint_client.send_sms( diff --git a/tests/app/conftest.py b/tests/app/conftest.py index b2d6de335d..cb9f1a4601 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -416,7 +416,7 @@ def create_sample_template( if template_category: data["template_category"] = template_category else: - cat = create_template_category(notify_db, notify_db_session, name_en=str(uuid.uuid4), name_fr=str(uuid.uuid4)) + cat = create_template_category(notify_db, notify_db_session, name_en=str(uuid.uuid4()), name_fr=str(uuid.uuid4())) data.update({"template_category_id": cat.id}) template = Template(**data) dao_create_template(template) @@ -523,7 +523,12 @@ def create_sample_email_template( subject_line="Email Subject", service=None, permissions=[EMAIL_TYPE, SMS_TYPE], + template_category=None, ): + if not template_category: + template_category = create_template_category( + notify_db, notify_db_session, name_en=str(uuid.uuid4()), name_fr=str(uuid.uuid4()) + ) if user is None: user = create_user() if service is None: @@ -539,6 +544,7 @@ def create_sample_email_template( "service": service, "created_by": user, "subject": subject_line, + "template_category_id": template_category.id, } template = Template(**data) dao_create_template(template) @@ -1424,8 +1430,21 @@ def contact_form_email_template(notify_db, notify_db_session): ) -def create_custom_template(service, user, template_config_name, template_type, content="", subject=None): - template = Template.query.get(current_app.config[template_config_name]) +def create_custom_template( + service, + user, + template_config_name, + template_type, + content="", + subject=None, + template_category=None, +): + id = current_app.config[template_config_name] + template = Template.query.get(id) + if not template_category: + template_category = create_template_category(db, db.session, name_en=str(uuid.uuid4()), name_fr=str(uuid.uuid4())) + if template: + template.template_category_id = template_category.id if not template: data = { "id": current_app.config[template_config_name], @@ -1436,6 +1455,7 @@ def create_custom_template(service, user, template_config_name, template_type, c "created_by": user, "subject": subject, "archived": False, + "template_category_id": template_category.id, } template = Template(**data) db.session.add(template) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 5c23083155..e47b273644 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -29,18 +29,18 @@ [ ("sms", None, False), ("email", "subject", False), - ("letter", "subject", False), ("sms", None, True), ("email", "subject", True), ], ) -def test_create_template(sample_service, sample_user, template_type, subject, redact_personalisation): +def test_create_template(sample_service, sample_user, template_type, subject, redact_personalisation, sample_template_category): data = { "name": "Sample Template", "template_type": template_type, "content": "Template content", "service": sample_service, "created_by": sample_user, + "template_category_id": sample_template_category.id, } if template_type == "letter": data["postage"] = "second" @@ -517,9 +517,6 @@ def test_process_type_set_correctly(self, sample_service, sample_user, sample_te } # We are not setting the template process_type, hence process_type defaults to "normal", but the value in the DB is empty template = Template(**data) - dao_create_template(template) - assert template.process_type == "normal" - assert template.process_type_column is None template.process_type = "priority" dao_update_template(template) diff --git a/tests/app/db.py b/tests/app/db.py index 7cc8f95491..53217bffe0 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -26,6 +26,7 @@ update_existing_sms_sender_with_inbound_number, ) from app.dao.services_dao import dao_add_user_to_service, dao_create_service +from app.dao.template_categories_dao import dao_create_template_category from app.dao.templates_dao import dao_create_template, dao_update_template from app.dao.users_dao import save_model_user from app.models import ( @@ -62,6 +63,7 @@ ServicePermission, ServiceSmsSender, Template, + TemplateCategory, TemplateFolder, User, ) @@ -194,6 +196,19 @@ def create_template( postage=None, process_type="normal", ): + if not template_category: + data = { + "name_en": str(uuid.uuid4()), + "name_fr": str(uuid.uuid4()), + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + "created_at": datetime.utcnow(), + "updated_at": datetime.utcnow(), + "sms_sending_vehicle": "long_code", + } + template_category = TemplateCategory(**data) + template_category = dao_create_template_category(template_category) data = { "name": template_name or "{} Template Name".format(template_type), "template_type": template_type, diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py deleted file mode 100644 index 1273a1b13a..0000000000 --- a/tests/app/notifications/test_process_letter_notifications.py +++ /dev/null @@ -1,61 +0,0 @@ -from app.models import LETTER_TYPE, NOTIFICATION_CREATED, Notification -from app.notifications.process_letter_notifications import create_letter_notification - - -def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key): - data = { - "personalisation": { - "address_line_1": "The Queen", - "address_line_2": "Buckingham Palace", - "postcode": "SW1 1AA", - } - } - - notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED) - - assert notification == Notification.query.one() - assert notification.job is None - assert notification.status == NOTIFICATION_CREATED - assert notification.template_id == sample_letter_template.id - assert notification.template_version == sample_letter_template.version - assert notification.api_key == sample_api_key - assert notification.notification_type == LETTER_TYPE - assert notification.key_type == sample_api_key.key_type - assert notification.reference is not None - assert notification.client_reference is None - assert notification.postage == "second" - - -def test_create_letter_notification_sets_reference(sample_letter_template, sample_api_key): - data = { - "personalisation": { - "address_line_1": "The Queen", - "address_line_2": "Buckingham Palace", - "postcode": "SW1 1AA", - }, - "reference": "foo", - } - - notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED) - - assert notification.client_reference == "foo" - - -def test_create_letter_notification_sets_billable_units(sample_letter_template, sample_api_key): - data = { - "personalisation": { - "address_line_1": "The Queen", - "address_line_2": "Buckingham Palace", - "postcode": "SW1 1AA", - }, - } - - notification = create_letter_notification( - data, - sample_letter_template, - sample_api_key, - NOTIFICATION_CREATED, - billable_units=3, - ) - - assert notification.billable_units == 3 diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index eaa0c00da0..432ecb72d6 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -16,7 +16,6 @@ from app.dao.service_sms_sender_dao import dao_update_service_sms_sender from app.models import ( BULK, - LETTER_TYPE, NORMAL, PRIORITY, ApiKey, @@ -38,7 +37,7 @@ ) from app.v2.errors import BadRequestError from tests.app.conftest import create_sample_api_key -from tests.app.db import create_service, create_service_sms_sender, create_template +from tests.app.db import create_service_sms_sender from tests.conftest import set_config @@ -374,29 +373,6 @@ def test_persist_email_notifications_stores_normalised_email( assert persisted_notification.to == recipient assert persisted_notification.normalised_to == expected_recipient_normalised - def test_persist_notification_with_billable_units_stores_correct_info(self, mocker, notify_db_session): - service = create_service(service_permissions=[LETTER_TYPE]) - template = create_template(service, template_type=LETTER_TYPE) - mocker.patch("app.dao.templates_dao.dao_get_template_by_id", return_value=template) - persist_notifications( - [ - dict( - template_id=template.id, - template_version=template.version, - recipient="123 Main Street", - service=template.service, - personalisation=None, - notification_type=template.template_type, - api_key_id=None, - key_type="normal", - billable_units=3, - template_postage=template.postage, - ) - ] - ) - persisted_notification = Notification.query.all()[0] - assert persisted_notification.billable_units == 3 - def test_persist_notifications_list(self, sample_job, sample_api_key, notify_db_session): persist_notifications( [ diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index b9dc37cfd5..3782d0d8e1 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -10,7 +10,6 @@ from app.models import ( EMAIL_TYPE, KEY_TYPE_NORMAL, - LETTER_TYPE, MOBILE_TYPE, SMS_TYPE, Notification, @@ -23,7 +22,6 @@ LiveServiceTooManySMSRequestsError, ) from tests.app.db import ( - create_letter_contact, create_reply_to_email, create_service, create_service_sms_sender, @@ -131,50 +129,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_email(persist_moc ) -def test_send_one_off_notification_calls_persist_correctly_for_letter(mocker, persist_mock, celery_mock, notify_db_session): - mocker.patch( - "app.service.send_notification.create_random_identifier", - return_value="this-is-random-in-real-life", - ) - service = create_service() - template = create_template( - service=service, - template_type=LETTER_TYPE, - postage="first", - subject="Test subject", - content="Hello (( Name))\nYour thing is due soon", - ) - - post_data = { - "template_id": str(template.id), - "to": "First Last", - "personalisation": { - "name": "foo", - "address line 1": "First Last", - "address line 2": "1 Example Street", - "postcode": "SW1A 1AA", - }, - "created_by": str(service.created_by_id), - } - - send_one_off_notification(service.id, post_data) - - persist_mock.assert_called_once_with( - template_id=template.id, - template_version=template.version, - template_postage="first", - recipient=post_data["to"], - service=template.service, - personalisation=post_data["personalisation"], - notification_type=LETTER_TYPE, - api_key_id=None, - key_type=KEY_TYPE_NORMAL, - created_by_id=str(service.created_by_id), - reply_to_text=None, - reference="this-is-random-in-real-life", - ) - - def test_send_one_off_notification_honors_research_mode(notify_db_session, persist_mock, celery_mock): service = create_service(research_mode=True) template = create_template(service=service) @@ -383,40 +337,6 @@ def test_send_one_off_notification_should_add_email_reply_to_text_for_notificati assert notification.reply_to_text == reply_to_email.email_address -def test_send_one_off_letter_notification_should_use_template_reply_to_text(sample_letter_template, celery_mock): - letter_contact = create_letter_contact(sample_letter_template.service, "Edinburgh, ED1 1AA", is_default=False) - sample_letter_template.reply_to = str(letter_contact.id) - - data = { - "to": "user@example.com", - "template_id": str(sample_letter_template.id), - "created_by": str(sample_letter_template.service.created_by_id), - } - - notification_id = send_one_off_notification(service_id=sample_letter_template.service.id, post_data=data) - notification = Notification.query.get(notification_id["id"]) - celery_mock.assert_called_once_with(notification=notification, research_mode=False, queue=QueueNames.NORMAL) - - assert notification.reply_to_text == "Edinburgh, ED1 1AA" - - -def test_send_one_off_letter_should_not_make_pdf_in_research_mode( - sample_letter_template, -): - sample_letter_template.service.research_mode = True - - data = { - "to": "A. Name", - "template_id": str(sample_letter_template.id), - "created_by": str(sample_letter_template.service.created_by_id), - } - - notification = send_one_off_notification(service_id=sample_letter_template.service.id, post_data=data) - notification = Notification.query.get(notification["id"]) - - assert notification.status == "delivered" - - def test_send_one_off_sms_notification_should_use_sms_sender_reply_to_text(sample_service, celery_mock): template = create_template(service=sample_service, template_type=SMS_TYPE) sms_sender = create_service_sms_sender(service=sample_service, sms_sender="6502532222", is_default=False) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 9121583f84..0e0bc9a4fe 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -43,10 +43,9 @@ [ (SMS_TYPE, None), (EMAIL_TYPE, "subject"), - (LETTER_TYPE, "subject"), ], ) -def test_should_create_a_new_template_for_a_service(client, sample_user, template_type, subject): +def test_should_create_a_new_template_for_a_service(client, sample_user, template_type, subject, sample_template_category): service = create_service(service_permissions=[template_type]) data = { "name": "my template", @@ -54,6 +53,7 @@ def test_should_create_a_new_template_for_a_service(client, sample_user, templat "content": "template content", "service": str(service.id), "created_by": str(sample_user.id), + "template_category_id": str(sample_template_category.id), } if subject: data.update({"subject": subject}) @@ -77,6 +77,7 @@ def test_should_create_a_new_template_for_a_service(client, sample_user, templat assert json_resp["data"]["version"] == 1 assert json_resp["data"]["process_type"] == "normal" assert json_resp["data"]["created_by"] == str(sample_user.id) + assert json_resp["data"]["template_category_id"] == str(sample_template_category.id) if subject: assert json_resp["data"]["subject"] == "subject" else: @@ -549,11 +550,6 @@ def test_should_get_only_templates_for_that_service(admin_request, notify_db_ses EMAIL_TYPE, ), (None, "hello ((name)) we’ve received your ((thing))", SMS_TYPE), - ( - "about your ((thing))", - "hello ((name)) we’ve received your ((thing))", - LETTER_TYPE, - ), ], ) def test_should_get_a_single_template(notify_db, client, sample_user, service_factory, subject, content, template_type): @@ -1606,112 +1602,25 @@ def test_process_type_should_be_reset_when_template_category_updated( expected_process_type, notify_api, ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": "true"}): # TODO remove statement when FF removed - template_orig = dao_get_template_by_id(sample_template_with_priority_override.id) - - calculated_tc = ( - template_category_id if template_category_id != "unchanged" else str(template_orig.template_category_id) - ) - admin_request.post( - "template.update_template", - service_id=sample_template_with_priority_override.service_id, - template_id=sample_template_with_priority_override.id, - _data={ - "template_category_id": calculated_tc, - "redact_personalisation": False, - "process_type": data_process_type, - }, - _expected_status=200, - ) - template = dao_get_template_by_id(sample_template_with_priority_override.id) - - assert str(template.template_category_id) == calculated_tc - assert template.process_type_column == expected_process_type_column - assert template.process_type == expected_process_type - - # TODO remove TEST when FF removed - @pytest.mark.parametrize( - "template_type, process_type", - [ - (SMS_TYPE, "bulk"), - (EMAIL_TYPE, "bulk"), - (SMS_TYPE, "normal"), - (EMAIL_TYPE, "normal"), - (SMS_TYPE, "priority"), - (EMAIL_TYPE, "priority"), - ], - ) - def test_update_template_override_process_type_ff_off( - self, admin_request, sample_user, notify_api, template_type, process_type - ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": False}): - service = create_service(service_name="service_1") - template = create_template( - service, - template_type=template_type, - template_name="testing template", - subject="Template subject", - content="Dear Sir/Madam, Hello. Yours Truly, The Government.", - template_category=None, - process_type="normal", - ) - - template_data = { - "id": str(template.id), - "name": "new name", - "template_type": template_type, - "content": "some content here :)", - "service": str(service.id), - "created_by": str(sample_user.id), - "template_category_id": None, - "process_type": process_type, - } - - response = admin_request.post( - "template.update_template", - service_id=service.id, - template_id=template.id, - _data=template_data, - _expected_status=200, - ) - assert response["data"]["process_type"] == process_type - assert response["data"]["template_category"] is None + template_orig = dao_get_template_by_id(sample_template_with_priority_override.id) - # TODO remove TEST when FF removed - @pytest.mark.parametrize( - "template_type, process_type", - [ - (SMS_TYPE, "bulk"), - (EMAIL_TYPE, "bulk"), - (SMS_TYPE, "normal"), - (EMAIL_TYPE, "normal"), - (SMS_TYPE, "priority"), - (EMAIL_TYPE, "priority"), - ], - ) - def test_create_template_default_process_type_ff_off( - self, admin_request, sample_user, notify_api, template_type, process_type - ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": False}): - service = create_service(service_name="service_1") - - template_data = { - "name": "new name", - "template_type": template_type, - "content": "some content here :)", - "subject": "yo", - "service": str(service.id), - "created_by": str(sample_user.id), - "template_category_id": None, - "process_type": process_type, - } - - response = admin_request.post( - "template.create_template", service_id=service.id, _data=template_data, _expected_status=201 - ) + calculated_tc = template_category_id if template_category_id != "unchanged" else str(template_orig.template_category_id) + admin_request.post( + "template.update_template", + service_id=sample_template_with_priority_override.service_id, + template_id=sample_template_with_priority_override.id, + _data={ + "template_category_id": calculated_tc, + "redact_personalisation": False, + "process_type": data_process_type, + }, + _expected_status=200, + ) + template = dao_get_template_by_id(sample_template_with_priority_override.id) - assert response["data"]["process_type"] == process_type - assert response["data"]["template_category"] is None + assert str(template.template_category_id) == calculated_tc + assert template.process_type_column == expected_process_type_column + assert template.process_type == expected_process_type @pytest.mark.parametrize( "template_type, initial_process_type, updated_process_type", @@ -1740,38 +1649,37 @@ def test_update_template_override_process_type_ff_on( initial_process_type, updated_process_type, ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": True}): - service = create_service(service_name="service_1") - template = create_template( - service, - template_type=template_type, - template_name="testing template", - subject="Template subject", - content="Dear Sir/Madam, Hello. Yours Truly, The Government.", - template_category=sample_template_category, - process_type=initial_process_type, - ) + service = create_service(service_name="service_1") + template = create_template( + service, + template_type=template_type, + template_name="testing template", + subject="Template subject", + content="Dear Sir/Madam, Hello. Yours Truly, The Government.", + template_category=sample_template_category, + process_type=initial_process_type, + ) - template_data = { - "id": str(template.id), - "name": "new name", - "template_type": template_type, - "content": "some content here :)", - "service": str(service.id), - "created_by": str(sample_user.id), - "template_category_id": str(sample_template_category.id), - "process_type": updated_process_type, - } + template_data = { + "id": str(template.id), + "name": "new name", + "template_type": template_type, + "content": "some content here :)", + "service": str(service.id), + "created_by": str(sample_user.id), + "template_category_id": str(sample_template_category.id), + "process_type": updated_process_type, + } - response = admin_request.post( - "template.update_template", - service_id=service.id, - template_id=template.id, - _data=template_data, - _expected_status=200, - ) - assert response["data"]["process_type"] == updated_process_type - assert response["data"]["template_category"]["id"] == str(sample_template_category.id) + response = admin_request.post( + "template.update_template", + service_id=service.id, + template_id=template.id, + _data=template_data, + _expected_status=200, + ) + assert response["data"]["process_type"] == updated_process_type + assert response["data"]["template_category"]["id"] == str(sample_template_category.id) @pytest.mark.parametrize( "template_type, process_type, template_category", @@ -1814,48 +1722,47 @@ def test_update_template_change_category_ff_on( sample_template_category_priority, sample_template_category_bulk, ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": True}): - service = create_service(service_name="service_1") - template = create_template( - service, - template_type=template_type, - template_name="testing template", - subject="Template subject", - content="Dear Sir/Madam, Hello. Yours Truly, The Government.", - template_category=sample_template_category, - process_type=process_type, - ) + service = create_service(service_name="service_1") + template = create_template( + service, + template_type=template_type, + template_name="testing template", + subject="Template subject", + content="Dear Sir/Madam, Hello. Yours Truly, The Government.", + template_category=sample_template_category, + process_type=process_type, + ) + tc = sample_template_category + if template_category == "normal": tc = sample_template_category - if template_category == "normal": - tc = sample_template_category - elif template_category == "bulk": - tc = sample_template_category_bulk - elif template_category == "priority": - tc = sample_template_category_priority - - template_data = { - "name": "new name", - "template_type": template_type, - "content": "some content here :)", - "subject": "yo", - "service": str(service.id), - "created_by": str(sample_user.id), - "template_category_id": str(tc.id), - "process_type": process_type, - } + elif template_category == "bulk": + tc = sample_template_category_bulk + elif template_category == "priority": + tc = sample_template_category_priority + + template_data = { + "name": "new name", + "template_type": template_type, + "content": "some content here :)", + "subject": "yo", + "service": str(service.id), + "created_by": str(sample_user.id), + "template_category_id": str(tc.id), + "process_type": process_type, + } - response = admin_request.post( - "template.update_template", - service_id=service.id, - template_id=template.id, - _data=template_data, - _expected_status=200, - ) + response = admin_request.post( + "template.update_template", + service_id=service.id, + template_id=template.id, + _data=template_data, + _expected_status=200, + ) - assert response["data"]["process_type_column"] == process_type - assert response["data"]["process_type"] == template_category if process_type is None else process_type - assert response["data"]["template_category_id"] == str(tc.id) + assert response["data"]["process_type_column"] == process_type + assert response["data"]["process_type"] == template_category if process_type is None else process_type + assert response["data"]["template_category_id"] == str(tc.id) @pytest.mark.parametrize( "template_type, process_type, calculated_process_type", @@ -1886,35 +1793,34 @@ def test_create_template_with_category_ff_on( sample_template_category_priority, sample_template_category_bulk, ): - with set_config_values(notify_api, {"FF_TEMPLATE_CATEGORY": True}): - service = create_service(service_name="service_1") + service = create_service(service_name="service_1") - tc = sample_template_category - if process_type is None: - if calculated_process_type == "normal": - tc = sample_template_category - elif calculated_process_type == "bulk": - tc = sample_template_category_bulk - elif calculated_process_type == "priority": - tc = sample_template_category_priority - else: + tc = sample_template_category + if process_type is None: + if calculated_process_type == "normal": tc = sample_template_category + elif calculated_process_type == "bulk": + tc = sample_template_category_bulk + elif calculated_process_type == "priority": + tc = sample_template_category_priority + else: + tc = sample_template_category - template_data = { - "name": "new name", - "template_type": template_type, - "content": "some content here :)", - "subject": "yo", - "service": str(service.id), - "created_by": str(sample_user.id), - "template_category_id": str(tc.id), - "process_type": process_type, - } + template_data = { + "name": "new name", + "template_type": template_type, + "content": "some content here :)", + "subject": "yo", + "service": str(service.id), + "created_by": str(sample_user.id), + "template_category_id": str(tc.id), + "process_type": process_type, + } - response = admin_request.post( - "template.create_template", service_id=service.id, _data=template_data, _expected_status=201 - ) + response = admin_request.post( + "template.create_template", service_id=service.id, _data=template_data, _expected_status=201 + ) - assert response["data"]["process_type_column"] == process_type - assert response["data"]["process_type"] == calculated_process_type - assert response["data"]["template_category_id"] == str(tc.id) + assert response["data"]["process_type_column"] == process_type + assert response["data"]["process_type"] == calculated_process_type + assert response["data"]["template_category_id"] == str(tc.id) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py deleted file mode 100644 index 1a01439f82..0000000000 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ /dev/null @@ -1,519 +0,0 @@ -import uuid -from unittest.mock import ANY - -import pytest -from flask import json, url_for - -from app.config import QueueNames -from app.models import ( - EMAIL_TYPE, - KEY_TYPE_NORMAL, - KEY_TYPE_TEAM, - KEY_TYPE_TEST, - LETTER_TYPE, - NOTIFICATION_CREATED, - NOTIFICATION_DELIVERED, - NOTIFICATION_PENDING_VIRUS_CHECK, - NOTIFICATION_SENDING, - SMS_TYPE, - Job, - Notification, -) -from app.schema_validation import validate -from app.v2.errors import RateLimitError -from app.v2.notifications.notification_schemas import post_letter_response -from tests import create_authorization_header -from tests.app.db import create_letter_contact, create_service, create_template -from tests.conftest import set_config_values - -test_address = { - "address_line_1": "test 1", - "address_line_2": "test 2", - "postcode": "test pc", -} - - -def letter_request( - client, - data, - service_id, - key_type=KEY_TYPE_NORMAL, - _expected_status=201, - precompiled=False, -): - if precompiled: - url = url_for("v2_notifications.post_precompiled_letter_notification") - else: - url = url_for("v2_notifications.post_notification", notification_type=LETTER_TYPE) - resp = client.post( - url, - data=json.dumps(data), - headers=[ - ("Content-Type", "application/json"), - create_authorization_header(service_id=service_id, key_type=key_type), - ], - ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == _expected_status, json_resp - return json_resp - - -@pytest.mark.skip(reason="Deprecated: LETTER CODE") -@pytest.mark.parametrize("reference", [None, "reference_from_client"]) -def test_post_letter_notification_returns_201(client, sample_letter_template, mocker, reference): - mock = mocker.patch("app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async") - data = { - "template_id": str(sample_letter_template.id), - "personalisation": { - "address_line_1": "Her Royal Highness Queen Elizabeth II", - "address_line_2": "Buckingham Palace", - "address_line_3": "London", - "postcode": "SW1 1AA", - "name": "Lizzie", - }, - } - - if reference: - data.update({"reference": reference}) - - resp_json = letter_request(client, data, service_id=sample_letter_template.service_id) - - assert validate(resp_json, post_letter_response) == resp_json - assert Job.query.count() == 0 - notification = Notification.query.one() - assert notification.status == NOTIFICATION_CREATED - assert resp_json["id"] == str(notification.id) - assert resp_json["reference"] == reference - assert resp_json["content"]["subject"] == sample_letter_template.subject - assert resp_json["content"]["body"] == sample_letter_template.content - assert "v2/notifications/{}".format(notification.id) in resp_json["uri"] - assert resp_json["template"]["id"] == str(sample_letter_template.id) - assert resp_json["template"]["version"] == sample_letter_template.version - assert ( - "services/{}/templates/{}".format(sample_letter_template.service_id, sample_letter_template.id) - in resp_json["template"]["uri"] - ) - assert not resp_json["scheduled_for"] - assert not notification.reply_to_text - mock.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF) - - -@pytest.mark.skip(reason="Deprecated: LETTER CODE") -def test_post_letter_notification_sets_postage(client, notify_db_session, mocker): - service = create_service(service_permissions=[LETTER_TYPE]) - template = create_template(service, template_type="letter", postage="first") - mocker.patch("app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async") - data = { - "template_id": str(template.id), - "personalisation": { - "address_line_1": "Her Royal Highness Queen Elizabeth II", - "address_line_2": "Buckingham Palace", - "address_line_3": "London", - "postcode": "SW1 1AA", - "name": "Lizzie", - }, - } - - resp_json = letter_request(client, data, service_id=service.id) - - assert validate(resp_json, post_letter_response) == resp_json - notification = Notification.query.one() - assert notification.postage == "first" - - -@pytest.mark.parametrize( - "env", - [ - "staging", - "live", - ], -) -def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_delivered( - notify_api, client, sample_letter_template, mocker, env -): - data = { - "template_id": str(sample_letter_template.id), - "personalisation": { - "address_line_1": "Her Royal Highness Queen Elizabeth II", - "address_line_2": "Buckingham Palace", - "address_line_3": "London", - "postcode": "SW1 1AA", - "name": "Lizzie", - }, - "reference": "foo", - } - - fake_create_letter_task = mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - fake_create_dvla_response_task = mocker.patch("app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async") - - with set_config_values(notify_api, {"NOTIFY_ENVIRONMENT": env}): - letter_request( - client, - data, - service_id=sample_letter_template.service_id, - key_type=KEY_TYPE_TEST, - ) - - notification = Notification.query.one() - - fake_create_letter_task.assert_called_once_with([str(notification.id)], queue="research-mode-tasks") - assert not fake_create_dvla_response_task.called - assert notification.status == NOTIFICATION_DELIVERED - - -@pytest.mark.parametrize( - "env", - [ - "development", - "preview", - ], -) -def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_sending_and_sends_fake_response_file( - notify_api, client, sample_letter_template, mocker, env -): - data = { - "template_id": str(sample_letter_template.id), - "personalisation": { - "address_line_1": "Her Royal Highness Queen Elizabeth II", - "address_line_2": "Buckingham Palace", - "address_line_3": "London", - "postcode": "SW1 1AA", - "name": "Lizzie", - }, - "reference": "foo", - } - - fake_create_letter_task = mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - fake_create_dvla_response_task = mocker.patch("app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async") - - with set_config_values(notify_api, {"NOTIFY_ENVIRONMENT": env}): - letter_request( - client, - data, - service_id=sample_letter_template.service_id, - key_type=KEY_TYPE_TEST, - ) - - notification = Notification.query.one() - - fake_create_letter_task.assert_called_once_with([str(notification.id)], queue="research-mode-tasks") - assert fake_create_dvla_response_task.called - assert notification.status == NOTIFICATION_SENDING - - -def test_post_letter_notification_returns_400_and_missing_template(client, sample_service_full_permissions): - data = {"template_id": str(uuid.uuid4()), "personalisation": test_address} - - error_json = letter_request( - client, - data, - service_id=sample_service_full_permissions.id, - _expected_status=400, - ) - - assert error_json["status_code"] == 400 - assert error_json["errors"] == [{"error": "BadRequestError", "message": "Template not found"}] - - -def test_post_letter_notification_returns_400_for_empty_personalisation( - client, sample_service_full_permissions, sample_letter_template -): - data = { - "template_id": str(sample_letter_template.id), - "personalisation": {"address_line_1": "", "address_line_2": "", "postcode": ""}, - } - - error_json = letter_request( - client, - data, - service_id=sample_service_full_permissions.id, - _expected_status=400, - ) - - assert error_json["status_code"] == 400 - assert all([e["error"] == "ValidationError" for e in error_json["errors"]]) - assert set([e["message"] for e in error_json["errors"]]) == { - "personalisation address_line_1 is required", - "personalisation address_line_2 is required", - "personalisation postcode is required", - } - - -def test_notification_returns_400_for_missing_template_field(client, sample_service_full_permissions): - data = {"personalisation": test_address} - - error_json = letter_request( - client, - data, - service_id=sample_service_full_permissions.id, - _expected_status=400, - ) - - assert error_json["status_code"] == 400 - assert error_json["errors"] == [{"error": "ValidationError", "message": "template_id is a required property"}] - - -def test_notification_returns_400_if_address_doesnt_have_underscores(client, sample_letter_template): - data = { - "template_id": str(sample_letter_template.id), - "personalisation": { - "address line 1": "Her Royal Highness Queen Elizabeth II", - "address-line-2": "Buckingham Palace", - "postcode": "SW1 1AA", - }, - } - - error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=400) - - assert error_json["status_code"] == 400 - assert len(error_json["errors"]) == 2 - assert { - "error": "ValidationError", - "message": "personalisation address_line_1 is a required property", - } in error_json["errors"] - assert { - "error": "ValidationError", - "message": "personalisation address_line_2 is a required property", - } in error_json["errors"] - - -def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(client, sample_letter_template, mocker): - persist_mock = mocker.patch("app.notifications.process_notifications.persist_notification") - mocker.patch( - "app.v2.notifications.post_notifications.check_rate_limiting", - side_effect=RateLimitError("LIMIT", "INTERVAL", "TYPE"), - ) - - data = { - "template_id": str(sample_letter_template.id), - "personalisation": test_address, - } - - error_json = letter_request(client, data, service_id=sample_letter_template.service_id, _expected_status=429) - - assert error_json["status_code"] == 429 - assert error_json["errors"] == [ - { - "error": "RateLimitError", - "message": "Exceeded rate limit for key type TYPE of LIMIT requests per INTERVAL seconds", - } - ] - - assert not persist_mock.called - - -@pytest.mark.parametrize( - "service_args", - [{"service_permissions": [EMAIL_TYPE, SMS_TYPE]}, {"restricted": True}], -) -def test_post_letter_notification_returns_403_if_not_allowed_to_send_notification(client, notify_db_session, service_args): - service = create_service(**service_args) - template = create_template(service, template_type=LETTER_TYPE) - - data = {"template_id": str(template.id), "personalisation": test_address} - - error_json = letter_request(client, data, service_id=service.id, _expected_status=400) - assert error_json["status_code"] == 400 - assert error_json["errors"] == [ - { - "error": "BadRequestError", - "message": "Service is not allowed to send letters", - } - ] - - -def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_template, mocker): - mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - data = { - "template_id": str(sample_letter_template.id), - "personalisation": { - "address_line_1": "Foo", - "address_line_2": "Bar", - "postcode": "Baz", - }, - } - - error_json = letter_request( - client, - data, - sample_letter_template.service_id, - key_type=KEY_TYPE_TEAM, - _expected_status=403, - ) - - assert error_json["status_code"] == 403 - assert error_json["errors"] == [ - { - "error": "BadRequestError", - "message": "Cannot send letters with a team api key", - } - ] - - -def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_letter_template, mocker): - mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - data = { - "template_id": str(sample_trial_letter_template.id), - "personalisation": { - "address_line_1": "Foo", - "address_line_2": "Bar", - "postcode": "Baz", - }, - } - - error_json = letter_request(client, data, sample_trial_letter_template.service_id, _expected_status=403) - - assert error_json["status_code"] == 403 - assert error_json["errors"] == [ - { - "error": "BadRequestError", - "message": "Cannot send letters when service is in trial mode", - } - ] - - -def test_post_letter_notification_is_delivered_but_still_creates_pdf_if_in_trial_mode_and_using_test_key( - client, sample_trial_letter_template, mocker -): - fake_create_letter_task = mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - - data = { - "template_id": sample_trial_letter_template.id, - "personalisation": { - "address_line_1": "Foo", - "address_line_2": "Bar", - "postcode": "Baz", - }, - } - - letter_request( - client, - data=data, - service_id=sample_trial_letter_template.service_id, - key_type=KEY_TYPE_TEST, - ) - - notification = Notification.query.one() - assert notification.status == NOTIFICATION_DELIVERED - fake_create_letter_task.assert_called_once_with([str(notification.id)], queue="research-mode-tasks") - - -def test_post_letter_notification_is_delivered_and_has_pdf_uploaded_to_test_letters_bucket_using_test_key( - client, notify_user, mocker -): - sample_letter_service = create_service(service_permissions=["letter"]) - mocker.patch("app.celery.letters_pdf_tasks.notify_celery.send_task") - s3mock = mocker.patch( - "app.v2.notifications.post_notifications.upload_letter_pdf", - return_value="test.pdf", - ) - data = {"reference": "letter-reference", "content": "bGV0dGVyLWNvbnRlbnQ="} - letter_request( - client, - data=data, - service_id=str(sample_letter_service.id), - key_type=KEY_TYPE_TEST, - precompiled=True, - ) - - notification = Notification.query.one() - assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK - s3mock.assert_called_once_with(ANY, b"letter-content", precompiled=True) - - -def test_post_letter_notification_persists_notification_reply_to_text(client, notify_db_session, mocker): - mocker.patch("app.celery.letters_pdf_tasks.create_letters_pdf.apply_async") - - service = create_service(service_permissions=[LETTER_TYPE]) - service_address = "12 Main Street, London" - letter_contact = create_letter_contact(service=service, contact_block=service_address, is_default=True) - template = create_template(service=service, template_type="letter", reply_to=letter_contact.id) - data = { - "template_id": template.id, - "personalisation": { - "address_line_1": "Foo", - "address_line_2": "Bar", - "postcode": "Baz", - }, - } - letter_request(client, data=data, service_id=service.id, key_type=KEY_TYPE_NORMAL) - - notifications = Notification.query.all() - assert len(notifications) == 1 - assert notifications[0].reply_to_text == service_address - - -def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker): - sample_service = create_service(service_permissions=["letter"]) - mocker.patch("app.v2.notifications.post_notifications.upload_letter_pdf") - - data = {"reference": "letter-reference", "content": "hi"} - auth_header = create_authorization_header(service_id=sample_service.id) - response = client.post( - path="v2/notifications/letter", - data=json.dumps(data), - headers=[("Content-Type", "application/json"), auth_header], - ) - - assert response.status_code == 400, response.get_data(as_text=True) - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json["errors"][0]["message"] == "Cannot decode letter content (invalid base64 encoding)" - - assert not Notification.query.first() - - -@pytest.mark.parametrize( - "notification_postage, expected_postage", - [("second", "second"), ("first", "first"), (None, "second")], -) -def test_post_precompiled_letter_notification_returns_201(client, notify_user, mocker, notification_postage, expected_postage): - sample_service = create_service(service_permissions=["letter"]) - s3mock = mocker.patch("app.v2.notifications.post_notifications.upload_letter_pdf") - mocker.patch("app.celery.letters_pdf_tasks.notify_celery.send_task") - data = {"reference": "letter-reference", "content": "bGV0dGVyLWNvbnRlbnQ="} - if notification_postage: - data["postage"] = notification_postage - auth_header = create_authorization_header(service_id=sample_service.id) - response = client.post( - path="v2/notifications/letter", - data=json.dumps(data), - headers=[("Content-Type", "application/json"), auth_header], - ) - - assert response.status_code == 201, response.get_data(as_text=True) - - s3mock.assert_called_once_with(ANY, b"letter-content", precompiled=True) - - notification = Notification.query.one() - - assert notification.billable_units == 0 - assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK - assert notification.postage == expected_postage - - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json == { - "id": str(notification.id), - "reference": "letter-reference", - "postage": expected_postage, - } - - -def test_post_letter_notification_throws_error_for_invalid_postage(client, notify_user, mocker): - sample_service = create_service(service_permissions=["letter"]) - data = { - "reference": "letter-reference", - "content": "bGV0dGVyLWNvbnRlbnQ=", - "postage": "space unicorn", - } - auth_header = create_authorization_header(service_id=sample_service.id) - response = client.post( - path="v2/notifications/letter", - data=json.dumps(data), - headers=[("Content-Type", "application/json"), auth_header], - ) - - assert response.status_code == 400, response.get_data(as_text=True) - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json["errors"][0]["message"] == "postage invalid. It must be either first or second." - - assert not Notification.query.first()