Update Template and TemplateCategory models (#2212)

* Update Template and TemplateCategory models

- Template.category is now Template.template_category
- In Template and TemplateHistory, template_category now leverage a primary join to both allow the full TemplateCategory model to be loaded when fetching either, and to prevent a list of associated templates from being populated on a TemplateCategory model
- Updated the Template and TemplateHistory schemas to include the TemplateCategory when returning data to API callers

* formatting

* Remove unused variable

* Address comments

- Restored @transactional decorator
- Simplified setting the default template categories during a delete

app/dao/template_categories_dao.py

@@ -20,7 +20,7 @@ def dao_get_template_category_by_id(template_category_id) -> TemplateCategory:
 
 
 def dao_get_template_category_by_template_id(template_id) -> TemplateCategory:
-    return Template.query.filter_by(id=template_id).one().category
+    return Template.query.filter_by(id=template_id).one().template_category
 
 
 # TODO: Add filters: Select all template categories used by at least 1 sms/email template
@@ -69,12 +69,12 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False):
                     if template.template_type == "sms"
                     else template_category.email_process_type
                 )
-                template.category = dao_get_template_category_by_id(default_category_id)
+                template.template_category_id = default_category_id
                 template.updated_at = datetime.utcnow()
                 db.session.add(template)
+                db.session.commit()
 
         db.session.delete(template_category)
-        db.session.commit()
 
 
 def _get_default_category_id(process_type):

app/models.py

@@ -1124,8 +1124,8 @@ def template_category_id(cls):
         return db.Column(UUID(as_uuid=True), db.ForeignKey("template_categories.id"), index=True, nullable=True)
 
     @declared_attr
-    def category(cls):
-        return db.relationship("TemplateCategory")
+    def template_category(cls):
+        return db.relationship("TemplateCategory", primaryjoin="Template.template_category_id == TemplateCategory.id")
 
     @declared_attr
     def created_by(cls):
@@ -1228,7 +1228,6 @@ class Template(TemplateBase):
 
     service = db.relationship("Service", backref="templates")
     version = db.Column(db.Integer, default=0, nullable=False)
