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

@@ -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:

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"

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
                     )

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):

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(

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)

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)

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,

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(
             [