From 4ef95ad5bc36cde8f87009e559b6a8749ecd8340 Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Tue, 9 Jul 2024 16:01:38 -0400 Subject: [PATCH] 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 | 6 +++--- app/models.py | 10 ++++++---- app/schemas.py | 2 ++ app/template/template_category_rest.py | 5 ++--- tests/app/conftest.py | 12 ++++++------ tests/app/dao/test_template_categories_dao.py | 10 +++++----- tests/app/db.py | 4 ++-- tests/app/template/test_rest.py | 2 +- tests/app/template/test_template_category_rest.py | 13 ++++++++----- tests/app/test_model.py | 4 ++-- 10 files changed, 37 insertions(+), 31 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index f38586e899..1215f1a523 100644 --- a/app/dao/template_categories_dao.py +++ b/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): diff --git a/app/models.py b/app/models.py index 81e9d33f26..2a4b23c0de 100644 --- a/app/models.py +++ b/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( diff --git a/app/schemas.py b/app/schemas.py index 644ffda597..e66c2cc73d 100644 --- a/app/schemas.py +++ b/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() diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index 76d9cbc1ba..f987ef3243 100644 --- a/app/template/template_category_rest.py +++ b/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) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 3c21efc8ab..8013ac5be8 100644 --- a/tests/app/conftest.py +++ b/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], ) diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index b9d242a516..07681618ea 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/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 diff --git a/tests/app/db.py b/tests/app/db.py index 30d265d1a1..f06478ab54 100644 --- a/tests/app/db.py +++ b/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: diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 77c034af2c..603d0b9d85 100644 --- a/tests/app/template/test_rest.py +++ b/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 diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index b20528bada..9cb686bc6e 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/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", diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 52e4abcbf6..5e3df7387f 100644 --- a/tests/app/test_model.py +++ b/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