-    category = db.relationship("TemplateCategory", lazy="joined", backref="templates")
 
     folder = db.relationship(
         "TemplateFolder",
@@ -1307,7 +1306,6 @@ class TemplateHistory(TemplateBase):
 
     service = db.relationship("Service")
     version = db.Column(db.Integer, primary_key=True, nullable=False)
-    category = db.relationship("TemplateCategory")
 
     @classmethod
     def from_json(cls, data):
@@ -1320,6 +1318,10 @@ def from_json(cls, data):
         fields.pop("folder", None)
         return super(TemplateHistory, cls).from_json(fields)
 
+    @declared_attr
+    def template_category(cls):
+        return db.relationship("TemplateCategory", primaryjoin="TemplateHistory.template_category_id == TemplateCategory.id")
+
     @declared_attr
     def template_redacted(cls):
         return db.relationship(

app/schemas.py

@@ -396,6 +396,7 @@ class TemplateSchema(BaseTemplateSchema):
     created_by = field_for(models.Template, "created_by", required=True)
     is_precompiled_letter = fields.Method("get_is_precompiled_letter")
     process_type = field_for(models.Template, "process_type")
+    template_category = fields.Nested(TemplateCategorySchema, dump_only=True)
     redact_personalisation = fields.Method("redact")
     created_at = FlexibleDateTime()
     updated_at = FlexibleDateTime()
@@ -418,6 +419,7 @@ class TemplateHistorySchema(BaseSchema):
     reply_to = fields.Method("get_reply_to", allow_none=True)
     reply_to_text = fields.Method("get_reply_to_text", allow_none=True)
     process_type = field_for(models.Template, "process_type")
+    template_category = fields.Nested(TemplateCategorySchema, dump_only=True)
     created_by = fields.Nested(UserSchema, only=["id", "name", "email_address"], dump_only=True)
     created_at = field_for(models.Template, "created_at", format="%Y-%m-%d %H:%M:%S.%f")
     updated_at = FlexibleDateTime()

app/template/template_category_rest.py

@@ -9,7 +9,7 @@
     dao_update_template_category,
 )
 from app.errors import register_errors
-from app.models import TemplateCategory
+from app.models import Template, TemplateCategory
 from app.schemas import template_category_schema
 
 template_category_blueprint = Blueprint(
@@ -97,8 +97,7 @@ def delete_template_category(template_category_id):
         dao_delete_template_category_by_id(template_category_id, cascade=True)
         return "", 200
 
-    template_category = dao_get_template_category_by_id(template_category_id)
-    if len(template_category.templates) > 0:
+    if Template.query.filter_by(template_category_id=template_category_id).count() > 0:
         return jsonify(message="Cannot delete a template category with templates assigned to it."), 400
     else:
         dao_delete_template_category_by_id(template_category_id)

tests/app/conftest.py

@@ -234,8 +234,8 @@ def _sample_service_custom_letter_contact_block(sample_service):
 
 @pytest.fixture(scope="function")
 def sample_template_category_with_templates(notify_db, notify_db_session, sample_template_category):
-    create_sample_template(notify_db, notify_db_session, category=sample_template_category)
-    create_sample_template(notify_db, notify_db_session, category=sample_template_category)
+    create_sample_template(notify_db, notify_db_session, template_category=sample_template_category)
+    create_sample_template(notify_db, notify_db_session, template_category=sample_template_category)
     return sample_template_category
 
 
@@ -335,7 +335,7 @@ def create_sample_template(
     subject_line="Subject",
     user=None,
     service=None,
-    category=None,
+    template_category=None,
     created_by=None,
     process_type="normal",
     permissions=[EMAIL_TYPE, SMS_TYPE],
@@ -363,8 +363,8 @@ def create_sample_template(
         data.update({"subject": subject_line})
     if template_type == "letter":
         data["postage"] = "second"
-    if category:
-        data["category"] = category
+    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))
         data.update({"template_category_id": cat.id})
@@ -403,7 +403,7 @@ def sample_template(
         service=None,
         created_by=None,
         process_type="normal",
-        category=None,
+        template_category=None,
         permissions=[EMAIL_TYPE, SMS_TYPE],
     )
 

tests/app/dao/test_template_categories_dao.py

@@ -124,7 +124,7 @@ def test_dao_get_template_category_by_template_id(category, template, notify_db_
     template = Template(**template)
     template.service = sample_service
     template.created_by = sample_user
-    template.category = template_category
+    template.template_category = template_category
     dao_create_template(template)
 
     assert dao_get_template_category_by_template_id(template.id) == template_category
@@ -354,8 +354,8 @@ def test_get_all_template_categories_with_filters(
         template_category = TemplateCategory(**category_data)
         dao_create_template_category(template_category)
 
-        create_sample_template(notify_db, notify_db_session, template_type="email", category=template_category)
-        create_sample_template(notify_db, notify_db_session, template_type="sms", category=template_category)
+        create_sample_template(notify_db, notify_db_session, template_type="email", template_category=template_category)
+        create_sample_template(notify_db, notify_db_session, template_type="sms", template_category=template_category)
 
     retrieved_categories = dao_get_all_template_categories(template_type=template_type, hidden=hidden)
 
@@ -373,7 +373,7 @@ def test_dao_delete_template_category_by_id_should_delete_category_when_no_assoc
 def test_dao_delete_template_category_by_id_should_not_allow_deletion_when_associated_with_template(
     notify_db, notify_db_session, sample_template_category
 ):
-    create_sample_template(notify_db, notify_db_session, category=sample_template_category)
+    create_sample_template(notify_db, notify_db_session, template_category=sample_template_category)
 
     dao_delete_template_category_by_id(sample_template_category.id)
 
@@ -383,7 +383,7 @@ def test_dao_delete_template_category_by_id_should_not_allow_deletion_when_assoc
 def test_dao_delete_template_category_by_id_should_allow_deletion_with_cascade_when_associated_with_template(
     notify_db, notify_db_session, sample_template_category, populate_generic_categories
 ):
-    template = create_sample_template(notify_db, notify_db_session, category=sample_template_category)
+    template = create_sample_template(notify_db, notify_db_session, template_category=sample_template_category)
 
     dao_delete_template_category_by_id(sample_template_category.id, cascade=True)
     # 3 here because we have 3 generic defaut categories that will remain post-delete

tests/app/db.py

@@ -188,7 +188,7 @@ def create_template(
     hidden=False,
     archived=False,
     folder=None,
-    category=None,
+    template_category=None,
     postage=None,
     process_type="normal",
 ):
@@ -201,7 +201,7 @@ def create_template(
         "reply_to": reply_to,
         "hidden": hidden,
         "folder": folder,
-        "category": category,
+        "template_category": template_category,
         "process_type": process_type,
     }
     if template_type == LETTER_TYPE:

tests/app/template/test_rest.py

@@ -1571,4 +1571,4 @@ def test_update_templates_category(sample_template, sample_template_category, ad
 
     template = dao_get_template_by_id(sample_template.id)
 
-    assert template.category.id == sample_template_category.id
+    assert template.template_category.id == sample_template_category.id

tests/app/template/test_template_category_rest.py

@@ -52,7 +52,7 @@ def test_get_template_category_by_id(client, sample_template_category):
 
 def test_get_template_category_by_template_id(client, notify_db, notify_db_session, sample_template_category):
     category = sample_template_category
-    template = create_sample_template(notify_db, notify_db_session, category=category)
+    template = create_sample_template(notify_db, notify_db_session, template_category=category)
 
     auth_header = create_authorization_header()
     endpoint = url_for("template_category.get_template_category_by_template_id", template_id=template.id)
@@ -123,12 +123,15 @@ def test_get_template_categories(
     ],
 )
 def test_delete_template_category_cascade(
-    cascade, expected_status_code, expected_msg, client, mocker, sample_template_category_with_templates
+    cascade,
+    expected_status_code,
+    expected_msg,
+    client,
+    mocker,
+    sample_template_category_with_templates,
+    populate_generic_categories,
 ):
     auth_header = create_authorization_header()
-    mocker.patch(
-        "app.dao.template_categories_dao.dao_get_template_category_by_id", return_value=sample_template_category_with_templates
-    )
 
     endpoint = url_for(
         "template_category.delete_template_category",

tests/app/test_model.py

@@ -376,11 +376,11 @@ def test_template_process_type(
     email_process_type,
     expected_template_process_type,
 ):
-    category = create_template_category(
+    template_category = create_template_category(
         notify_db, notify_db_session, sms_process_type=sms_process_type, email_process_type=email_process_type
     )
     template = create_template(
-        service=create_service(), template_type=template_type, process_type=process_type, category=category
+        service=create_service(), template_type=template_type, process_type=process_type, template_category=template_category
     )
 
     assert template.template_process_type == expected_template_process_type