From 4bab29168ca349526e7bc3297948455e072cca63 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 11 Jun 2024 16:04:02 -0400 Subject: [PATCH 01/33] Draft migration to add TemplateCategories table - Initial work on the models --- app/models.py | 28 ++++++++++ .../versions/0454_add_template_category.py | 55 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 migrations/versions/0454_add_template_category.py diff --git a/app/models.py b/app/models.py index f79867918e..920f218de1 100644 --- a/app/models.py +++ b/app/models.py @@ -24,6 +24,7 @@ convert_local_timezone_to_utc, convert_utc_to_local_timezone, ) +from pkg_resources import declare_namespace from sqlalchemy import CheckConstraint, Index, UniqueConstraint from sqlalchemy.dialects.postgresql import JSON, JSONB, UUID from sqlalchemy.ext.associationproxy import association_proxy @@ -1032,6 +1033,18 @@ def get_users_with_permission(self): PRECOMPILED_TEMPLATE_NAME = "Pre-compiled PDF" +class TemplateCategories(BaseModel): + __tablename__ = "template_category" + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + name_en = db.Column(db.String(255), nullable=False) + name_fr = db.Column(db.String(255), nullable=False) + description_en = db.Column(db.String(200), nullable=True) + description_fr = db.Column(db.String(200), nullable=True) + sms_process_type = db.Column(db.String(200), nullable=False) + email_process_type = db.Column(db.String(200), nullable=False) + hidden = db.Column(db.Boolean, nullable=False, default=False) + class TemplateBase(BaseModel): __abstract__ = True @@ -1078,6 +1091,10 @@ def service_id(cls): def created_by_id(cls): return db.Column(UUID(as_uuid=True), db.ForeignKey("users.id"), index=True, nullable=False) + @declared_attr + def template_category_id(cls): + return db.Column(UUID(as_uuid=True), db.ForeignKey("template_categories.id"), index=True, nullable=True) + @declared_attr def created_by(cls): return db.relationship("User") @@ -1179,6 +1196,8 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) + template_categories = db.relationship("TemplateCategories", backref="templates") + folder = db.relationship( "TemplateFolder", @@ -1198,6 +1217,15 @@ def get_link(self): _external=True, ) + @property + def template_process_type(self): + if self.template_type == SMS_TYPE: + return self.process_type if self.process_type else self.template_categories.sms_process_type + elif self.template_type == EMAIL_TYPE: + return self.process_type if self.process_type else self.template_categories.email_process_type + return self.process_type + + @classmethod def from_json(cls, data, folder=None): """ diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py new file mode 100644 index 0000000000..c6a0be975a --- /dev/null +++ b/migrations/versions/0454_add_template_category.py @@ -0,0 +1,55 @@ +""" + +Revision ID: 0454_add_template_category +Revises: 0453_add_callback_failure_email +Create Date: 2024-06-11 13:32:00 +""" + +from datetime import datetime + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + + +def upgrade(): + op.create_table( + "template_categories", + sa.Column("id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("name_en", sa.String(length=255), nullable=False), + sa.Column("name_fr", sa.String(length=255), nullable=False), + sa.Column("description_en", sa.String(length=255), nullable=True), + sa.Column("description_fr", sa.String(length=255), nullable=True), + sa.Column("sms_process_type", sa.String(length=255), nullable=False), + sa.Column("email_process_type", sa.String(length=255), nullable=False), + sa.Column("hidden", sa.Boolean(), nullable=False), + ) + + op.add_column( + "templates", + sa.Colum("template_category_id", postgresql.UUID(as_uuid=True), nullable=True) + ) + op.create_index( + op.f("ix_template_category_id", "templates", ["template_category_id"], unique=False) + ) + op.create_foreign_key( + "fk_template_template_categories", + "template", + "template_category", + ["template_category_id"], + ["id"] + ) + op.get_bind() + + # Insert the generic Low priority (bulk) category + # op.execute(""" + # INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + # VALUES ('00000000-0000-0000-0000-000000000000', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true + # """ + # ) + +def downgrade(): + op.drop_constraint("fk_template_template_category", "templates", type_="foreignkey") + op.drop_index(op.f("ix_template_category_id"), table_name="templates") + op.drop_column("templates", "template_category_id") + op.drop_table("template_category_id") \ No newline at end of file From da56d6033266ed0abd6565e8736e82c10d99f710 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 11 Jun 2024 16:09:16 -0400 Subject: [PATCH 02/33] Fix prop logic for template_process_type --- app/models.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/models.py b/app/models.py index 920f218de1..f41d62d264 100644 --- a/app/models.py +++ b/app/models.py @@ -1219,13 +1219,12 @@ def get_link(self): @property def template_process_type(self): - if self.template_type == SMS_TYPE: - return self.process_type if self.process_type else self.template_categories.sms_process_type - elif self.template_type == EMAIL_TYPE: - return self.process_type if self.process_type else self.template_categories.email_process_type + if self.template_type == SMS_TYPE and self.template_categories.sms_process_type: + return self.template_categories.sms_process_type + elif self.template_type == EMAIL_TYPE and self.template_categories.email_process_type: + return self.template_categories.email_process_type return self.process_type - @classmethod def from_json(cls, data, folder=None): """ From e7346fdc8a7478b8eac44646968d4164940e3ca0 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 12 Jun 2024 17:12:30 -0400 Subject: [PATCH 03/33] Add indexes, unique constraints - Fix typos - Update sqlalchemy model --- app/models.py | 6 +-- .../versions/0454_add_template_category.py | 39 ++++++++++++++----- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/app/models.py b/app/models.py index f41d62d264..c5dbc93220 100644 --- a/app/models.py +++ b/app/models.py @@ -1034,11 +1034,11 @@ def get_users_with_permission(self): PRECOMPILED_TEMPLATE_NAME = "Pre-compiled PDF" class TemplateCategories(BaseModel): - __tablename__ = "template_category" + __tablename__ = "template_categories" id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - name_en = db.Column(db.String(255), nullable=False) - name_fr = db.Column(db.String(255), nullable=False) + name_en = db.Column(db.String(255), unique=True, nullable=False) + name_fr = db.Column(db.String(255), unique=True, nullable=False) description_en = db.Column(db.String(200), nullable=True) description_fr = db.Column(db.String(200), nullable=True) sms_process_type = db.Column(db.String(200), nullable=False) diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index c6a0be975a..36527f27f0 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -1,6 +1,6 @@ """ -Revision ID: 0454_add_template_category +Revision ID: 0454_add_template_categories Revises: 0453_add_callback_failure_email Create Date: 2024-06-11 13:32:00 """ @@ -11,11 +11,14 @@ from alembic import op from sqlalchemy.dialects import postgresql +revision = "0454_add_template_categories" +down_revision = "0452_set_pgaudit_config" + def upgrade(): op.create_table( "template_categories", - sa.Column("id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("id", postgresql.UUID(as_uuid=True), primary_key=True, nullable=False), sa.Column("name_en", sa.String(length=255), nullable=False), sa.Column("name_fr", sa.String(length=255), nullable=False), sa.Column("description_en", sa.String(length=255), nullable=True), @@ -23,23 +26,39 @@ def upgrade(): sa.Column("sms_process_type", sa.String(length=255), nullable=False), sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), + sa.UniqueConstraint("name_en"), + sa.UniqueConstraint("name_fr") ) op.add_column( "templates", - sa.Colum("template_category_id", postgresql.UUID(as_uuid=True), nullable=True) + sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True) + ) + op.create_index( + op.f("ix_template_category_id"), + "templates", + ["template_category_id"], + unique=False, ) op.create_index( - op.f("ix_template_category_id", "templates", ["template_category_id"], unique=False) + op.f("ix_template_categories_name_en"), + "template_categories", + ["name_en"], + unique=False, + ) + op.create_index( + op.f("ix_template_categories_name_fr"), + "template_categories", + ["name_fr"], + unique=False, ) op.create_foreign_key( "fk_template_template_categories", - "template", - "template_category", + "templates", + "template_categories", ["template_category_id"], ["id"] ) - op.get_bind() # Insert the generic Low priority (bulk) category # op.execute(""" @@ -49,7 +68,9 @@ def upgrade(): # ) def downgrade(): - op.drop_constraint("fk_template_template_category", "templates", type_="foreignkey") + op.drop_constraint("fk_template_template_categories", "templates", type_="foreignkey") op.drop_index(op.f("ix_template_category_id"), table_name="templates") + op.drop_index(op.f("ix_template_categories_name_en"), table_name="template_categories") + op.drop_index(op.f("ix_template_categories_name_fr"), table_name="template_categories") op.drop_column("templates", "template_category_id") - op.drop_table("template_category_id") \ No newline at end of file + op.drop_table("template_categories") \ No newline at end of file From 70334b0932f95507c45fcb3fc342a4e8655ee6de Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 13 Jun 2024 12:25:44 -0400 Subject: [PATCH 04/33] Add CRUD methods for TemplateCategories - Added template_category_id to the template_history table - Adjusted model class name TemplateCategories -> TemplateCategory - WIP: Added some basic tests for the TemplateCategory dao --- app/dao/template_categories_dao.py | 29 ++++ app/models.py | 7 +- .../versions/0454_add_template_category.py | 20 +-- tests/app/dao/test_template_categories_dao.py | 153 ++++++++++++++++++ 4 files changed, 192 insertions(+), 17 deletions(-) create mode 100644 app/dao/template_categories_dao.py create mode 100644 tests/app/dao/test_template_categories_dao.py diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py new file mode 100644 index 0000000000..5aca12ff7b --- /dev/null +++ b/app/dao/template_categories_dao.py @@ -0,0 +1,29 @@ +import uuid + +from flask import current_app +from sqlalchemy import asc + +from app import db +from app.dao.dao_utils import transactional +from app.models import TemplateCategory + + +def dao_create_template_category(template_category: TemplateCategory): + template_category.id = uuid.uuid4() + db.session.add(template_category) + + +def dao_update_template_category(template_category: TemplateCategory): + db.session.add(template_category) + + +def dao_get_template_category_by_id(template_category_id): + return TemplateCategory.query.filter_by(id=template_category_id).one() + + +def dao_get_template_category_by_template_id(template_id): + return TemplateCategory.query.join(TemplateCategory.templates).filter_by(id=template_id).one() + + +def dao_get_all_template_categories(): + return TemplateCategory.query.order_by(asc(TemplateCategory.name_en)).all() diff --git a/app/models.py b/app/models.py index c5dbc93220..c7a0eb0d28 100644 --- a/app/models.py +++ b/app/models.py @@ -24,7 +24,6 @@ convert_local_timezone_to_utc, convert_utc_to_local_timezone, ) -from pkg_resources import declare_namespace from sqlalchemy import CheckConstraint, Index, UniqueConstraint from sqlalchemy.dialects.postgresql import JSON, JSONB, UUID from sqlalchemy.ext.associationproxy import association_proxy @@ -1033,7 +1032,8 @@ def get_users_with_permission(self): PRECOMPILED_TEMPLATE_NAME = "Pre-compiled PDF" -class TemplateCategories(BaseModel): + +class TemplateCategory(BaseModel): __tablename__ = "template_categories" id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) @@ -1196,8 +1196,7 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) - template_categories = db.relationship("TemplateCategories", backref="templates") - + template_category = db.relationship("TemplateCategory", backref="templates") folder = db.relationship( "TemplateFolder", diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index 36527f27f0..de1f631325 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -27,13 +27,11 @@ def upgrade(): sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), sa.UniqueConstraint("name_en"), - sa.UniqueConstraint("name_fr") + sa.UniqueConstraint("name_fr"), ) - op.add_column( - "templates", - sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True) - ) + op.add_column("templates", sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True)) + op.add_column("templates_history", sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True)) op.create_index( op.f("ix_template_category_id"), "templates", @@ -52,13 +50,7 @@ def upgrade(): ["name_fr"], unique=False, ) - op.create_foreign_key( - "fk_template_template_categories", - "templates", - "template_categories", - ["template_category_id"], - ["id"] - ) + op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) # Insert the generic Low priority (bulk) category # op.execute(""" @@ -67,10 +59,12 @@ def upgrade(): # """ # ) + def downgrade(): op.drop_constraint("fk_template_template_categories", "templates", type_="foreignkey") op.drop_index(op.f("ix_template_category_id"), table_name="templates") op.drop_index(op.f("ix_template_categories_name_en"), table_name="template_categories") op.drop_index(op.f("ix_template_categories_name_fr"), table_name="template_categories") op.drop_column("templates", "template_category_id") - op.drop_table("template_categories") \ No newline at end of file + op.drop_column("templates_history", "template_category_id") + op.drop_table("template_categories") diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py new file mode 100644 index 0000000000..2b5fbcfd36 --- /dev/null +++ b/tests/app/dao/test_template_categories_dao.py @@ -0,0 +1,153 @@ +import pytest + +from app.dao.template_categories_dao import ( + dao_create_template_category, + dao_get_all_template_categories, + dao_get_template_category_by_id, + dao_get_template_category_by_template_id, + dao_update_template_category, +) +from app.dao.templates_dao import dao_create_template +from app.models import BULK, NORMAL, PRIORITY, Template, TemplateCategory + + +def test_create_template_category(notify_db_session): + data = { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + } + + template_category = TemplateCategory(**data) + dao_create_template_category(template_category) + + assert TemplateCategory.query.count() == 1 + assert len(dao_get_all_template_categories()) == 1 + + +def test_update_template_category(notify_db_session): + data = { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + } + + template_category = TemplateCategory(**data) + dao_create_template_category(template_category) + + template_category.name_en = "new english" + template_category.name_fr = "new french" + template_category.description_en = "new english description" + template_category.description_fr = "new french description" + template_category.sms_process_type = BULK + template_category.email_process_type = BULK + template_category.hidden = True + dao_update_template_category(template_category) + + assert TemplateCategory.query.count() == 1 + assert len(dao_get_all_template_categories()) == 1 + assert dao_get_all_template_categories()[0].name_en == "new english" + assert dao_get_all_template_categories()[0].name_fr == "new french" + assert dao_get_all_template_categories()[0].description_en == "new english description" + assert dao_get_all_template_categories()[0].description_fr == "new french description" + assert dao_get_all_template_categories()[0].sms_process_type == BULK + assert dao_get_all_template_categories()[0].email_process_type == BULK + assert dao_get_all_template_categories()[0].hidden == True + assert dao_get_all_template_categories()[0].id == template_category.id + + +def test_get_template_category_by_template_id(notify_db_session, sample_service, sample_user): + category_data = { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + } + + template_category = TemplateCategory(**category_data) + dao_create_template_category(template_category) + + template_data = { + "name": "Sample Template", + "template_type": "email", + "content": "Template content", + "service": sample_service, + "created_by": sample_user, + } + + template = Template(**template_data) + template.template_category = template_category + dao_create_template(template) + + assert dao_get_template_category_by_template_id(template.id) == template_category + + +def test_get_template_category_by_id(notify_db_session): + data = { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + } + + template_category = TemplateCategory(**data) + dao_create_template_category(template_category) + + assert dao_get_template_category_by_id(template_category.id) == template_category + + +def test_get_all_template_categories(notify_db_session): + data1 = { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + } + + data2 = { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": BULK, + "email_process_type": BULK, + "hidden": True, + } + + template_category1 = TemplateCategory(**data1) + template_category2 = TemplateCategory(**data2) + dao_create_template_category(template_category1) + dao_create_template_category(template_category2) + + assert len(dao_get_all_template_categories()) == 2 + assert dao_get_all_template_categories()[0].name_en == "english" + assert dao_get_all_template_categories()[0].name_fr == "french" + assert dao_get_all_template_categories()[0].description_en == "english description" + assert dao_get_all_template_categories()[0].description_fr == "french description" + assert dao_get_all_template_categories()[0].sms_process_type == NORMAL + assert dao_get_all_template_categories()[0].email_process_type == NORMAL + assert dao_get_all_template_categories()[0].hidden == False + assert dao_get_all_template_categories()[1].name_en == "english2" + assert dao_get_all_template_categories()[1].name_fr == "french2" + assert dao_get_all_template_categories()[1].description_en == "english description2" + assert dao_get_all_template_categories()[1].description_fr == "french description2" + assert dao_get_all_template_categories()[1].sms_process_type == BULK + assert dao_get_all_template_categories()[1].email_process_type == BULK + assert dao_get_all_template_categories()[1].hidden == True From 0430b4ba2a91f7fb079b264f3979929c68389f8b Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 13 Jun 2024 12:31:08 -0400 Subject: [PATCH 05/33] Insert low, med, high default categories during migration --- .../versions/0454_add_template_category.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index de1f631325..07d60a0383 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -14,6 +14,10 @@ revision = "0454_add_template_categories" down_revision = "0452_set_pgaudit_config" +# TODO: Add these to the config instead +DEFAULT_LOW = "0dda24c2-982a-4f44-9749-0e38b2607e89" +DEFAULT_MEDIUM = "f75d6706-21b7-437e-b93a-2c0ab771e28e" +DEFAULT_HIGH = "c4f87d7c-a55b-4c0f-91fe-e56c65bb1871" def upgrade(): op.create_table( @@ -52,12 +56,22 @@ def upgrade(): ) op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) - # Insert the generic Low priority (bulk) category - # op.execute(""" - # INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - # VALUES ('00000000-0000-0000-0000-000000000000', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true - # """ - # ) + # Insert the generic low, medium, and high categories + op.execute(f""" + INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ({DEFAULT_LOW}, 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true + """ + ) + op.execute(f""" + INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ({DEFAULT_MEDIUM}, 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', true + """ + ) + op.execute(f""" + INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ({DEFAULT_HIGH}, 'High Category (Priority)', 'Catégorie Haute (Priorité)', true + """ + ) def downgrade(): From 28637f279601399c4a9feff107d25454e44312a9 Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 13 Jun 2024 13:12:35 -0400 Subject: [PATCH 06/33] Fix prop logic for template_process_type again --- app/models.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models.py b/app/models.py index c7a0eb0d28..dbe2174e27 100644 --- a/app/models.py +++ b/app/models.py @@ -1218,10 +1218,10 @@ def get_link(self): @property def template_process_type(self): - if self.template_type == SMS_TYPE and self.template_categories.sms_process_type: - return self.template_categories.sms_process_type - elif self.template_type == EMAIL_TYPE and self.template_categories.email_process_type: - return self.template_categories.email_process_type + if self.template_type == SMS_TYPE: + return self.process_type if self.process_type else self.template_categories.sms_process_type + elif self.template_type == EMAIL_TYPE: + return self.process_type if self.process_type else self.template_categories.email_process_type return self.process_type @classmethod From b76744e25e65b77f33170e4476a148f1a665a2eb Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 13 Jun 2024 16:51:15 -0400 Subject: [PATCH 07/33] WIP: Add API endpoints for interacting with TemplateCategories - Added a schema for TemplateCategory - Added from_json and serialize to the TemplateCategory model - Added the template/category Blueprint - Add some initial CRUD api endpoints --- app/dao/template_categories_dao.py | 3 +- app/dao/templates_dao.py | 34 +++++++++++++ app/models.py | 19 ++++++- app/schemas.py | 27 ++++++++++ app/template/rest.py | 20 ++++++++ app/template/template_category_rest.py | 49 +++++++++++++++++++ .../versions/0454_add_template_category.py | 10 ++-- tests/app/dao/test_template_categories_dao.py | 14 +++--- 8 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 app/template/template_category_rest.py diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 5aca12ff7b..03d4851498 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,6 +1,5 @@ import uuid -from flask import current_app from sqlalchemy import asc from app import db @@ -8,11 +7,13 @@ from app.models import TemplateCategory +@transactional def dao_create_template_category(template_category: TemplateCategory): template_category.id = uuid.uuid4() db.session.add(template_category) +@transactional def dao_update_template_category(template_category: TemplateCategory): db.session.add(template_category) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 822296cae4..7c4dd09bd1 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -78,6 +78,40 @@ def dao_update_template_reply_to(template_id, reply_to): return template +@transactional +def dao_update_template_category(template_id, category_id): + Template.query.filter_by(id=template_id).update( + { + "template_category_id": category_id, + "updated_at": datetime.utcnow(), + "version": Template.version + 1, + } + ) + + template = Template.query.filter_by(id=template_id).one() + + history = TemplateHistory( + **{ + "id": template.id, + "name": template.name, + "template_type": template.template_type, + "created_at": template.created_at, + "updated_at": template.updated_at, + "content": template.content, + "service_id": template.service_id, + "subject": template.subject, + "postage": template.postage, + "created_by_id": template.created_by_id, + "version": template.version, + "archived": template.archived, + "process_type": template.process_type, + "service_letter_contact_id": template.service_letter_contact_id, + } + ) + db.session.add(history) + return template + + @transactional def dao_redact_template(template, user_id): template.template_redacted.redact_personalisation = True diff --git a/app/models.py b/app/models.py index dbe2174e27..a9abcc0277 100644 --- a/app/models.py +++ b/app/models.py @@ -1045,6 +1045,23 @@ class TemplateCategory(BaseModel): email_process_type = db.Column(db.String(200), nullable=False) hidden = db.Column(db.Boolean, nullable=False, default=False) + def serialize(self): + return { + "id": self.id, + "name_en": self.name_en, + "name_fr": self.name_fr, + "description_en": self.description_en, + "description_fr": self.description_fr, + "sms_process_type": self.sms_process_type, + "email_process_type": self.email_process_type, + "hidden": self.hidden, + } + + @classmethod + def from_json(cls, data): + fields = data.copy() + return cls(**fields) + class TemplateBase(BaseModel): __abstract__ = True @@ -1196,7 +1213,7 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) - template_category = db.relationship("TemplateCategory", backref="templates") + category = db.relationship("TemplateCategory", backref="templates") folder = db.relationship( "TemplateFolder", diff --git a/app/schemas.py b/app/schemas.py index 75a6a2e78f..79996c0949 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -107,6 +107,32 @@ def make_instance(self, data, **kwargs): return super(BaseSchema, self).make_instance(data) +class TemplateCategorySchema(BaseSchema): + class Meta(BaseSchema.Meta): + model = models.TemplateCategory + exclude = ("id",) + + @validates("name_en") + def validate_name_en(self, value): + if not value: + raise ValidationError("Invalid name") + + @validates("name_fr") + def validate_name_fr(self, value): + if not value: + raise ValidationError("Invalid name") + + @validates("sms_process_type") + def validate_sms_process_type(self, value): + if value not in models.TEMPLATE_PROCESS_TYPE: + raise ValidationError("Invalid SMS process type") + + @validates("email_process_type") + def validate_email_process_type(self, value): + if value not in models.TEMPLATE_PROCESS_TYPE: + raise ValidationError("Invalid email process type") + + class UserSchema(BaseSchema): permissions = fields.Method("user_permissions", dump_only=True) password_changed_at = field_for(models.User, "password_changed_at", format="%Y-%m-%d %H:%M:%S.%f") @@ -805,6 +831,7 @@ def validate_archived(self, data, **kwargs): service_history_schema = ServiceHistorySchema() api_key_history_schema = ApiKeyHistorySchema() template_history_schema = TemplateHistorySchema() +template_category_schema = TemplateCategorySchema() event_schema = EventSchema() provider_details_schema = ProviderDetailsSchema() provider_details_history_schema = ProviderDetailsHistorySchema() diff --git a/app/template/rest.py b/app/template/rest.py index 46e5348dbf..32043496a5 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -25,6 +25,7 @@ dao_get_template_versions, dao_redact_template, dao_update_template, + dao_update_template_category, dao_update_template_reply_to, get_precompiled_letter_template, ) @@ -132,6 +133,25 @@ def create_template(service_id): return jsonify(data=template_schema.dump(new_template)), 201 +@template_blueprint.route("//template-category/", methods=["POST"]) +def update_templates_category(template_id, template_category_id): + updated = dao_update_template_category(template_id, template_category_id) + return jsonify(data=template_schema.dump(updated)), 200 + + +@template_blueprint.route("//process-type", methods=["POST"]) +def update_template_process_type(template_id): + data = request.get_json() + if "process_type" not in data: + message = "Field is required" + errors = {"process_type": [message]} + raise InvalidRequest(errors, status_code=400) + + # updated = dao_update_template_process_type(template_id=template_id, process_type=data.get("process_type")) + # return jsonify(data=template_schema.dump(updated)), 200 + pass + + @template_blueprint.route("/", methods=["POST"]) def update_template(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py new file mode 100644 index 0000000000..dfd1d20efb --- /dev/null +++ b/app/template/template_category_rest.py @@ -0,0 +1,49 @@ +from flask import Blueprint, jsonify, request + +from app.dao.template_categories_dao import ( + dao_create_template_category, + dao_get_all_template_categories, + dao_get_template_category_by_id, + dao_update_template_category, +) +from app.models import TemplateCategory +from app.schemas import template_category_schema + +template_category_blueprint = Blueprint( + "template_category", + __name__, + url_prefix="template/category", +) + + +@template_category_blueprint.route("", methods=["POST"]) +def create_template_category(): + data = request.get_json() + + template_category_schema.load(data) + template_category = TemplateCategory.from_json(data) + + dao_create_template_category(template_category) + + return jsonify(data=template_category_schema.dump(template_category)), 201 + + +@template_category_blueprint.route("/", methods=["POST"]) +def update_template_category(template_category_id): + request_json = request.get_json() + update_dict = template_category_schema.load(request_json) + + category_to_update = dao_get_template_category_by_id(template_category_id) + + for key in request_json: + setattr(category_to_update, key, update_dict[key]) + + dao_update_template_category(category_to_update) + + return jsonify(data=category_to_update.serialize()), 200 + + +@template_category_blueprint.route("", methods=["GET"]) +def get_template_categories(): + template_categories = dao_get_all_template_categories() + return jsonify(data=template_category_schema.dump(template_categories, many=True)), 200 diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index 07d60a0383..e59b5599b7 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -19,6 +19,7 @@ DEFAULT_MEDIUM = "f75d6706-21b7-437e-b93a-2c0ab771e28e" DEFAULT_HIGH = "c4f87d7c-a55b-4c0f-91fe-e56c65bb1871" + def upgrade(): op.create_table( "template_categories", @@ -57,17 +58,20 @@ def upgrade(): op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) # Insert the generic low, medium, and high categories - op.execute(f""" + op.execute( + f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ({DEFAULT_LOW}, 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true """ ) - op.execute(f""" + op.execute( + f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ({DEFAULT_MEDIUM}, 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', true """ ) - op.execute(f""" + op.execute( + f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ({DEFAULT_HIGH}, 'High Category (Priority)', 'Catégorie Haute (Priorité)', true """ diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index 2b5fbcfd36..a0925cbe0b 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -1,5 +1,3 @@ -import pytest - from app.dao.template_categories_dao import ( dao_create_template_category, dao_get_all_template_categories, @@ -8,7 +6,7 @@ dao_update_template_category, ) from app.dao.templates_dao import dao_create_template -from app.models import BULK, NORMAL, PRIORITY, Template, TemplateCategory +from app.models import BULK, NORMAL, Template, TemplateCategory def test_create_template_category(notify_db_session): @@ -60,7 +58,7 @@ def test_update_template_category(notify_db_session): assert dao_get_all_template_categories()[0].description_fr == "new french description" assert dao_get_all_template_categories()[0].sms_process_type == BULK assert dao_get_all_template_categories()[0].email_process_type == BULK - assert dao_get_all_template_categories()[0].hidden == True + assert dao_get_all_template_categories()[0].hidden assert dao_get_all_template_categories()[0].id == template_category.id @@ -116,8 +114,8 @@ def test_get_all_template_categories(notify_db_session): "name_fr": "french", "description_en": "english description", "description_fr": "french description", - "sms_process_type": NORMAL, - "email_process_type": NORMAL, + "sms_process_type": "normal", + "email_process_type": "normal", "hidden": False, } @@ -143,11 +141,11 @@ def test_get_all_template_categories(notify_db_session): assert dao_get_all_template_categories()[0].description_fr == "french description" assert dao_get_all_template_categories()[0].sms_process_type == NORMAL assert dao_get_all_template_categories()[0].email_process_type == NORMAL - assert dao_get_all_template_categories()[0].hidden == False + assert not dao_get_all_template_categories()[0].hidden assert dao_get_all_template_categories()[1].name_en == "english2" assert dao_get_all_template_categories()[1].name_fr == "french2" assert dao_get_all_template_categories()[1].description_en == "english description2" assert dao_get_all_template_categories()[1].description_fr == "french description2" assert dao_get_all_template_categories()[1].sms_process_type == BULK assert dao_get_all_template_categories()[1].email_process_type == BULK - assert dao_get_all_template_categories()[1].hidden == True + assert dao_get_all_template_categories()[1].hidden From e34dc4a530d54f5c745ee4b952b956d995f3f177 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 17 Jun 2024 11:34:53 -0400 Subject: [PATCH 08/33] Implement dao and api to update process_type --- app/dao/templates_dao.py | 31 +++++++++++++++++++++++++++++++ app/template/rest.py | 6 +++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 7c4dd09bd1..26d83eb13a 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -78,6 +78,37 @@ def dao_update_template_reply_to(template_id, reply_to): return template +@transactional +def dao_update_template_process_type(template_id, process_type): + Template.query.filter_by(id=template_id).update( + { + "process_type": process_type, + } + ) + template = Template.query.filter_by(id=template_id).one() + + history = TemplateHistory( + **{ + "id": template.id, + "name": template.name, + "template_type": template.template_type, + "created_at": template.created_at, + "updated_at": template.updated_at, + "content": template.content, + "service_id": template.service_id, + "subject": template.subject, + "postage": template.postage, + "created_by_id": template.created_by_id, + "version": template.version, + "archived": template.archived, + "process_type": template.process_type, + "service_letter_contact_id": template.service_letter_contact_id, + } + ) + db.session.add(history) + return template + + @transactional def dao_update_template_category(template_id, category_id): Template.query.filter_by(id=template_id).update( diff --git a/app/template/rest.py b/app/template/rest.py index 32043496a5..9f063e54f3 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -26,6 +26,7 @@ dao_redact_template, dao_update_template, dao_update_template_category, + dao_update_template_process_type, dao_update_template_reply_to, get_precompiled_letter_template, ) @@ -147,9 +148,8 @@ def update_template_process_type(template_id): errors = {"process_type": [message]} raise InvalidRequest(errors, status_code=400) - # updated = dao_update_template_process_type(template_id=template_id, process_type=data.get("process_type")) - # return jsonify(data=template_schema.dump(updated)), 200 - pass + updated = dao_update_template_process_type(template_id=template_id, process_type=data.get("process_type")) + return jsonify(data=template_schema.dump(updated)), 200 @template_blueprint.route("/", methods=["POST"]) From 6a291fce31a0ed2216d96d872b18d4556e3f7c12 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 17 Jun 2024 12:04:04 -0400 Subject: [PATCH 09/33] Address PR comments - Changed the category relationship in Template from TemplateCategory to template_category - dao_get_template_category_by_template_id now simply selects the template, and returns the associated template_category instead of using an expensive join --- app/dao/template_categories_dao.py | 11 ++++++----- app/models.py | 5 ++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 03d4851498..dd9cd4a7f7 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -4,7 +4,7 @@ from app import db from app.dao.dao_utils import transactional -from app.models import TemplateCategory +from app.models import Template, TemplateCategory @transactional @@ -18,13 +18,14 @@ def dao_update_template_category(template_category: TemplateCategory): db.session.add(template_category) -def dao_get_template_category_by_id(template_category_id): +def dao_get_template_category_by_id(template_category_id) -> TemplateCategory: return TemplateCategory.query.filter_by(id=template_category_id).one() -def dao_get_template_category_by_template_id(template_id): - return TemplateCategory.query.join(TemplateCategory.templates).filter_by(id=template_id).one() +def dao_get_template_category_by_template_id(template_id) -> TemplateCategory: + return Template.query.filter_by(id=template_id).one().template_category -def dao_get_all_template_categories(): +# TODO: Add filters: Select all template categories used by at least 1 sms/email template +def dao_get_all_template_categories(template_type=None, hidden=False): return TemplateCategory.query.order_by(asc(TemplateCategory.name_en)).all() diff --git a/app/models.py b/app/models.py index a9abcc0277..b79da6d5ad 100644 --- a/app/models.py +++ b/app/models.py @@ -1213,7 +1213,7 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) - category = db.relationship("TemplateCategory", backref="templates") + category = db.relationship("template_category", backref="templates") folder = db.relationship( "TemplateFolder", @@ -1235,6 +1235,9 @@ def get_link(self): @property def template_process_type(self): + """By default we use the process_type from TemplateCategory, but allow admins to override it on a per-template basis. + Only when overriden do we use the process_type from the template itself. + """ if self.template_type == SMS_TYPE: return self.process_type if self.process_type else self.template_categories.sms_process_type elif self.template_type == EMAIL_TYPE: From fcb355091514f2cf9cb085de21edc3895e587e06 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 17 Jun 2024 15:34:20 -0400 Subject: [PATCH 10/33] Finish adding needed api endpoints - Moved the default categories into our config file - Added dao_update_template_category - Added dao_delete_template_category_by_id - Added routes to: get and delete a template category - Added a route to get all template categories - Updated the migration file to use default category uuid's from the config file --- app/config.py | 3 + app/dao/template_categories_dao.py | 58 +++++++++++++++++-- app/template/template_category_rest.py | 39 ++++++++++--- ...egory.py => 0453_add_template_category.py} | 18 +++--- 4 files changed, 95 insertions(+), 23 deletions(-) rename migrations/versions/{0454_add_template_category.py => 0453_add_template_category.py} (81%) diff --git a/app/config.py b/app/config.py index b8b6521ad5..5acf768a2f 100644 --- a/app/config.py +++ b/app/config.py @@ -340,6 +340,9 @@ class Config(object): HEARTBEAT_TEMPLATE_SMS_LOW = "ab3a603b-d602-46ea-8c83-e05cb280b950" HEARTBEAT_TEMPLATE_SMS_MEDIUM = "a48b54ce-40f6-4e4a-abe8-1e2fa389455b" HEARTBEAT_TEMPLATE_SMS_HIGH = "4969a9e9-ddfd-476e-8b93-6231e6f1be4a" + DEFAULT_TEMPLATE_CATEGORY_LOW = "0dda24c2-982a-4f44-9749-0e38b2607e89" + DEFAULT_TEMPLATE_CATEGORY_MEDIUM = "f75d6706-21b7-437e-b93a-2c0ab771e28e" + DEFAULT_TEMPLATE_CATEGORY_HIGH = "c4f87d7c-a55b-4c0f-91fe-e56c65bb1871" # 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/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index dd9cd4a7f7..80b2b54490 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -2,8 +2,10 @@ from sqlalchemy import asc +from flask import current_app from app import db from app.dao.dao_utils import transactional +from app.errors import InvalidRequest from app.models import Template, TemplateCategory @@ -13,11 +15,6 @@ def dao_create_template_category(template_category: TemplateCategory): db.session.add(template_category) -@transactional -def dao_update_template_category(template_category: TemplateCategory): - db.session.add(template_category) - - def dao_get_template_category_by_id(template_category_id) -> TemplateCategory: return TemplateCategory.query.filter_by(id=template_category_id).one() @@ -29,3 +26,54 @@ def dao_get_template_category_by_template_id(template_id) -> TemplateCategory: # TODO: Add filters: Select all template categories used by at least 1 sms/email template def dao_get_all_template_categories(template_type=None, hidden=False): return TemplateCategory.query.order_by(asc(TemplateCategory.name_en)).all() + + +@transactional +def dao_update_template_category(template_category: TemplateCategory): + db.session.add(template_category) + + +@transactional +def dao_delete_template_category_by_id(template_category_id, cascade = False): + """ + Deletes a `TemplateCategory`. By default, if the `TemplateCategory` is associated with any `Template`, it will not be deleted. + If the `cascade` option is specified then the category will be forcible removed: + 1. The `Category` will be dissociated from templates that use it + 2. Dissociated templates will be assigned a default category based on the sms/email process type of the category it was associated with + previously + 3. Finally, the `Category` will be deleted + + Args: + template_category_id (str): The id of the template_category to delete + cascade (bool, optional): Specify whether to dissociate the category from templates that use it to force removal. Defaults to False. + + Raises: + e: _description_ + """ + template_category = dao_get_template_category_by_id(template_category_id) + templates = Template.query.filter_by(template_category_id=template_category_id).all() + + if templates: + if cascade: + try: + for template in templates: + process_type = template_category.sms_process_type if template.template_type == 'sms' else template_category.email_process_type + template.category = dao_get_template_category_by_id(_get_default_category_id(process_type)) + db.session.add(template) + + db.session.delete(template_category) + except Exception as e: + db.session.rollback() + raise e + else: + db.session.delete(template_category) + db.session.commit() + + +def _get_default_category_id(process_type): + default_categories = { + 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], + 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], + 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] + } + return default_categories.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index dfd1d20efb..0f58095d49 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -2,11 +2,13 @@ from app.dao.template_categories_dao import ( dao_create_template_category, + dao_delete_template_category_by_id, dao_get_all_template_categories, dao_get_template_category_by_id, + dao_get_template_category_by_template_id, dao_update_template_category, ) -from app.models import TemplateCategory +from app.models import Template, TemplateCategory from app.schemas import template_category_schema template_category_blueprint = Blueprint( @@ -28,7 +30,23 @@ def create_template_category(): return jsonify(data=template_category_schema.dump(template_category)), 201 -@template_category_blueprint.route("/", methods=["POST"]) +@template_category_blueprint.route("", methods=["GET"]) +def get_template_category(template_category_id = None, template_id = None): + if template_id: + template_category = dao_get_template_category_by_template_id(template_id) + else: + template_category = dao_get_template_category_by_id(template_id) + + return jsonify(template_category.dump()), 200 + + +@template_category_blueprint.route("", methods=["GET"]) +def get_template_categories(): + template_categories = dao_get_all_template_categories() + return jsonify(data=template_category_schema.dump(template_categories, many=True)), 200 + + +@template_category_blueprint.route("/", methods=["POST"]) def update_template_category(template_category_id): request_json = request.get_json() update_dict = template_category_schema.load(request_json) @@ -40,10 +58,17 @@ def update_template_category(template_category_id): dao_update_template_category(category_to_update) - return jsonify(data=category_to_update.serialize()), 200 + return jsonify(data=category_to_update.dump()), 200 +@template_category_blueprint.route("/", methods=["DELETE"]) +def delete_template_category(template_category_id): + cascade = request.args.get("cascade", False) -@template_category_blueprint.route("", methods=["GET"]) -def get_template_categories(): - template_categories = dao_get_all_template_categories() - return jsonify(data=template_category_schema.dump(template_categories, many=True)), 200 + if not cascade: + templates = Template.query.filter_by(template_category_id=template_category_id).one() + if templates: + return jsonify(message="Cannot delete template category with templates assigned to it"), 400 + + dao_delete_template_category_by_id(template_category_id, cascade) + + return "", 204 \ No newline at end of file diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0453_add_template_category.py similarity index 81% rename from migrations/versions/0454_add_template_category.py rename to migrations/versions/0453_add_template_category.py index e59b5599b7..f4cf1904bc 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -1,7 +1,7 @@ """ -Revision ID: 0454_add_template_categories -Revises: 0453_add_callback_failure_email +Revision ID: 0453_add_template_categories +Revises: 0452_set_pgaudit_config Create Date: 2024-06-11 13:32:00 """ @@ -9,16 +9,12 @@ import sqlalchemy as sa from alembic import op +from flask import current_app from sqlalchemy.dialects import postgresql -revision = "0454_add_template_categories" +revision = "0453_add_template_categories" down_revision = "0452_set_pgaudit_config" -# TODO: Add these to the config instead -DEFAULT_LOW = "0dda24c2-982a-4f44-9749-0e38b2607e89" -DEFAULT_MEDIUM = "f75d6706-21b7-437e-b93a-2c0ab771e28e" -DEFAULT_HIGH = "c4f87d7c-a55b-4c0f-91fe-e56c65bb1871" - def upgrade(): op.create_table( @@ -61,19 +57,19 @@ def upgrade(): op.execute( f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({DEFAULT_LOW}, 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true + VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']}, 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true """ ) op.execute( f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({DEFAULT_MEDIUM}, 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', true + VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM']}, 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', true """ ) op.execute( f""" INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({DEFAULT_HIGH}, 'High Category (Priority)', 'Catégorie Haute (Priorité)', true + VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH']}, 'High Category (Priority)', 'Catégorie Haute (Priorité)', true """ ) From d8799ac4813aec0a6afcc41a77c1d1268b0d7018 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 17 Jun 2024 15:53:07 -0400 Subject: [PATCH 11/33] Chore: logic cleanup --- app/dao/template_categories_dao.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 80b2b54490..367c90c6e1 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -9,6 +9,12 @@ from app.models import Template, TemplateCategory +DEFAULT_TEMPLATE_CATEGORIES = { + 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], + 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], + 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] +} + @transactional def dao_create_template_category(template_category: TemplateCategory): template_category.id = uuid.uuid4() @@ -58,7 +64,9 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): try: for template in templates: process_type = template_category.sms_process_type if template.template_type == 'sms' else template_category.email_process_type - template.category = dao_get_template_category_by_id(_get_default_category_id(process_type)) + default_category_id = DEFAULT_TEMPLATE_CATEGORIES.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) + template.category = dao_get_template_category_by_id(default_category_id) + db.session.add(template) db.session.delete(template_category) @@ -69,11 +77,3 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): db.session.delete(template_category) db.session.commit() - -def _get_default_category_id(process_type): - default_categories = { - 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], - 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], - 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] - } - return default_categories.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) From 6da3eaad0896570ac0918962fd0f20374ffb7bd0 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 18 Jun 2024 13:01:40 -0400 Subject: [PATCH 12/33] First batch of unit tests & bug fixes --- app/__init__.py | 3 + app/dao/template_categories_dao.py | 18 ++-- app/models.py | 2 +- app/template/template_category_rest.py | 26 +++--- .../versions/0453_add_template_category.py | 12 +-- tests/app/conftest.py | 56 +++++++++++++ .../template/test_template_category_rest.py | 84 +++++++++++++++++++ 7 files changed, 174 insertions(+), 27 deletions(-) create mode 100644 tests/app/template/test_template_category_rest.py diff --git a/app/__init__.py b/app/__init__.py index c3c144620e..78fae3406f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -201,6 +201,7 @@ def register_blueprint(application): from app.service.rest import service_blueprint from app.status.healthcheck import status as status_blueprint from app.template.rest import template_blueprint + from app.template.template_category_rest import template_category_blueprint from app.template_folder.rest import template_folder_blueprint from app.template_statistics.rest import ( template_statistics as template_statistics_blueprint, @@ -259,6 +260,8 @@ def register_blueprint(application): register_notify_blueprint(application, letter_branding_blueprint, requires_admin_auth) + register_notify_blueprint(application, template_category_blueprint, requires_admin_auth) + def register_v2_blueprints(application): from app.authentication.auth import requires_auth diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 367c90c6e1..80b2b54490 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -9,12 +9,6 @@ from app.models import Template, TemplateCategory -DEFAULT_TEMPLATE_CATEGORIES = { - 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], - 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], - 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] -} - @transactional def dao_create_template_category(template_category: TemplateCategory): template_category.id = uuid.uuid4() @@ -64,9 +58,7 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): try: for template in templates: process_type = template_category.sms_process_type if template.template_type == 'sms' else template_category.email_process_type - default_category_id = DEFAULT_TEMPLATE_CATEGORIES.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) - template.category = dao_get_template_category_by_id(default_category_id) - + template.category = dao_get_template_category_by_id(_get_default_category_id(process_type)) db.session.add(template) db.session.delete(template_category) @@ -77,3 +69,11 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): db.session.delete(template_category) db.session.commit() + +def _get_default_category_id(process_type): + default_categories = { + 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], + 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], + 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] + } + return default_categories.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) diff --git a/app/models.py b/app/models.py index b79da6d5ad..ec3c486701 100644 --- a/app/models.py +++ b/app/models.py @@ -1213,7 +1213,7 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) - category = db.relationship("template_category", backref="templates") + category = db.relationship("TemplateCategory", backref="templates") folder = db.relationship( "TemplateFolder", diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index 0f58095d49..f9498bf7d7 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -8,15 +8,17 @@ dao_get_template_category_by_template_id, dao_update_template_category, ) +from app.errors import register_errors from app.models import Template, TemplateCategory from app.schemas import template_category_schema template_category_blueprint = Blueprint( "template_category", __name__, - url_prefix="template/category", + url_prefix="/template/category", ) +register_errors(template_category_blueprint) @template_category_blueprint.route("", methods=["POST"]) def create_template_category(): @@ -27,23 +29,25 @@ def create_template_category(): dao_create_template_category(template_category) - return jsonify(data=template_category_schema.dump(template_category)), 201 + return jsonify(template_category=template_category_schema.dump(template_category)), 201 @template_category_blueprint.route("", methods=["GET"]) -def get_template_category(template_category_id = None, template_id = None): - if template_id: - template_category = dao_get_template_category_by_template_id(template_id) - else: - template_category = dao_get_template_category_by_id(template_id) +def get_template_category(template_category_id = None): + template_category = dao_get_template_category_by_id(template_category_id) + return jsonify(template_category=template_category_schema.dump(template_category)), 200 - return jsonify(template_category.dump()), 200 + +@template_category_blueprint.route("", methods=["GET"]) +def get_template_category_by_template_id(template_id): + template_category = dao_get_template_category_by_template_id(template_id) + return jsonify(template_category=template_category_schema.dump(template_category)), 200 @template_category_blueprint.route("", methods=["GET"]) def get_template_categories(): - template_categories = dao_get_all_template_categories() - return jsonify(data=template_category_schema.dump(template_categories, many=True)), 200 + template_categories = template_category_schema.dump(dao_get_all_template_categories(), many=True) + return jsonify(template_categories=template_categories), 200 @template_category_blueprint.route("/", methods=["POST"]) @@ -58,7 +62,7 @@ def update_template_category(template_category_id): dao_update_template_category(category_to_update) - return jsonify(data=category_to_update.dump()), 200 + return jsonify(template_category=category_to_update.dump()), 200 @template_category_blueprint.route("/", methods=["DELETE"]) def delete_template_category(template_category_id): diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index f4cf1904bc..8daf18145c 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -56,20 +56,20 @@ def upgrade(): # Insert the generic low, medium, and high categories op.execute( f""" - INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']}, 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', true + INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true) """ ) op.execute( f""" - INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM']}, 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', true + INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"]}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true) """ ) op.execute( f""" - INSERT INTO template_category (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ({current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH']}, 'High Category (Priority)', 'Catégorie Haute (Priorité)', true + INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) + VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"]}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true) """ ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 3088d1e88e..e05d16a8b4 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -21,6 +21,7 @@ from app.dao.organisation_dao import dao_create_organisation from app.dao.provider_rates_dao import create_provider_rates 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 from app.dao.users_dao import create_secret_code, create_user_code from app.history_meta import create_history @@ -52,6 +53,7 @@ ServiceEmailReplyTo, ServiceSafelist, Template, + TemplateCategory, TemplateHistory, ) from tests import create_authorization_header @@ -230,6 +232,57 @@ def _sample_service_custom_letter_contact_block(sample_service): return sample_service +@pytest.fixture(scope="function") +def sample_template_category( + notify_db, + notify_db_session, + name_en="Category Name", + name_fr="Category Name (FR)", + description_en="Category Description", + description_fr="Category Description (FR)", + sms_process_type="normal", + email_process_type="normal", + hidden=False, +): + return create_template_category( + notify_db, + notify_db_session, + name_en="Category Name", + name_fr="Category Name (FR)", + description_en="Category Description", + description_fr="Category Description (FR)", + sms_process_type="normal", + email_process_type="normal", + hidden=False, + ) + + +def create_template_category( + notify_db, + notify_db_session, + name_en="Category Name", + name_fr="Category Name (FR)", + description_en="Category Description", + description_fr="Category Description (FR)", + sms_process_type="normal", + email_process_type="normal", + hidden=False, +): + data = { + "name_en": name_en, + "name_fr": name_fr, + "description_en": description_en, + "description_fr": description_fr, + "sms_process_type": sms_process_type, + "email_process_type": email_process_type, + "hidden": hidden, + } + template_category = TemplateCategory(**data) + dao_create_template_category(template_category) + + return template_category + + def create_sample_template( notify_db, notify_db_session, @@ -241,6 +294,7 @@ def create_sample_template( subject_line="Subject", user=None, service=None, + template_category=None, created_by=None, process_type="normal", permissions=[EMAIL_TYPE, SMS_TYPE], @@ -268,6 +322,8 @@ def create_sample_template( data.update({"subject": subject_line}) if template_type == "letter": data["postage"] = "second" + if template_category: + data["category"] = template_category template = Template(**data) dao_create_template(template) diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py new file mode 100644 index 0000000000..a2e4170256 --- /dev/null +++ b/tests/app/template/test_template_category_rest.py @@ -0,0 +1,84 @@ +import json +from flask import url_for + +from tests import create_authorization_header +from tests.app.conftest import create_sample_template, create_template_category + + +def test_should_create_new_template_category(client, notify_db, notify_db_session): + data = { + "name_en": "new english", + "name_fr": "new french", + "description_en": "new english description", + "description_fr": "new french description", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + } + + auth_header = create_authorization_header() + + response = client.post( + "/template/category", + headers=[("Content-Type", "application/json"), auth_header], + json=data, + ) + + assert response.status_code == 201 + assert response.json["template_category"]["name_en"] == "new english" + assert response.json["template_category"]["name_fr"] == "new french" + assert response.json["template_category"]["description_en"] == "new english description" + assert response.json["template_category"]["description_fr"] == "new french description" + assert response.json["template_category"]["sms_process_type"] == "bulk" + assert response.json["template_category"]["email_process_type"] == "bulk" + assert response.json["template_category"]["hidden"] + + +def test_get_template_category_by_id(client, sample_template_category): + auth_header = create_authorization_header() + response = client.get( + f"/template/category/{sample_template_category.id}", + headers=[("Content-Type", "application/json"), auth_header], + ) + + assert response.status_code == 200 + assert response.json["template_category"]["name_en"] == sample_template_category.name_en + assert response.json["template_category"]["name_fr"] == sample_template_category.name_fr + assert response.json["template_category"]["description_en"] == sample_template_category.description_en + assert response.json["template_category"]["description_fr"] == sample_template_category.description_fr + assert response.json["template_category"]["sms_process_type"] == sample_template_category.sms_process_type + assert response.json["template_category"]["email_process_type"] == sample_template_category.email_process_type + assert response.json["template_category"]["hidden"] == sample_template_category.hidden + + +def test_get_template_category_by_template_id(client, notify_db, notify_db_session, sample_template_category): + template = create_sample_template(notify_db, notify_db_session, template_category=sample_template_category) + + auth_header = create_authorization_header() + response = client.get( + f"/template/category/{template.id}", + headers=[("Content-Type", "application/json"), auth_header], + ) + + assert response.status_code == 200 + assert response.json["template_category"]["name_en"] == sample_template_category.name_en + assert response.json["template_category"]["name_fr"] == sample_template_category.name_fr + assert response.json["template_category"]["description_en"] == sample_template_category.description_en + assert response.json["template_category"]["description_fr"] == sample_template_category.description_fr + assert response.json["template_category"]["sms_process_type"] == sample_template_category.sms_process_type + assert response.json["template_category"]["email_process_type"] == sample_template_category.email_process_type + assert response.json["template_category"]["hidden"] == sample_template_category.hidden + + +def test_get_template_categories(client, notify_db, notify_db_session): + tc_hidden = create_template_category(notify_db, notify_db_session, name_en="hidden", name_fr="hidden(fr)", hidden=True) + tc_not_hidden = create_template_category(notify_db, notify_db_session, name_en="not hidden", name_fr="not hidden(fr)", hidden=False) + + auth_header = create_authorization_header() + response = client.get( + "/template/category", + headers=[("Content-Type", "application/json"), auth_header], + ) + + assert response.status_code == 200 + assert len(response.json["template_categories"]) == 2 \ No newline at end of file From a0fd75748ba52c25334a76618cab3bc409043ef3 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 19 Jun 2024 12:18:26 -0400 Subject: [PATCH 13/33] Implement filtering when fetching all categories - Add tests --- app/dao/template_categories_dao.py | 13 +- tests/app/dao/test_template_categories_dao.py | 214 ++++++++++++++---- 2 files changed, 187 insertions(+), 40 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 80b2b54490..318593d5b2 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,6 +1,7 @@ import uuid from sqlalchemy import asc +from sqlalchemy.orm import joinedload from flask import current_app from app import db @@ -25,7 +26,17 @@ def dao_get_template_category_by_template_id(template_id) -> TemplateCategory: # TODO: Add filters: Select all template categories used by at least 1 sms/email template def dao_get_all_template_categories(template_type=None, hidden=False): - return TemplateCategory.query.order_by(asc(TemplateCategory.name_en)).all() + query = TemplateCategory.query + + if template_type is not None: + query = query.join(Template).filter(Template.template_type == template_type) + + if not hidden: + query = query.filter(TemplateCategory.hidden == False) + + query = query.distinct() + + return query.all() @transactional diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index a0925cbe0b..91042bfa0b 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -1,3 +1,4 @@ +import pytest from app.dao.template_categories_dao import ( dao_create_template_category, dao_get_all_template_categories, @@ -7,7 +8,7 @@ ) from app.dao.templates_dao import dao_create_template from app.models import BULK, NORMAL, Template, TemplateCategory - +from tests.app.conftest import create_sample_template def test_create_template_category(notify_db_session): data = { @@ -107,45 +108,180 @@ def test_get_template_category_by_id(notify_db_session): assert dao_get_template_category_by_id(template_category.id) == template_category +def test_dao_get_all_template_categories_no_filtering(notify_db_session): + # Insert categories into the database + template_category1 = TemplateCategory( + name_en="english", + name_fr="french", + description_en="english description", + description_fr="french description", + sms_process_type="normal", + email_process_type="normal", + hidden=False, + ) + dao_create_template_category(template_category1) -def test_get_all_template_categories(notify_db_session): - data1 = { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - } + template_category2 = TemplateCategory( + name_en="english2", + name_fr="french2", + description_en="english description2", + description_fr="french description2", + sms_process_type="bulk", + email_process_type="bulk", + hidden=False, + ) + dao_create_template_category(template_category2) - data2 = { - "name_en": "english2", - "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", - "sms_process_type": BULK, - "email_process_type": BULK, - "hidden": True, - } + # Retrieve categories with no filters + retrieved_categories = dao_get_all_template_categories() - template_category1 = TemplateCategory(**data1) - template_category2 = TemplateCategory(**data2) - dao_create_template_category(template_category1) - dao_create_template_category(template_category2) + # Assertions + assert len(retrieved_categories) == 2 + assert template_category1 in retrieved_categories + assert template_category2 in retrieved_categories + + +@pytest.mark.parametrize( + "template_type, hidden, expected_count, categories_to_insert", + [ + # Filter by template type SMS + ("sms", None, 2, [ + { + "name_en": "english", + "name_fr": "french", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ]), + + # Filter by template type email + ("email", None, 2, [ + { + "name_en": "english", + "name_fr": "french", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ]), + + # Filter by hidden False + (None, False, 1, [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ]), + + # Filter by hidden True + (None, True, 2, [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ]), + + # Filter by template type SMS and hidden False + ("sms", False, 1, [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ]), + + # Filter by template type email and hidden True + ("email", True, 2, [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ]), + ] +) +def test_get_all_template_categories(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): + # Insert categories into the database + for category_data in categories_to_insert: + template_category = TemplateCategory(**category_data) + dao_create_template_category(template_category) + + # Email + create_sample_template(notify_db, notify_db_session, template_type='email', template_category=template_category) + # SMS + create_sample_template(notify_db, notify_db_session, template_type='sms', template_category=template_category) + + # Retrieve categories with the specified filters + retrieved_categories = dao_get_all_template_categories(template_type=template_type, hidden=hidden) - assert len(dao_get_all_template_categories()) == 2 - assert dao_get_all_template_categories()[0].name_en == "english" - assert dao_get_all_template_categories()[0].name_fr == "french" - assert dao_get_all_template_categories()[0].description_en == "english description" - assert dao_get_all_template_categories()[0].description_fr == "french description" - assert dao_get_all_template_categories()[0].sms_process_type == NORMAL - assert dao_get_all_template_categories()[0].email_process_type == NORMAL - assert not dao_get_all_template_categories()[0].hidden - assert dao_get_all_template_categories()[1].name_en == "english2" - assert dao_get_all_template_categories()[1].name_fr == "french2" - assert dao_get_all_template_categories()[1].description_en == "english description2" - assert dao_get_all_template_categories()[1].description_fr == "french description2" - assert dao_get_all_template_categories()[1].sms_process_type == BULK - assert dao_get_all_template_categories()[1].email_process_type == BULK - assert dao_get_all_template_categories()[1].hidden + # Assertions + assert len(retrieved_categories) == expected_count From e172bb7bd47a670d37a1e14367c7cdfdd4e29978 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 19 Jun 2024 12:24:56 -0400 Subject: [PATCH 14/33] Add lazy join on TemplateCategory --- app/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models.py b/app/models.py index ec3c486701..363f2b79d3 100644 --- a/app/models.py +++ b/app/models.py @@ -1213,7 +1213,7 @@ class Template(TemplateBase): service = db.relationship("Service", backref="templates") version = db.Column(db.Integer, default=0, nullable=False) - category = db.relationship("TemplateCategory", backref="templates") + category = db.relationship("TemplateCategory", lazy="joined", backref="templates") folder = db.relationship( "TemplateFolder", From 8a374c21b51552f41a7c90b474bb68ea2b0ebcce Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 19 Jun 2024 15:29:31 -0400 Subject: [PATCH 15/33] Clean up dao tests - Squash more bugs - Fix formatting - Added a couple more tests for the filters on dao_get_all_template_categories --- app/dao/template_categories_dao.py | 8 +- app/dao/templates_dao.py | 2 +- app/models.py | 2 +- app/template/template_category_rest.py | 6 +- tests/app/dao/test_template_categories_dao.py | 488 ++++++++++-------- .../template/test_template_category_rest.py | 9 +- 6 files changed, 295 insertions(+), 220 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 318593d5b2..d817718a54 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,12 +1,10 @@ import uuid -from sqlalchemy import asc from sqlalchemy.orm import joinedload from flask import current_app from app import db from app.dao.dao_utils import transactional -from app.errors import InvalidRequest from app.models import Template, TemplateCategory @@ -25,14 +23,14 @@ def dao_get_template_category_by_template_id(template_id) -> TemplateCategory: # TODO: Add filters: Select all template categories used by at least 1 sms/email template -def dao_get_all_template_categories(template_type=None, hidden=False): +def dao_get_all_template_categories(template_type=None, hidden=None): query = TemplateCategory.query if template_type is not None: query = query.join(Template).filter(Template.template_type == template_type) - if not hidden: - query = query.filter(TemplateCategory.hidden == False) + if hidden is not None: + query = query.filter(TemplateCategory.hidden == hidden) query = query.distinct() diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 26d83eb13a..8f06080986 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -89,7 +89,7 @@ def dao_update_template_process_type(template_id, process_type): history = TemplateHistory( **{ - "id": template.id, + "id": template.id, "name": template.name, "template_type": template.template_type, "created_at": template.created_at, diff --git a/app/models.py b/app/models.py index 363f2b79d3..a2e9488a52 100644 --- a/app/models.py +++ b/app/models.py @@ -1236,7 +1236,7 @@ def get_link(self): @property def template_process_type(self): """By default we use the process_type from TemplateCategory, but allow admins to override it on a per-template basis. - Only when overriden do we use the process_type from the template itself. + Only when overriden do we use the process_type from the template itself. """ if self.template_type == SMS_TYPE: return self.process_type if self.process_type else self.template_categories.sms_process_type diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index f9498bf7d7..78cb38787d 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -20,6 +20,7 @@ register_errors(template_category_blueprint) + @template_category_blueprint.route("", methods=["POST"]) def create_template_category(): data = request.get_json() @@ -33,7 +34,7 @@ def create_template_category(): @template_category_blueprint.route("", methods=["GET"]) -def get_template_category(template_category_id = None): +def get_template_category(template_category_id=None): template_category = dao_get_template_category_by_id(template_category_id) return jsonify(template_category=template_category_schema.dump(template_category)), 200 @@ -64,6 +65,7 @@ def update_template_category(template_category_id): return jsonify(template_category=category_to_update.dump()), 200 + @template_category_blueprint.route("/", methods=["DELETE"]) def delete_template_category(template_category_id): cascade = request.args.get("cascade", False) @@ -75,4 +77,4 @@ def delete_template_category(template_category_id): dao_delete_template_category_by_id(template_category_id, cascade) - return "", 204 \ No newline at end of file + return "", 204 diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index 91042bfa0b..7a83d1a675 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -1,4 +1,5 @@ import pytest + from app.dao.template_categories_dao import ( dao_create_template_category, dao_get_all_template_categories, @@ -10,6 +11,7 @@ from app.models import BULK, NORMAL, Template, TemplateCategory from tests.app.conftest import create_sample_template + def test_create_template_category(notify_db_session): data = { "name_en": "english", @@ -28,64 +30,75 @@ def test_create_template_category(notify_db_session): assert len(dao_get_all_template_categories()) == 1 -def test_update_template_category(notify_db_session): - data = { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": NORMAL, - "email_process_type": NORMAL, - "hidden": False, - } - - template_category = TemplateCategory(**data) +@pytest.mark.parametrize( + "category, updated_category", + [ + ( + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + }, + { + "name_en": "new english", + "name_fr": "new french", + "description_en": "new english description", + "description_fr": "new french description", + "sms_process_type": BULK, + "email_process_type": BULK, + "hidden": True, + }, + ) + ], +) +def test_update_template_category(notify_db_session, category, updated_category): + template_category = TemplateCategory(**category) dao_create_template_category(template_category) - template_category.name_en = "new english" - template_category.name_fr = "new french" - template_category.description_en = "new english description" - template_category.description_fr = "new french description" - template_category.sms_process_type = BULK - template_category.email_process_type = BULK - template_category.hidden = True + for key, value in updated_category.items(): + setattr(template_category, key, value) + dao_update_template_category(template_category) - assert TemplateCategory.query.count() == 1 - assert len(dao_get_all_template_categories()) == 1 - assert dao_get_all_template_categories()[0].name_en == "new english" - assert dao_get_all_template_categories()[0].name_fr == "new french" - assert dao_get_all_template_categories()[0].description_en == "new english description" - assert dao_get_all_template_categories()[0].description_fr == "new french description" - assert dao_get_all_template_categories()[0].sms_process_type == BULK - assert dao_get_all_template_categories()[0].email_process_type == BULK - assert dao_get_all_template_categories()[0].hidden - assert dao_get_all_template_categories()[0].id == template_category.id + fetched_category = dao_get_all_template_categories()[0] + assert fetched_category.id == template_category.id + for key, value in updated_category.items(): + assert getattr(fetched_category, key) == value -def test_get_template_category_by_template_id(notify_db_session, sample_service, sample_user): - category_data = { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": NORMAL, - "email_process_type": NORMAL, - "hidden": False, - } - template_category = TemplateCategory(**category_data) +@pytest.mark.parametrize( + "category, template", + [ + ( + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": NORMAL, + "email_process_type": NORMAL, + "hidden": False, + }, + { + "name": "Sample Template", + "template_type": "email", + "content": "Template content", + }, + ) + ], +) +def test_get_template_category_by_template_id(category, template, notify_db_session, sample_service, sample_user): + template_category = TemplateCategory(**category) dao_create_template_category(template_category) - template_data = { - "name": "Sample Template", - "template_type": "email", - "content": "Template content", - "service": sample_service, - "created_by": sample_user, - } - - template = Template(**template_data) + template = Template(**template) + template.service = sample_service + template.created_by = sample_user template.template_category = template_category dao_create_template(template) @@ -108,180 +121,243 @@ def test_get_template_category_by_id(notify_db_session): assert dao_get_template_category_by_id(template_category.id) == template_category -def test_dao_get_all_template_categories_no_filtering(notify_db_session): - # Insert categories into the database - template_category1 = TemplateCategory( - name_en="english", - name_fr="french", - description_en="english description", - description_fr="french description", - sms_process_type="normal", - email_process_type="normal", - hidden=False, - ) - dao_create_template_category(template_category1) - - template_category2 = TemplateCategory( - name_en="english2", - name_fr="french2", - description_en="english description2", - description_fr="french description2", - sms_process_type="bulk", - email_process_type="bulk", - hidden=False, - ) - dao_create_template_category(template_category2) - - # Retrieve categories with no filters - retrieved_categories = dao_get_all_template_categories() - - # Assertions - assert len(retrieved_categories) == 2 - assert template_category1 in retrieved_categories - assert template_category2 in retrieved_categories - @pytest.mark.parametrize( "template_type, hidden, expected_count, categories_to_insert", [ + ( + None, + None, + 2, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ], + ), # Filter by template type SMS - ("sms", None, 2, [ - { - "name_en": "english", - "name_fr": "french", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": False, - }, - ]), - + ( + "sms", + None, + 2, + [ + { + "name_en": "english", + "name_fr": "french", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ], + ), # Filter by template type email - ("email", None, 2, [ - { - "name_en": "english", - "name_fr": "french", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": False, - }, - ]), - + ( + "email", + None, + 2, + [ + { + "name_en": "english", + "name_fr": "french", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ], + ), # Filter by hidden False - (None, False, 1, [ - { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": True, - }, - ]), - + ( + None, + False, + 1, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ], + ), # Filter by hidden True - (None, True, 2, [ - { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": True, - }, - ]), - + ( + None, + True, + 1, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ], + ), # Filter by template type SMS and hidden False - ("sms", False, 1, [ - { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": True, - }, - ]), - + ( + "sms", + False, + 1, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ], + ), + ( + "sms", + False, + 0, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": True, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ], + ), # Filter by template type email and hidden True - ("email", True, 2, [ - { - "name_en": "english", - "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", - "sms_process_type": "normal", - "email_process_type": "normal", - "hidden": False, - }, - { - "name_en": "english2", - "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", - "sms_process_type": "bulk", - "email_process_type": "bulk", - "hidden": True, - }, - ]), - ] + ( + "email", + True, + 1, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": True, + }, + ], + ), + ( + "email", + True, + 0, + [ + { + "name_en": "english", + "name_fr": "french", + "description_en": "english description", + "description_fr": "french description", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": False, + }, + { + "name_en": "english2", + "name_fr": "french2", + "description_en": "english description2", + "description_fr": "french description2", + "sms_process_type": "bulk", + "email_process_type": "bulk", + "hidden": False, + }, + ], + ), + ], ) def test_get_all_template_categories(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): - # Insert categories into the database for category_data in categories_to_insert: template_category = TemplateCategory(**category_data) dao_create_template_category(template_category) - # Email - create_sample_template(notify_db, notify_db_session, template_type='email', template_category=template_category) - # SMS - create_sample_template(notify_db, notify_db_session, template_type='sms', template_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) - # Retrieve categories with the specified filters retrieved_categories = dao_get_all_template_categories(template_type=template_type, hidden=hidden) - # Assertions assert len(retrieved_categories) == expected_count diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index a2e4170256..977cff3c12 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -1,6 +1,3 @@ -import json -from flask import url_for - from tests import create_authorization_header from tests.app.conftest import create_sample_template, create_template_category @@ -72,7 +69,9 @@ def test_get_template_category_by_template_id(client, notify_db, notify_db_sessi def test_get_template_categories(client, notify_db, notify_db_session): tc_hidden = create_template_category(notify_db, notify_db_session, name_en="hidden", name_fr="hidden(fr)", hidden=True) - tc_not_hidden = create_template_category(notify_db, notify_db_session, name_en="not hidden", name_fr="not hidden(fr)", hidden=False) + tc_not_hidden = create_template_category( + notify_db, notify_db_session, name_en="not hidden", name_fr="not hidden(fr)", hidden=False + ) auth_header = create_authorization_header() response = client.get( @@ -81,4 +80,4 @@ def test_get_template_categories(client, notify_db, notify_db_session): ) assert response.status_code == 200 - assert len(response.json["template_categories"]) == 2 \ No newline at end of file + assert len(response.json["template_categories"]) == 2 From 46d0130aa1f2568bd5e69daffbbd307db476c7d3 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 19 Jun 2024 17:26:25 -0400 Subject: [PATCH 16/33] Add tests for deleting a template category - Excluded the template_category table from deletion in notify_db_session to preserve the 3 generic template categories between test runs - Fixed inserts in the migration, apparently alembic / sqlalchemy doesn't like multi-line f-strings - Made a few tests shorter by excluding the description_en and description_fr columns as they are optional --- app/dao/template_categories_dao.py | 2 - .../versions/0453_add_template_category.py | 31 ++++------ tests/app/dao/test_template_categories_dao.py | 57 +++++++++---------- tests/conftest.py | 1 + 4 files changed, 39 insertions(+), 52 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index d817718a54..57a9aab91f 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,7 +1,5 @@ import uuid -from sqlalchemy.orm import joinedload - from flask import current_app from app import db from app.dao.dao_utils import transactional diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index 8daf18145c..8ce8a161b2 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -31,6 +31,17 @@ def upgrade(): sa.UniqueConstraint("name_fr"), ) + # Insert the generic low, medium, and high categories + op.execute( + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]) + ) + op.execute( + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"]) + ) + op.execute( + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"]) + ) + op.add_column("templates", sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True)) op.add_column("templates_history", sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True)) op.create_index( @@ -53,26 +64,6 @@ def upgrade(): ) op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) - # Insert the generic low, medium, and high categories - op.execute( - f""" - INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true) - """ - ) - op.execute( - f""" - INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"]}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true) - """ - ) - op.execute( - f""" - INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) - VALUES ('{current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"]}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true) - """ - ) - def downgrade(): op.drop_constraint("fk_template_template_categories", "templates", type_="foreignkey") diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index 7a83d1a675..793e75a48e 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -1,3 +1,4 @@ +from flask import current_app import pytest from app.dao.template_categories_dao import ( @@ -6,8 +7,9 @@ dao_get_template_category_by_id, dao_get_template_category_by_template_id, dao_update_template_category, + dao_delete_template_category_by_id ) -from app.dao.templates_dao import dao_create_template +from app.dao.templates_dao import dao_create_template, dao_get_template_by_id from app.models import BULK, NORMAL, Template, TemplateCategory from tests.app.conftest import create_sample_template @@ -133,8 +135,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -142,8 +142,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": False, @@ -203,8 +201,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -212,8 +208,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": True, @@ -229,8 +223,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -238,8 +230,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": True, @@ -255,8 +245,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -264,8 +252,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": True, @@ -280,8 +266,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": True, @@ -289,8 +273,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": True, @@ -306,8 +288,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -315,8 +295,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": True, @@ -331,8 +309,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english", "name_fr": "french", - "description_en": "english description", - "description_fr": "french description", "sms_process_type": "normal", "email_process_type": "normal", "hidden": False, @@ -340,8 +316,6 @@ def test_get_template_category_by_id(notify_db_session): { "name_en": "english2", "name_fr": "french2", - "description_en": "english description2", - "description_fr": "french description2", "sms_process_type": "bulk", "email_process_type": "bulk", "hidden": False, @@ -350,7 +324,7 @@ def test_get_template_category_by_id(notify_db_session): ), ], ) -def test_get_all_template_categories(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): +def test_get_all_template_categories_with_filters(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): for category_data in categories_to_insert: template_category = TemplateCategory(**category_data) dao_create_template_category(template_category) @@ -361,3 +335,26 @@ def test_get_all_template_categories(template_type, hidden, expected_count, cate retrieved_categories = dao_get_all_template_categories(template_type=template_type, hidden=hidden) assert len(retrieved_categories) == expected_count + + +def test_dao_delete_template_category_by_id_should_delete_category_when_no_associated_templates(notify_db_session, sample_template_category): + dao_delete_template_category_by_id(sample_template_category.id) + + assert TemplateCategory.query.count() == 0 + + +def test_dao_delete_template_category_by_id_should_not_allow_deletion_when_associated_with_template(notify_db, notify_db_session, 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) + + assert TemplateCategory.query.count() == 1 + + +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): + 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 + assert TemplateCategory.query.count() == 3 + assert str(template.template_category_id) == current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"] \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index cfa57a7637..cfa9556a67 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -141,6 +141,7 @@ def notify_db_session(notify_db): "auth_type", "invite_status_type", "service_callback_type", + "template_categories", ]: notify_db.engine.execute(tbl.delete()) notify_db.session.commit() From 98c93bf991ec935de2fef502ee667fce81436ad1 Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 20 Jun 2024 16:08:08 -0400 Subject: [PATCH 17/33] Add API tests, squash bugs - Allow passing of a uuid to dao_create_template_category - Fixed issues with get_template_categories and delete_template_category filters / flags - Added a fixture to re-populate the template_category table with generic categories and removed template_categories from the list of tables that are excluded from the post-test db clear --- app/dao/template_categories_dao.py | 3 +- app/template/template_category_rest.py | 47 ++++++++-- tests/app/conftest.py | 41 +++++++++ tests/app/dao/test_template_categories_dao.py | 3 +- .../template/test_template_category_rest.py | 88 +++++++++++++++++-- tests/conftest.py | 1 - 6 files changed, 165 insertions(+), 18 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 57a9aab91f..58d5b19112 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -8,7 +8,8 @@ @transactional def dao_create_template_category(template_category: TemplateCategory): - template_category.id = uuid.uuid4() + if template_category.id is None: + template_category.id = uuid.uuid4() db.session.add(template_category) diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index 78cb38787d..4d3ce28639 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -47,7 +47,21 @@ def get_template_category_by_template_id(template_id): @template_category_blueprint.route("", methods=["GET"]) def get_template_categories(): - template_categories = template_category_schema.dump(dao_get_all_template_categories(), many=True) + template_type = request.args.get("template_type", None) + hidden = request.args.get("hidden", None) + + # Validate request args + if template_type is not None: + if template_type not in ["sms", "email"]: + return jsonify(message="Invalid filter 'template_type', valid template_types: 'sms', 'email'"), 400 + + if hidden is not None: + try: + hidden = _coerce_to_boolean(hidden) + except ValueError: + return jsonify(message="Invalid filter 'hidden', must be a boolean."), 400 + + template_categories = template_category_schema.dump(dao_get_all_template_categories(template_type, hidden), many=True) return jsonify(template_categories=template_categories), 200 @@ -70,11 +84,30 @@ def update_template_category(template_category_id): def delete_template_category(template_category_id): cascade = request.args.get("cascade", False) - if not cascade: - templates = Template.query.filter_by(template_category_id=template_category_id).one() - if templates: - return jsonify(message="Cannot delete template category with templates assigned to it"), 400 + try: + cascade = _coerce_to_boolean(cascade) + except ValueError: + return jsonify(message="Invalid query parameter 'cascade', must be a boolean."), 400 - dao_delete_template_category_by_id(template_category_id, cascade) + if cascade: + dao_delete_template_category_by_id(template_category_id, cascade) + return "", 200 - return "", 204 + template_category = dao_get_template_category_by_id(template_category_id) + if len(template_category.templates) > 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) + return "", 200 + + +def _coerce_to_boolean(value): + if isinstance(value, bool): + return value + if isinstance(value, str): + lower = value.lower() + if lower in ["true", "1"]: + return True + elif lower in ["false", "0"]: + return False + raise ValueError(f"Could not coerce '{value}' to a boolean") \ No newline at end of file diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e05d16a8b4..4404ec0917 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -232,6 +232,47 @@ def _sample_service_custom_letter_contact_block(sample_service): return 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, template_category=sample_template_category) + create_sample_template(notify_db, notify_db_session, template_category=sample_template_category) + return sample_template_category + + +@pytest.fixture(scope="function") +def populate_generic_categories(notify_db_session): + generic_categories = [ + { + "id": current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"], + "name_en": "Low Category (Bulk)", + "name_fr": "Catégorie Basse (En Vrac)", + "sms_process_type": "low", + "email_process_type": "low", + "hidden": True, + }, + { + "id": current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"], + "name_en": "Medium Category (Normal)", + "name_fr": "Catégorie Moyenne (Normale)", + "sms_process_type": "normal", + "email_process_type": "normal", + "hidden": True, + }, + { + "id": current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"], + "name_en": "High Category (Priority)", + "name_fr": "Catégorie Haute (Priorité)", + "sms_process_type": "high", + "email_process_type": "high", + "hidden": True, + }, + ] + for category in generic_categories: + dao_create_template_category(TemplateCategory(**category)) + + yield + + @pytest.fixture(scope="function") def sample_template_category( notify_db, diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index 793e75a48e..b6d67d9573 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -325,6 +325,7 @@ def test_get_template_category_by_id(notify_db_session): ], ) def test_get_all_template_categories_with_filters(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): + for category_data in categories_to_insert: template_category = TemplateCategory(**category_data) dao_create_template_category(template_category) @@ -351,7 +352,7 @@ def test_dao_delete_template_category_by_id_should_not_allow_deletion_when_assoc assert TemplateCategory.query.count() == 1 -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): +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, template_category=sample_template_category) dao_delete_template_category_by_id(sample_template_category.id, cascade=True) diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index 977cff3c12..8b54482703 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -1,3 +1,8 @@ +from urllib.parse import urlencode +import uuid +from flask import url_for +import pytest + from tests import create_authorization_header from tests.app.conftest import create_sample_template, create_template_category @@ -67,17 +72,84 @@ def test_get_template_category_by_template_id(client, notify_db, notify_db_sessi assert response.json["template_category"]["hidden"] == sample_template_category.hidden -def test_get_template_categories(client, notify_db, notify_db_session): - tc_hidden = create_template_category(notify_db, notify_db_session, name_en="hidden", name_fr="hidden(fr)", hidden=True) - tc_not_hidden = create_template_category( - notify_db, notify_db_session, name_en="not hidden", name_fr="not hidden(fr)", hidden=False +@pytest.mark.parametrize( + "template_type, hidden, expected_status_code, expected_msg", + [ + ("invalid_template_type", True, 400, "Invalid filter 'template_type', valid template_types: 'sms', 'email'"), + ("sms", "not_a_boolean", 400, "Invalid filter 'hidden', must be a boolean."), + ("email", "True", 200, None), + ("email", "False", 200, None), + ("email", None, 200, None), + ("sms", "True", 200, None), + ("sms", "False", 200, None), + ("sms", None, 200, None), + (None, None, 200, None), + (None, "True", 200, None), + (None, "False", 200, None) + ] +) +def test_get_template_categories(template_type, hidden, expected_status_code, expected_msg, sample_template_category, client, notify_db, notify_db_session, mocker): + auth_header = create_authorization_header() + + query_params = {} + if template_type: + query_params["template_type"] = template_type + if hidden: + query_params["hidden"] = hidden + + query_string = f"?{urlencode(query_params)}" if len(query_params) > 0 else "" + + mocker.patch("app.dao.template_categories_dao.dao_get_all_template_categories", return_value=[sample_template_category]) + + response = client.get( + f"/template/category{query_string}", + headers=[("Content-Type", "application/json"), auth_header], ) + assert response.status_code == expected_status_code + if not expected_status_code == 200: + assert response.json['message'] == expected_msg + + +def test_delete_template_category_query_param_validation(client): auth_header = create_authorization_header() - response = client.get( - "/template/category", + + endpoint = url_for( + "template_category.delete_template_category", + template_category_id=str(uuid.uuid4()), + cascade="not_a_boolean" + ) + + response = client.delete( + endpoint, headers=[("Content-Type", "application/json"), auth_header], ) - assert response.status_code == 200 - assert len(response.json["template_categories"]) == 2 + assert response.status_code == 400 + assert response.json["message"] == "Invalid query parameter 'cascade', must be a boolean." + +@pytest.mark.parametrize( + "cascade, expected_status_code, expected_msg", + [ + ("True", 200, ""), + ("False", 400, "Cannot delete a template category with templates assigned to it."), + ], +) +def test_delete_template_category_cascade(cascade, expected_status_code, expected_msg, client, mocker, sample_template_category_with_templates): + 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", + template_category_id=sample_template_category_with_templates.id, + cascade=cascade + ) + + response = client.delete( + endpoint, + headers=[("Content-Type", "application/json"), auth_header], + ) + + assert response.status_code == expected_status_code + if expected_status_code == 400: + assert response.json["message"] == expected_msg diff --git a/tests/conftest.py b/tests/conftest.py index cfa9556a67..cfa57a7637 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -141,7 +141,6 @@ def notify_db_session(notify_db): "auth_type", "invite_status_type", "service_callback_type", - "template_categories", ]: notify_db.engine.execute(tbl.delete()) notify_db.session.commit() From c7cab45173e01517ed93bf7fb67ed4a8fb0476d6 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 24 Jun 2024 10:51:37 -0400 Subject: [PATCH 18/33] Fix pre-existing tests - formatting --- app/dao/template_categories_dao.py | 17 +++++--- app/models.py | 1 + app/template/template_category_rest.py | 4 +- .../versions/0453_add_template_category.py | 12 ++++-- tests/app/dao/test_template_categories_dao.py | 27 ++++++++----- .../template/test_template_category_rest.py | 40 +++++++++++++------ 6 files changed, 67 insertions(+), 34 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 58d5b19112..8601cb4815 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,6 +1,7 @@ import uuid from flask import current_app + from app import db from app.dao.dao_utils import transactional from app.models import Template, TemplateCategory @@ -42,7 +43,7 @@ def dao_update_template_category(template_category: TemplateCategory): @transactional -def dao_delete_template_category_by_id(template_category_id, cascade = False): +def dao_delete_template_category_by_id(template_category_id, cascade=False): """ Deletes a `TemplateCategory`. By default, if the `TemplateCategory` is associated with any `Template`, it will not be deleted. If the `cascade` option is specified then the category will be forcible removed: @@ -65,7 +66,11 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): if cascade: try: for template in templates: - process_type = template_category.sms_process_type if template.template_type == 'sms' else template_category.email_process_type + process_type = ( + template_category.sms_process_type + if template.template_type == "sms" + else template_category.email_process_type + ) template.category = dao_get_template_category_by_id(_get_default_category_id(process_type)) db.session.add(template) @@ -80,8 +85,8 @@ def dao_delete_template_category_by_id(template_category_id, cascade = False): def _get_default_category_id(process_type): default_categories = { - 'bulk': current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW'], - 'normal': current_app.config['DEFAULT_TEMPLATE_CATEGORY_MEDIUM'], - 'priority': current_app.config['DEFAULT_TEMPLATE_CATEGORY_HIGH'] + "bulk": current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"], + "normal": current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"], + "priority": current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"], } - return default_categories.get(process_type, current_app.config['DEFAULT_TEMPLATE_CATEGORY_LOW']) + return default_categories.get(process_type, current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]) diff --git a/app/models.py b/app/models.py index a2e9488a52..dda2430326 100644 --- a/app/models.py +++ b/app/models.py @@ -1292,6 +1292,7 @@ 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): diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index 4d3ce28639..d5f52286e8 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 Template, TemplateCategory +from app.models import TemplateCategory from app.schemas import template_category_schema template_category_blueprint = Blueprint( @@ -110,4 +110,4 @@ def _coerce_to_boolean(value): return True elif lower in ["false", "0"]: return False - raise ValueError(f"Could not coerce '{value}' to a boolean") \ No newline at end of file + raise ValueError(f"Could not coerce '{value}' to a boolean") diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index 8ce8a161b2..3d71d228aa 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -33,13 +33,19 @@ def upgrade(): # Insert the generic low, medium, and high categories op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]) + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true)".format( + current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"] + ) ) op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"]) + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true)".format( + current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"] + ) ) op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true)".format(current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"]) + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true)".format( + current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"] + ) ) op.add_column("templates", sa.Column("template_category_id", postgresql.UUID(as_uuid=True), nullable=True)) diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index b6d67d9573..6f15b9c729 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -1,15 +1,15 @@ -from flask import current_app import pytest +from flask import current_app from app.dao.template_categories_dao import ( dao_create_template_category, + dao_delete_template_category_by_id, dao_get_all_template_categories, dao_get_template_category_by_id, dao_get_template_category_by_template_id, dao_update_template_category, - dao_delete_template_category_by_id ) -from app.dao.templates_dao import dao_create_template, dao_get_template_by_id +from app.dao.templates_dao import dao_create_template from app.models import BULK, NORMAL, Template, TemplateCategory from tests.app.conftest import create_sample_template @@ -324,8 +324,9 @@ def test_get_template_category_by_id(notify_db_session): ), ], ) -def test_get_all_template_categories_with_filters(template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session): - +def test_get_all_template_categories_with_filters( + template_type, hidden, expected_count, categories_to_insert, notify_db, notify_db_session +): for category_data in categories_to_insert: template_category = TemplateCategory(**category_data) dao_create_template_category(template_category) @@ -338,24 +339,30 @@ def test_get_all_template_categories_with_filters(template_type, hidden, expecte assert len(retrieved_categories) == expected_count -def test_dao_delete_template_category_by_id_should_delete_category_when_no_associated_templates(notify_db_session, sample_template_category): +def test_dao_delete_template_category_by_id_should_delete_category_when_no_associated_templates( + notify_db_session, sample_template_category +): dao_delete_template_category_by_id(sample_template_category.id) assert TemplateCategory.query.count() == 0 -def test_dao_delete_template_category_by_id_should_not_allow_deletion_when_associated_with_template(notify_db, notify_db_session, sample_template_category): - template = create_sample_template(notify_db, notify_db_session, template_category=sample_template_category) +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, template_category=sample_template_category) dao_delete_template_category_by_id(sample_template_category.id) assert TemplateCategory.query.count() == 1 -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): +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, 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 assert TemplateCategory.query.count() == 3 - assert str(template.template_category_id) == current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"] \ No newline at end of file + assert str(template.template_category_id) == current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"] diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index 8b54482703..4c1d6f9694 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -1,10 +1,11 @@ -from urllib.parse import urlencode import uuid -from flask import url_for +from urllib.parse import urlencode + import pytest +from flask import url_for from tests import create_authorization_header -from tests.app.conftest import create_sample_template, create_template_category +from tests.app.conftest import create_sample_template def test_should_create_new_template_category(client, notify_db, notify_db_session): @@ -85,10 +86,20 @@ def test_get_template_category_by_template_id(client, notify_db, notify_db_sessi ("sms", None, 200, None), (None, None, 200, None), (None, "True", 200, None), - (None, "False", 200, None) - ] + (None, "False", 200, None), + ], ) -def test_get_template_categories(template_type, hidden, expected_status_code, expected_msg, sample_template_category, client, notify_db, notify_db_session, mocker): +def test_get_template_categories( + template_type, + hidden, + expected_status_code, + expected_msg, + sample_template_category, + client, + notify_db, + notify_db_session, + mocker, +): auth_header = create_authorization_header() query_params = {} @@ -108,16 +119,14 @@ def test_get_template_categories(template_type, hidden, expected_status_code, ex assert response.status_code == expected_status_code if not expected_status_code == 200: - assert response.json['message'] == expected_msg + assert response.json["message"] == expected_msg def test_delete_template_category_query_param_validation(client): auth_header = create_authorization_header() endpoint = url_for( - "template_category.delete_template_category", - template_category_id=str(uuid.uuid4()), - cascade="not_a_boolean" + "template_category.delete_template_category", template_category_id=str(uuid.uuid4()), cascade="not_a_boolean" ) response = client.delete( @@ -128,6 +137,7 @@ def test_delete_template_category_query_param_validation(client): assert response.status_code == 400 assert response.json["message"] == "Invalid query parameter 'cascade', must be a boolean." + @pytest.mark.parametrize( "cascade, expected_status_code, expected_msg", [ @@ -135,14 +145,18 @@ def test_delete_template_category_query_param_validation(client): ("False", 400, "Cannot delete a template category with templates assigned to it."), ], ) -def test_delete_template_category_cascade(cascade, expected_status_code, expected_msg, client, mocker, sample_template_category_with_templates): +def test_delete_template_category_cascade( + cascade, expected_status_code, expected_msg, client, mocker, sample_template_category_with_templates +): 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) + 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", template_category_id=sample_template_category_with_templates.id, - cascade=cascade + cascade=cascade, ) response = client.delete( From 60555865917d751431f46af909f1a55e50500574 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 24 Jun 2024 12:48:44 -0400 Subject: [PATCH 19/33] Misc. fixes - Fix incorrectly named prop call in dao - Added category as a declared attribute in TemplateBase model - Added a proper route to get a category by template id - Added a foreign key to templates_history for the category id - Fix a couple more tests --- app/dao/template_categories_dao.py | 2 +- app/models.py | 4 ++++ app/template/template_category_rest.py | 6 +++--- .../versions/0453_add_template_category.py | 4 ++++ tests/app/dao/test_template_categories_dao.py | 4 ++-- .../template/test_template_category_rest.py | 21 +++++++++++-------- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 8601cb4815..d0ec0da93a 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -19,7 +19,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().template_category + return Template.query.filter_by(id=template_id).one().category # TODO: Add filters: Select all template categories used by at least 1 sms/email template diff --git a/app/models.py b/app/models.py index dda2430326..c8e637f0e0 100644 --- a/app/models.py +++ b/app/models.py @@ -1112,6 +1112,10 @@ def created_by_id(cls): 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") + @declared_attr def created_by(cls): return db.relationship("User") diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index d5f52286e8..cb17f5e19f 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -33,13 +33,13 @@ def create_template_category(): return jsonify(template_category=template_category_schema.dump(template_category)), 201 -@template_category_blueprint.route("", methods=["GET"]) -def get_template_category(template_category_id=None): +@template_category_blueprint.route("/", methods=["GET"]) +def get_template_category(template_category_id): template_category = dao_get_template_category_by_id(template_category_id) return jsonify(template_category=template_category_schema.dump(template_category)), 200 -@template_category_blueprint.route("", methods=["GET"]) +@template_category_blueprint.route("/by-template-id/", methods=["GET"]) def get_template_category_by_template_id(template_id): template_category = dao_get_template_category_by_template_id(template_id) return jsonify(template_category=template_category_schema.dump(template_category)), 200 diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index 3d71d228aa..44b5e925c0 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -69,10 +69,14 @@ def upgrade(): unique=False, ) op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) + op.create_foreign_key( + "fk_template_history_template_categories", "templates_history", "template_categories", ["template_category_id"], ["id"] + ) def downgrade(): op.drop_constraint("fk_template_template_categories", "templates", type_="foreignkey") + op.drop_constraint("fk_template_history_template_categories", "templates_history", type_="foreignkey") op.drop_index(op.f("ix_template_category_id"), table_name="templates") op.drop_index(op.f("ix_template_categories_name_en"), table_name="template_categories") op.drop_index(op.f("ix_template_categories_name_fr"), table_name="template_categories") diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index 6f15b9c729..b2874a366f 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -94,14 +94,14 @@ def test_update_template_category(notify_db_session, category, updated_category) ) ], ) -def test_get_template_category_by_template_id(category, template, notify_db_session, sample_service, sample_user): +def test_dao_get_template_category_by_template_id(category, template, notify_db_session, sample_service, sample_user): template_category = TemplateCategory(**category) dao_create_template_category(template_category) template = Template(**template) template.service = sample_service template.created_by = sample_user - template.template_category = template_category + template.category = template_category dao_create_template(template) assert dao_get_template_category_by_template_id(template.id) == template_category diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index 4c1d6f9694..139facfec5 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -55,22 +55,25 @@ 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): - template = create_sample_template(notify_db, notify_db_session, template_category=sample_template_category) + category = sample_template_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) + response = client.get( - f"/template/category/{template.id}", + endpoint, headers=[("Content-Type", "application/json"), auth_header], ) assert response.status_code == 200 - assert response.json["template_category"]["name_en"] == sample_template_category.name_en - assert response.json["template_category"]["name_fr"] == sample_template_category.name_fr - assert response.json["template_category"]["description_en"] == sample_template_category.description_en - assert response.json["template_category"]["description_fr"] == sample_template_category.description_fr - assert response.json["template_category"]["sms_process_type"] == sample_template_category.sms_process_type - assert response.json["template_category"]["email_process_type"] == sample_template_category.email_process_type - assert response.json["template_category"]["hidden"] == sample_template_category.hidden + assert response.json["template_category"]["name_en"] == category.name_en + assert response.json["template_category"]["name_fr"] == category.name_fr + assert response.json["template_category"]["description_en"] == category.description_en + assert response.json["template_category"]["description_fr"] == category.description_fr + assert response.json["template_category"]["sms_process_type"] == category.sms_process_type + assert response.json["template_category"]["email_process_type"] == category.email_process_type + assert response.json["template_category"]["hidden"] == category.hidden @pytest.mark.parametrize( From 231128f4bab185e0af669764b498405403fe64bd Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 24 Jun 2024 12:56:59 -0400 Subject: [PATCH 20/33] We definitely didn't want that FK on templatehistory... --- migrations/versions/0453_add_template_category.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index 44b5e925c0..3d71d228aa 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -69,14 +69,10 @@ def upgrade(): unique=False, ) op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) - op.create_foreign_key( - "fk_template_history_template_categories", "templates_history", "template_categories", ["template_category_id"], ["id"] - ) def downgrade(): op.drop_constraint("fk_template_template_categories", "templates", type_="foreignkey") - op.drop_constraint("fk_template_history_template_categories", "templates_history", type_="foreignkey") op.drop_index(op.f("ix_template_category_id"), table_name="templates") op.drop_index(op.f("ix_template_categories_name_en"), table_name="template_categories") op.drop_index(op.f("ix_template_categories_name_fr"), table_name="template_categories") From 33006d7279122dbaa839ea353bad26d1b748f5df Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 24 Jun 2024 16:50:32 -0400 Subject: [PATCH 21/33] Logic cleanups - Tweak tests - No longer excluding template_category_id in template_category_schema - Updated migration revision as 0453 was added for pinpoint work --- app/dao/template_categories_dao.py | 3 +- app/schemas.py | 1 - app/template/rest.py | 4 +- app/template/template_category_rest.py | 62 ++++++++----------- .../versions/0453_add_template_category.py | 8 +-- .../template/test_template_category_rest.py | 19 +----- 6 files changed, 35 insertions(+), 62 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index d0ec0da93a..818840ff04 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -32,14 +32,13 @@ def dao_get_all_template_categories(template_type=None, hidden=None): if hidden is not None: query = query.filter(TemplateCategory.hidden == hidden) - query = query.distinct() - return query.all() @transactional def dao_update_template_category(template_category: TemplateCategory): db.session.add(template_category) + db.session.commit() @transactional diff --git a/app/schemas.py b/app/schemas.py index 79996c0949..644ffda597 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -110,7 +110,6 @@ def make_instance(self, data, **kwargs): class TemplateCategorySchema(BaseSchema): class Meta(BaseSchema.Meta): model = models.TemplateCategory - exclude = ("id",) @validates("name_en") def validate_name_en(self, value): diff --git a/app/template/rest.py b/app/template/rest.py index 9f063e54f3..4e376351c8 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -134,8 +134,8 @@ def create_template(service_id): return jsonify(data=template_schema.dump(new_template)), 201 -@template_blueprint.route("//template-category/", methods=["POST"]) -def update_templates_category(template_id, template_category_id): +@template_blueprint.route("//category/", methods=["POST"]) +def update_templates_category(service_id, template_id, template_category_id): updated = dao_update_template_category(template_id, template_category_id) return jsonify(data=template_schema.dump(updated)), 200 diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index cb17f5e19f..60436322ec 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -48,49 +48,53 @@ def get_template_category_by_template_id(template_id): @template_category_blueprint.route("", methods=["GET"]) def get_template_categories(): template_type = request.args.get("template_type", None) - hidden = request.args.get("hidden", None) + + hidden = request.args.get("hidden") + if hidden is not None: + if hidden == "True": + hidden = True + elif hidden == "False": + hidden = False + else: + hidden = None # Validate request args if template_type is not None: if template_type not in ["sms", "email"]: return jsonify(message="Invalid filter 'template_type', valid template_types: 'sms', 'email'"), 400 - if hidden is not None: - try: - hidden = _coerce_to_boolean(hidden) - except ValueError: - return jsonify(message="Invalid filter 'hidden', must be a boolean."), 400 - template_categories = template_category_schema.dump(dao_get_all_template_categories(template_type, hidden), many=True) return jsonify(template_categories=template_categories), 200 @template_category_blueprint.route("/", methods=["POST"]) def update_template_category(template_category_id): - request_json = request.get_json() - update_dict = template_category_schema.load(request_json) - - category_to_update = dao_get_template_category_by_id(template_category_id) + current_category = dict(template_category_schema.dump(dao_get_template_category_by_id(template_category_id))) + current_category.update(request.get_json()) - for key in request_json: - setattr(category_to_update, key, update_dict[key]) + updated_category = template_category_schema.load(current_category) + dao_update_template_category(updated_category) - dao_update_template_category(category_to_update) - - return jsonify(template_category=category_to_update.dump()), 200 + return jsonify(template_category=template_category_schema.dump(updated_category)), 200 @template_category_blueprint.route("/", methods=["DELETE"]) def delete_template_category(template_category_id): - cascade = request.args.get("cascade", False) + """Deletes a template category. By default, if the template category is associated with any template, it will not be deleted. + This can be overriden by specifying the `cascade` query parameter. + + Args: + template_category_id (str): The id of the template_category to delete + + Request Args: + cascade (bool, optional): Specify whether to dissociate the category from templates that use it to force removal. Defaults to False. - try: - cascade = _coerce_to_boolean(cascade) - except ValueError: - return jsonify(message="Invalid query parameter 'cascade', must be a boolean."), 400 + Returns: + (flask.Response): The response message and http status code. + """ - if cascade: - dao_delete_template_category_by_id(template_category_id, cascade) + if request.args.get("cascade") == "True": + dao_delete_template_category_by_id(template_category_id, cascade=True) return "", 200 template_category = dao_get_template_category_by_id(template_category_id) @@ -99,15 +103,3 @@ def delete_template_category(template_category_id): else: dao_delete_template_category_by_id(template_category_id) return "", 200 - - -def _coerce_to_boolean(value): - if isinstance(value, bool): - return value - if isinstance(value, str): - lower = value.lower() - if lower in ["true", "1"]: - return True - elif lower in ["false", "0"]: - return False - raise ValueError(f"Could not coerce '{value}' to a boolean") diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0453_add_template_category.py index 3d71d228aa..5170aa81c1 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0453_add_template_category.py @@ -1,7 +1,7 @@ """ -Revision ID: 0453_add_template_categories -Revises: 0452_set_pgaudit_config +Revision ID: 0454_add_template_categories +Revises: 0453_set_supports_international Create Date: 2024-06-11 13:32:00 """ @@ -12,8 +12,8 @@ from flask import current_app from sqlalchemy.dialects import postgresql -revision = "0453_add_template_categories" -down_revision = "0452_set_pgaudit_config" +revision = "0454_add_template_categories" +down_revision = "0453_set_supports_international" def upgrade(): diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index 139facfec5..900df1a7f6 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -1,4 +1,3 @@ -import uuid from urllib.parse import urlencode import pytest @@ -80,7 +79,7 @@ def test_get_template_category_by_template_id(client, notify_db, notify_db_sessi "template_type, hidden, expected_status_code, expected_msg", [ ("invalid_template_type", True, 400, "Invalid filter 'template_type', valid template_types: 'sms', 'email'"), - ("sms", "not_a_boolean", 400, "Invalid filter 'hidden', must be a boolean."), + ("sms", "not_a_boolean", 200, None), ("email", "True", 200, None), ("email", "False", 200, None), ("email", None, 200, None), @@ -125,22 +124,6 @@ def test_get_template_categories( assert response.json["message"] == expected_msg -def test_delete_template_category_query_param_validation(client): - auth_header = create_authorization_header() - - endpoint = url_for( - "template_category.delete_template_category", template_category_id=str(uuid.uuid4()), cascade="not_a_boolean" - ) - - response = client.delete( - endpoint, - headers=[("Content-Type", "application/json"), auth_header], - ) - - assert response.status_code == 400 - assert response.json["message"] == "Invalid query parameter 'cascade', must be a boolean." - - @pytest.mark.parametrize( "cascade, expected_status_code, expected_msg", [ From 8e0afa2d360d33a7ec4fe8fbe528bfa8ef5c338e Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 25 Jun 2024 10:06:31 -0400 Subject: [PATCH 22/33] Rename migration - Remove NOT NULL constraint on templates.process_type --- ...3_add_template_category.py => 0454_add_template_category.py} | 2 ++ 1 file changed, 2 insertions(+) rename migrations/versions/{0453_add_template_category.py => 0454_add_template_category.py} (96%) diff --git a/migrations/versions/0453_add_template_category.py b/migrations/versions/0454_add_template_category.py similarity index 96% rename from migrations/versions/0453_add_template_category.py rename to migrations/versions/0454_add_template_category.py index 5170aa81c1..04518bffc7 100644 --- a/migrations/versions/0453_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -68,6 +68,7 @@ def upgrade(): ["name_fr"], unique=False, ) + op.alter_column("templates", "process_type", nullable=True) op.create_foreign_key("fk_template_template_categories", "templates", "template_categories", ["template_category_id"], ["id"]) @@ -76,6 +77,7 @@ def downgrade(): op.drop_index(op.f("ix_template_category_id"), table_name="templates") op.drop_index(op.f("ix_template_categories_name_en"), table_name="template_categories") op.drop_index(op.f("ix_template_categories_name_fr"), table_name="template_categories") + op.alter_column("templates", "process_type", nullable=False) op.drop_column("templates", "template_category_id") op.drop_column("templates_history", "template_category_id") op.drop_table("template_categories") From f927c991b610693d87dc4fc3b5a052e072060529 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 25 Jun 2024 13:21:35 -0400 Subject: [PATCH 23/33] Add tests for models - Simplify the dao delete logic --- app/dao/template_categories_dao.py | 31 +++++++++++--------------- tests/app/db.py | 2 ++ tests/app/test_model.py | 35 +++++++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 818840ff04..5c67d5c292 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -47,8 +47,7 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): Deletes a `TemplateCategory`. By default, if the `TemplateCategory` is associated with any `Template`, it will not be deleted. If the `cascade` option is specified then the category will be forcible removed: 1. The `Category` will be dissociated from templates that use it - 2. Dissociated templates will be assigned a default category based on the sms/email process type of the category it was associated with - previously + 2. The template is assigned a category matching the priority of the previous category 3. Finally, the `Category` will be deleted Args: @@ -61,23 +60,19 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): template_category = dao_get_template_category_by_id(template_category_id) templates = Template.query.filter_by(template_category_id=template_category_id).all() - if templates: + if not templates or cascade: + # When there are templates and we are cascading, we set the category to a default + # that matches the template's previous category's priority if cascade: - try: - for template in templates: - process_type = ( - template_category.sms_process_type - if template.template_type == "sms" - else template_category.email_process_type - ) - template.category = dao_get_template_category_by_id(_get_default_category_id(process_type)) - db.session.add(template) - - db.session.delete(template_category) - except Exception as e: - db.session.rollback() - raise e - else: + for template in templates: + # Get the a default category that matches the previous priority of the template, based on template type + default_category_id = _get_default_category_id( + template_category.sms_process_type if template.template_type == "sms" + else template_category.email_process_type + ) + template.category = dao_get_template_category_by_id(default_category_id) + db.session.add(template) + db.session.delete(template_category) db.session.commit() diff --git a/tests/app/db.py b/tests/app/db.py index c9ff33427c..30d265d1a1 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -188,6 +188,7 @@ def create_template( hidden=False, archived=False, folder=None, + category=None, postage=None, process_type="normal", ): @@ -200,6 +201,7 @@ def create_template( "reply_to": reply_to, "hidden": hidden, "folder": folder, + "category": category, "process_type": process_type, } if template_type == LETTER_TYPE: diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 1df0f58f2f..782bea0c68 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -4,8 +4,10 @@ from app import signer_personalisation from app.models import ( + BULK, EMAIL_TYPE, MOBILE_TYPE, + NORMAL, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, NOTIFICATION_FAILED, @@ -16,6 +18,7 @@ NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_TECHNICAL_FAILURE, PRECOMPILED_TEMPLATE_NAME, + PRIORITY, SMS_TYPE, Notification, ServiceSafelist, @@ -31,6 +34,10 @@ save_notification, ) +from tests.app.conftest import ( + create_template_category, +) + @pytest.mark.parametrize("mobile_number", ["650 253 2222", "+1 650 253 2222"]) def test_should_build_service_safelist_from_mobile_number(mobile_number): @@ -354,6 +361,32 @@ def test_template_folder_is_parent(sample_service): assert not folders[1].is_parent_of(folders[0]) +@pytest.mark.parametrize( + "template_type, process_type, sms_process_type, email_process_type, expected_template_process_type", + [ + (SMS_TYPE, None, NORMAL, BULK, NORMAL), + (EMAIL_TYPE, None, BULK, NORMAL, NORMAL), + (SMS_TYPE, BULK, PRIORITY, PRIORITY, BULK), + (EMAIL_TYPE, BULK, PRIORITY, PRIORITY, BULK), + ], +) +def test_template_process_type(notify_db, notify_db_session, template_type, process_type, sms_process_type, email_process_type, expected_template_process_type): + 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 + ) + + assert template.template_process_type == expected_template_process_type + + def test_fido2_key_serialization(sample_fido2_key): json = sample_fido2_key.serialize() assert json["name"] == sample_fido2_key.name @@ -370,4 +403,4 @@ class TestNotificationModel: def test_queue_name_in_notifications(self, sample_service): template = create_template(sample_service, template_type="email") notification = save_notification(create_notification(template, to_field="test@example.com", queue_name="tester")) - assert notification.queue_name == "tester" + assert notification.queue_name == "tester" \ No newline at end of file From 041a64737859a0e17cb6cefc11f5ed54f6c794aa Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 25 Jun 2024 13:53:53 -0400 Subject: [PATCH 24/33] Add tests that were missed for template rest and dao --- tests/app/conftest.py | 7 ++++--- tests/app/dao/test_templates_dao.py | 14 ++++++++++++++ tests/app/template/test_rest.py | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 4404ec0917..d1c2b7e192 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -335,7 +335,7 @@ def create_sample_template( subject_line="Subject", user=None, service=None, - template_category=None, + 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 template_category: - data["category"] = template_category + if category: + data["category"] = category template = Template(**data) dao_create_template(template) @@ -400,6 +400,7 @@ def sample_template( service=None, created_by=None, process_type="normal", + category=None, permissions=[EMAIL_TYPE, SMS_TYPE], ) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index ce6c07bdc7..acac17108d 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -16,6 +16,7 @@ dao_get_template_versions, dao_redact_template, dao_update_template, + dao_update_template_category, dao_update_template_reply_to, ) from app.models import Template, TemplateHistory, TemplateRedacted @@ -490,3 +491,16 @@ def test_template_postage_constraint_on_update(sample_service, sample_user): created.postage = "third" with pytest.raises(expected_exception=SQLAlchemyError): dao_update_template(created) + + +def test_dao_update_template_category(sample_template, sample_template_category): + dao_update_template_category(sample_template.id, sample_template_category.id) + + updated_template = Template.query.get(sample_template.id) + assert updated_template.template_category_id == sample_template_category.id + assert updated_template.updated_at is not None + assert updated_template.version == 2 + + history = TemplateHistory.query.filter_by(id=sample_template.id, version=updated_template.version).one() + assert history.template_category_id == None + assert history.updated_at == updated_template.updated_at \ No newline at end of file diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 5ca28a1177..dda5092b16 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1558,3 +1558,20 @@ def test_should_template_be_redacted(): dao_update_organisation(some_org.id, organisation_type="province_or_territory") assert should_template_be_redacted(some_org) + + +def test_update_templates_category(sample_template, sample_template_category, admin_request): + + assert sample_template.category == None + + response = admin_request.post( + "template.update_templates_category", + service_id=sample_template.service_id, + template_id=sample_template.id, + template_category_id=sample_template_category.id, + _expected_status=200, + ) + + template = dao_get_template_by_id(sample_template.id) + + assert template.category.id == sample_template_category.id \ No newline at end of file From 0be6d19cc0e4d8308f0810a19734e857b030e464 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 25 Jun 2024 14:05:57 -0400 Subject: [PATCH 25/33] Rename /template/category to /template-category - Adjust tests - formatting --- app/dao/template_categories_dao.py | 3 ++- app/template/template_category_rest.py | 2 +- tests/app/dao/test_templates_dao.py | 2 +- tests/app/template/test_rest.py | 3 +-- .../template/test_template_category_rest.py | 17 ++++-------- tests/app/test_model.py | 27 +++++++++---------- 6 files changed, 23 insertions(+), 31 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 5c67d5c292..84ac15aa23 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -67,7 +67,8 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): for template in templates: # Get the a default category that matches the previous priority of the template, based on template type default_category_id = _get_default_category_id( - template_category.sms_process_type if template.template_type == "sms" + template_category.sms_process_type + if template.template_type == "sms" else template_category.email_process_type ) template.category = dao_get_template_category_by_id(default_category_id) diff --git a/app/template/template_category_rest.py b/app/template/template_category_rest.py index 60436322ec..76d9cbc1ba 100644 --- a/app/template/template_category_rest.py +++ b/app/template/template_category_rest.py @@ -15,7 +15,7 @@ template_category_blueprint = Blueprint( "template_category", __name__, - url_prefix="/template/category", + url_prefix="/template-category", ) register_errors(template_category_blueprint) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index acac17108d..47042404e4 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -503,4 +503,4 @@ def test_dao_update_template_category(sample_template, sample_template_category) history = TemplateHistory.query.filter_by(id=sample_template.id, version=updated_template.version).one() assert history.template_category_id == None - assert history.updated_at == updated_template.updated_at \ No newline at end of file + assert history.updated_at == updated_template.updated_at diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index dda5092b16..f9b4f3ce6d 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1561,7 +1561,6 @@ def test_should_template_be_redacted(): def test_update_templates_category(sample_template, sample_template_category, admin_request): - assert sample_template.category == None response = admin_request.post( @@ -1574,4 +1573,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 \ No newline at end of file + assert 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 900df1a7f6..c43f450b16 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -17,11 +17,10 @@ def test_should_create_new_template_category(client, notify_db, notify_db_sessio "email_process_type": "bulk", "hidden": True, } - auth_header = create_authorization_header() response = client.post( - "/template/category", + url_for("template_category.create_template_category"), headers=[("Content-Type", "application/json"), auth_header], json=data, ) @@ -39,7 +38,7 @@ def test_should_create_new_template_category(client, notify_db, notify_db_sessio def test_get_template_category_by_id(client, sample_template_category): auth_header = create_authorization_header() response = client.get( - f"/template/category/{sample_template_category.id}", + url_for("template_category.get_template_category", template_category_id=sample_template_category.id), headers=[("Content-Type", "application/json"), auth_header], ) @@ -55,7 +54,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, template_category=category) + template = create_sample_template(notify_db, notify_db_session, category=category) auth_header = create_authorization_header() endpoint = url_for("template_category.get_template_category_by_template_id", template_id=template.id) @@ -104,18 +103,12 @@ def test_get_template_categories( ): auth_header = create_authorization_header() - query_params = {} - if template_type: - query_params["template_type"] = template_type - if hidden: - query_params["hidden"] = hidden - - query_string = f"?{urlencode(query_params)}" if len(query_params) > 0 else "" + endpoint = url_for("template_category.get_template_categories", template_type=template_type, hidden=hidden) mocker.patch("app.dao.template_categories_dao.dao_get_all_template_categories", return_value=[sample_template_category]) response = client.get( - f"/template/category{query_string}", + endpoint, headers=[("Content-Type", "application/json"), auth_header], ) diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 782bea0c68..52e4abcbf6 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -23,6 +23,7 @@ Notification, ServiceSafelist, ) +from tests.app.conftest import create_template_category from tests.app.db import ( create_inbound_number, create_letter_contact, @@ -34,10 +35,6 @@ save_notification, ) -from tests.app.conftest import ( - create_template_category, -) - @pytest.mark.parametrize("mobile_number", ["650 253 2222", "+1 650 253 2222"]) def test_should_build_service_safelist_from_mobile_number(mobile_number): @@ -370,18 +367,20 @@ def test_template_folder_is_parent(sample_service): (EMAIL_TYPE, BULK, PRIORITY, PRIORITY, BULK), ], ) -def test_template_process_type(notify_db, notify_db_session, template_type, process_type, sms_process_type, email_process_type, expected_template_process_type): +def test_template_process_type( + notify_db, + notify_db_session, + template_type, + process_type, + sms_process_type, + email_process_type, + expected_template_process_type, +): category = create_template_category( - notify_db, - notify_db_session, - sms_process_type = sms_process_type, - email_process_type = email_process_type + 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, category=category ) assert template.template_process_type == expected_template_process_type @@ -403,4 +402,4 @@ class TestNotificationModel: def test_queue_name_in_notifications(self, sample_service): template = create_template(sample_service, template_type="email") notification = save_notification(create_notification(template, to_field="test@example.com", queue_name="tester")) - assert notification.queue_name == "tester" \ No newline at end of file + assert notification.queue_name == "tester" From 44ad1f16274655eca469946926d94501b89a4328 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 25 Jun 2024 14:14:59 -0400 Subject: [PATCH 26/33] various fixes --- tests/app/conftest.py | 4 ++-- tests/app/dao/test_template_categories_dao.py | 8 ++++---- tests/app/dao/test_templates_dao.py | 2 +- tests/app/template/test_rest.py | 5 ++--- tests/app/template/test_template_category_rest.py | 2 -- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d1c2b7e192..576eae315a 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, template_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, category=sample_template_category) + create_sample_template(notify_db, notify_db_session, category=sample_template_category) return sample_template_category diff --git a/tests/app/dao/test_template_categories_dao.py b/tests/app/dao/test_template_categories_dao.py index b2874a366f..73832a2a43 100644 --- a/tests/app/dao/test_template_categories_dao.py +++ b/tests/app/dao/test_template_categories_dao.py @@ -331,8 +331,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", template_category=template_category) - create_sample_template(notify_db, notify_db_session, template_type="sms", 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) retrieved_categories = dao_get_all_template_categories(template_type=template_type, hidden=hidden) @@ -350,7 +350,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, template_category=sample_template_category) + create_sample_template(notify_db, notify_db_session, category=sample_template_category) dao_delete_template_category_by_id(sample_template_category.id) @@ -360,7 +360,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, template_category=sample_template_category) + template = create_sample_template(notify_db, notify_db_session, 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/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 47042404e4..516ceb6220 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -502,5 +502,5 @@ def test_dao_update_template_category(sample_template, sample_template_category) assert updated_template.version == 2 history = TemplateHistory.query.filter_by(id=sample_template.id, version=updated_template.version).one() - assert history.template_category_id == None + assert not history.template_category_id assert history.updated_at == updated_template.updated_at diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index f9b4f3ce6d..42ed94362d 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1561,9 +1561,8 @@ def test_should_template_be_redacted(): def test_update_templates_category(sample_template, sample_template_category, admin_request): - assert sample_template.category == None - - response = admin_request.post( + assert not sample_template.category + admin_request.post( "template.update_templates_category", service_id=sample_template.service_id, template_id=sample_template.id, diff --git a/tests/app/template/test_template_category_rest.py b/tests/app/template/test_template_category_rest.py index c43f450b16..b20528bada 100644 --- a/tests/app/template/test_template_category_rest.py +++ b/tests/app/template/test_template_category_rest.py @@ -1,5 +1,3 @@ -from urllib.parse import urlencode - import pytest from flask import url_for From 1c84439ffea57fe072d90850dcdfd170d9fbf9cb Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Tue, 2 Jul 2024 12:33:28 -0300 Subject: [PATCH 27/33] Re-word dao_delete_template_category_by_id Clarify that new category assignments aren't done at random but are chosen from one of the default categories. --- app/dao/template_categories_dao.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 84ac15aa23..cf6879535d 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -47,15 +47,12 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): Deletes a `TemplateCategory`. By default, if the `TemplateCategory` is associated with any `Template`, it will not be deleted. If the `cascade` option is specified then the category will be forcible removed: 1. The `Category` will be dissociated from templates that use it - 2. The template is assigned a category matching the priority of the previous category - 3. Finally, the `Category` will be deleted + 2. The `Template` is assigned to one of the default categories that matches the priority of the deleted category + 3. Finally the `Category` will be deleted Args: template_category_id (str): The id of the template_category to delete cascade (bool, optional): Specify whether to dissociate the category from templates that use it to force removal. Defaults to False. - - Raises: - e: _description_ """ template_category = dao_get_template_category_by_id(template_category_id) templates = Template.query.filter_by(template_category_id=template_category_id).all() From 3c37d1dc0a285488c8cca5dcabdf6f6163f815f4 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 13:25:16 -0400 Subject: [PATCH 28/33] Add created_at and updated_at fields - Set a default category id if a category is not passed to create_sample_template --- app/dao/template_categories_dao.py | 2 ++ app/models.py | 4 ++++ migrations/versions/0454_add_template_category.py | 10 ++++++---- tests/app/conftest.py | 2 ++ 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index cf6879535d..8d7d480bec 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -1,4 +1,5 @@ import uuid +from datetime import datetime from flask import current_app @@ -69,6 +70,7 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): else template_category.email_process_type ) template.category = dao_get_template_category_by_id(default_category_id) + template.updated_at = datetime.utcnow db.session.add(template) db.session.delete(template_category) diff --git a/app/models.py b/app/models.py index c8e637f0e0..2d0e665c6a 100644 --- a/app/models.py +++ b/app/models.py @@ -1044,6 +1044,8 @@ class TemplateCategory(BaseModel): sms_process_type = db.Column(db.String(200), nullable=False) email_process_type = db.Column(db.String(200), nullable=False) hidden = db.Column(db.Boolean, nullable=False, default=False) + created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) + updated_at = db.Column(db.DateTime, onupdate=datetime.datetime.utcnow) def serialize(self): return { @@ -1055,6 +1057,8 @@ def serialize(self): "sms_process_type": self.sms_process_type, "email_process_type": self.email_process_type, "hidden": self.hidden, + "created_at": self.created_at, + "updated_at": self.updated_at, } @classmethod diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index 04518bffc7..407a8febc3 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -27,23 +27,25 @@ def upgrade(): sa.Column("sms_process_type", sa.String(length=255), nullable=False), sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), + sa.Column("created_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=True), sa.UniqueConstraint("name_en"), sa.UniqueConstraint("name_fr"), ) # Insert the generic low, medium, and high categories op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true)".format( - current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"] + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden, created_at) VALUES ('{}', 'Low Category (Bulk)', 'Catégorie Basse (En Vrac)', 'low', 'low', true, now())".format( + current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"], ) ) op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true)".format( + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden, created_at) VALUES ('{}', 'Medium Category (Normal)', 'Catégorie Moyenne (Normale)', 'low', 'low', true, now())".format( current_app.config["DEFAULT_TEMPLATE_CATEGORY_MEDIUM"] ) ) op.execute( - "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden) VALUES ('{}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true)".format( + "INSERT INTO template_categories (id, name_en, name_fr, sms_process_type, email_process_type, hidden, created_at) VALUES ('{}', 'High Category (Priority)', 'Catégorie Haute (Priorité)', 'low', 'low', true, now())".format( current_app.config["DEFAULT_TEMPLATE_CATEGORY_HIGH"] ) ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 576eae315a..158796f4d0 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -365,6 +365,8 @@ def create_sample_template( data["postage"] = "second" if category: data["category"] = category + else: + data.update({"template_category_id": current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]}) template = Template(**data) dao_create_template(template) From 38bc4e6c412cae255a8c84e517ab73a90e990882 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 15:10:52 -0400 Subject: [PATCH 29/33] Add default value for created at, fix tests --- migrations/versions/0454_add_template_category.py | 2 +- tests/app/conftest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index 407a8febc3..1df278bcd4 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -27,7 +27,7 @@ def upgrade(): sa.Column("sms_process_type", sa.String(length=255), nullable=False), sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), - sa.Column("created_at", sa.DateTime(), nullable=False), + sa.Column("created_at", sa.DateTime(), default=datetime.now(), nullable=False), sa.Column("updated_at", sa.DateTime(), nullable=True), sa.UniqueConstraint("name_en"), sa.UniqueConstraint("name_fr"), diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 158796f4d0..e0281cf9f2 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -366,7 +366,7 @@ def create_sample_template( if category: data["category"] = category else: - data.update({"template_category_id": current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"]}) + data.update({"template_category_id": uuid.UUID(current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"])}) template = Template(**data) dao_create_template(template) From 5e1a947b7b7aa0d665d957453d26a29ac7cf057e Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 15:12:16 -0400 Subject: [PATCH 30/33] Quick fix --- migrations/versions/0454_add_template_category.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index 1df278bcd4..e8c7a61ab9 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -28,7 +28,7 @@ def upgrade(): sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), sa.Column("created_at", sa.DateTime(), default=datetime.now(), nullable=False), - sa.Column("updated_at", sa.DateTime(), nullable=True), + sa.Column("updated_at", sa.DateTime(), default=datetime.now(), nullable=True), sa.UniqueConstraint("name_en"), sa.UniqueConstraint("name_fr"), ) From ce80c8222fd319de842d8c12ec758d1458b45137 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 15:50:11 -0400 Subject: [PATCH 31/33] Fix column defaults - Create a template category when creating a sample template and do not pass in a category as we cannot rely on the defaults existing in the test db --- migrations/versions/0454_add_template_category.py | 4 ++-- tests/app/conftest.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/migrations/versions/0454_add_template_category.py b/migrations/versions/0454_add_template_category.py index e8c7a61ab9..1ba66c6c8d 100644 --- a/migrations/versions/0454_add_template_category.py +++ b/migrations/versions/0454_add_template_category.py @@ -27,8 +27,8 @@ def upgrade(): sa.Column("sms_process_type", sa.String(length=255), nullable=False), sa.Column("email_process_type", sa.String(length=255), nullable=False), sa.Column("hidden", sa.Boolean(), nullable=False), - sa.Column("created_at", sa.DateTime(), default=datetime.now(), nullable=False), - sa.Column("updated_at", sa.DateTime(), default=datetime.now(), nullable=True), + sa.Column("created_at", sa.DateTime(), server_default=sa.func.now(), nullable=False), + sa.Column("updated_at", sa.DateTime(), server_default=sa.func.now(), nullable=True), sa.UniqueConstraint("name_en"), sa.UniqueConstraint("name_fr"), ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e0281cf9f2..3c21efc8ab 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -366,7 +366,8 @@ def create_sample_template( if category: data["category"] = category else: - data.update({"template_category_id": uuid.UUID(current_app.config["DEFAULT_TEMPLATE_CATEGORY_LOW"])}) + 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) From ac90f0dc7c379a1c111c44b1e0799a57f1ac180e Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 16:04:56 -0400 Subject: [PATCH 32/33] Fix silly typo.. --- app/dao/template_categories_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/template_categories_dao.py b/app/dao/template_categories_dao.py index 8d7d480bec..f38586e899 100644 --- a/app/dao/template_categories_dao.py +++ b/app/dao/template_categories_dao.py @@ -70,7 +70,7 @@ def dao_delete_template_category_by_id(template_category_id, cascade=False): else template_category.email_process_type ) template.category = dao_get_template_category_by_id(default_category_id) - template.updated_at = datetime.utcnow + template.updated_at = datetime.utcnow() db.session.add(template) db.session.delete(template_category) From d182ffeea8fa66ed6ed2e018bdcf0d0ba25433c8 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 3 Jul 2024 16:20:24 -0400 Subject: [PATCH 33/33] Remove unneeded assert --- tests/app/template/test_rest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 42ed94362d..77c034af2c 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1561,7 +1561,6 @@ def test_should_template_be_redacted(): def test_update_templates_category(sample_template, sample_template_category, admin_request): - assert not sample_template.category admin_request.post( "template.update_templates_category", service_id=sample_template.service_id,