From 7e16eeb386a1e34f20f47ce611cb04995305ebae Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 11 Oct 2024 13:30:59 -0700 Subject: [PATCH 01/17] upgrade org and template dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 12 +++++++----- app/service_invite/rest.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 9e44bcdd5..3a8a5e602 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -1,3 +1,4 @@ +from sqlalchemy import select from sqlalchemy.sql.expression import func from app import db @@ -6,14 +7,15 @@ from app.models import Domain, Organization, Service, User def dao_get_organizations(): - return Organization.query.order_by( + stmt = select(Organization).order_by( Organization.active.desc(), Organization.name.asc() - ).all() + ) + return db.session.execute(stmt).scalars().all() def dao_count_organizations_with_live_services(): - return ( - db.session.query(Organization.id) + stmt = ( + select(func.count(Organization.id)) .join(Organization.services) .filter( Service.active.is_(True), @@ -21,8 +23,8 @@ def dao_count_organizations_with_live_services(): Service.count_as_live.is_(True), ) .distinct() - .count() ) + return db.session.execute(stmt).scalar() or 0 def dao_get_organization_services(organization_id): diff --git a/app/service_invite/rest.py b/app/service_invite/rest.py index f6d9627da..5728b3ed5 100644 --- a/app/service_invite/rest.py +++ b/app/service_invite/rest.py @@ -86,7 +86,7 @@ def _create_service_invite(invited_user, invite_link_host): redis_store.set( f"email-personalisation-{saved_notification.id}", json.dumps(personalisation), - ex=2*24*60*60, + ex=2 * 24 * 60 * 60, ) send_notification_to_queue(saved_notification, queue=QueueNames.NOTIFY) From daac3bf0f34aa966ee25de907ada00a8c4a05b67 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 11 Oct 2024 14:18:35 -0700 Subject: [PATCH 02/17] upgrade org and template dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 3a8a5e602..761d1b576 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -22,9 +22,9 @@ def dao_count_organizations_with_live_services(): Service.restricted.is_(False), Service.count_as_live.is_(True), ) - .distinct() + ) - return db.session.execute(stmt).scalar() or 0 + return db.session.execute(stmt).distinct().scalar() or 0 def dao_get_organization_services(organization_id): From 88910f5718349d435b496477578d6e54a86f784c Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 07:34:39 -0700 Subject: [PATCH 03/17] upgrade organization_dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 38 ++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 761d1b576..7fde65a2a 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -1,4 +1,4 @@ -from sqlalchemy import select +from sqlalchemy import delete, select from sqlalchemy.sql.expression import func from app import db @@ -22,41 +22,42 @@ def dao_count_organizations_with_live_services(): Service.restricted.is_(False), Service.count_as_live.is_(True), ) - ) return db.session.execute(stmt).distinct().scalar() or 0 def dao_get_organization_services(organization_id): - return Organization.query.filter_by(id=organization_id).one().services + stmt = select(Organization).filter_by(id=organization_id) + return db.session.execute(stmt).scalars().one().services def dao_get_organization_live_services(organization_id): - return Service.query.filter_by( - organization_id=organization_id, restricted=False - ).all() + stmt = select(Service).filter_by(organization_id=organization_id, restricted=False) + return db.session.execute(stmt).scalars().all() def dao_get_organization_by_id(organization_id): - return Organization.query.filter_by(id=organization_id).one() + stmt = select(Organization).filter_by(id=organization_id) + return db.session.execute(stmt).scalars().one() def dao_get_organization_by_email_address(email_address): email_address = email_address.lower().replace(".gsi.gov.uk", ".gov.uk") - - for domain in Domain.query.order_by(func.char_length(Domain.domain).desc()).all(): + stmt = select(Domain).order_by(func.char_length(Domain.domain).desc()) + domains = db.session.execute(stmt).scalars().all() + for domain in domains: if email_address.endswith( "@{}".format(domain.domain) ) or email_address.endswith(".{}".format(domain.domain)): - return Organization.query.filter_by(id=domain.organization_id).one() + stmt = select(Organization).filter_by(id=domain.organization_id) + return db.session.execute(stmt).scalars().one() return None def dao_get_organization_by_service_id(service_id): - return ( - Organization.query.join(Organization.services).filter_by(id=service_id).first() - ) + stmt = select(Organization).join(Organization.services).filter_by(id=service_id) + return db.session.execute(stmt).scalars().first() @autocommit @@ -70,7 +71,8 @@ def dao_update_organization(organization_id, **kwargs): num_updated = Organization.query.filter_by(id=organization_id).update(kwargs) if isinstance(domains, list): - Domain.query.filter_by(organization_id=organization_id).delete() + stmt = delete(Domain).filter_by(organization_id=organization_id) + db.session.execute(stmt) db.session.bulk_save_objects( [ Domain(domain=domain.lower(), organization_id=organization_id) @@ -78,7 +80,7 @@ def dao_update_organization(organization_id, **kwargs): ] ) - organization = Organization.query.get(organization_id) + organization = db.session.get(Organization, organization_id) if "organization_type" in kwargs: _update_organization_services( organization, "organization_type", only_where_none=False @@ -103,7 +105,8 @@ def _update_organization_services(organization, attribute, only_where_none=True) @autocommit @version_class(Service) def dao_add_service_to_organization(service, organization_id): - organization = Organization.query.filter_by(id=organization_id).one() + stmt = select(Organization).filter_by(id=organization_id) + organization = db.session.execute(stmt).scalars().one() service.organization_id = organization_id service.organization_type = organization.organization_type @@ -124,7 +127,8 @@ def dao_get_users_for_organization(organization_id): @autocommit def dao_add_user_to_organization(organization_id, user_id): organization = dao_get_organization_by_id(organization_id) - user = User.query.filter_by(id=user_id).one() + stmt = select(User).filter_by(id=user_id) + user = db.session.execute(stmt).scalars().one() user.organizations.append(organization) db.session.add(organization) return user From 9174efe53597e6502f9d09b458f90515da541d8b Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 07:43:47 -0700 Subject: [PATCH 04/17] upgrade organization_dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 7fde65a2a..366508cf0 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -23,7 +23,8 @@ def dao_count_organizations_with_live_services(): Service.count_as_live.is_(True), ) ) - return db.session.execute(stmt).distinct().scalar() or 0 + # TODO Need distinct here? + return db.session.execute(stmt).scalar() or 0 def dao_get_organization_services(organization_id): From fe85970a86e7e4f896df14d8db5266fb4dab8315 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 07:55:43 -0700 Subject: [PATCH 05/17] upgrade organization_dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 366508cf0..6f045144d 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -15,7 +15,7 @@ def dao_get_organizations(): def dao_count_organizations_with_live_services(): stmt = ( - select(func.count(Organization.id)) + select(func.count(Organization.id).distinct()).select_from(Organization) .join(Organization.services) .filter( Service.active.is_(True), From ca365858bbefbea96c7670f3142ef61a99b2ec43 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 08:05:23 -0700 Subject: [PATCH 06/17] upgrade organization_dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 6f045144d..c686e35a1 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -15,7 +15,7 @@ def dao_get_organizations(): def dao_count_organizations_with_live_services(): stmt = ( - select(func.count(Organization.id).distinct()).select_from(Organization) + select(func.count().distinct()).select_from(Organization) .join(Organization.services) .filter( Service.active.is_(True), From eac1122dae08ed6b1092bcf8a9d0be1121ae4ef8 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 08:14:12 -0700 Subject: [PATCH 07/17] upgrade organization_dao to sqlalchemy 2.0 --- app/dao/organization_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index c686e35a1..548327631 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -15,7 +15,7 @@ def dao_get_organizations(): def dao_count_organizations_with_live_services(): stmt = ( - select(func.count().distinct()).select_from(Organization) + select(func.count(func.distinct(Organization.id))) .join(Organization.services) .filter( Service.active.is_(True), From 5a48c359c6ba603cfdd08b84472967932f91e657 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 08:34:29 -0700 Subject: [PATCH 08/17] fix template folder dao --- app/dao/organization_dao.py | 5 +++-- app/dao/template_folder_dao.py | 10 ++++++--- app/dao/templates_dao.py | 40 +++++++++++++++++++--------------- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 548327631..34afd98e0 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -1,4 +1,4 @@ -from sqlalchemy import delete, select +from sqlalchemy import delete, select, update from sqlalchemy.sql.expression import func from app import db @@ -69,7 +69,8 @@ def dao_create_organization(organization): @autocommit def dao_update_organization(organization_id, **kwargs): domains = kwargs.pop("domains", None) - num_updated = Organization.query.filter_by(id=organization_id).update(kwargs) + stmt = update(Organization).where(id=organization_id).values(**kwargs) + num_updated = db.session.execute(stmt).rowcount if isinstance(domains, list): stmt = delete(Domain).filter_by(organization_id=organization_id) diff --git a/app/dao/template_folder_dao.py b/app/dao/template_folder_dao.py index ae1224179..269f407e0 100644 --- a/app/dao/template_folder_dao.py +++ b/app/dao/template_folder_dao.py @@ -1,16 +1,20 @@ +from sqlalchemy import select + from app import db from app.dao.dao_utils import autocommit from app.models import TemplateFolder def dao_get_template_folder_by_id_and_service_id(template_folder_id, service_id): - return TemplateFolder.query.filter( + stmt = select(TemplateFolder).filter( TemplateFolder.id == template_folder_id, TemplateFolder.service_id == service_id - ).one() + ) + return db.session.execute(stmt).scalars().one() def dao_get_valid_template_folders_by_id(folder_ids): - return TemplateFolder.query.filter(TemplateFolder.id.in_(folder_ids)).all() + stmt = select(TemplateFolder).filter(TemplateFolder.id.in_(folder_ids)) + return db.session.execute(stmt).scalars().all() @autocommit diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 55d4363d6..7c5d7459e 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -1,6 +1,6 @@ import uuid -from sqlalchemy import asc, desc +from sqlalchemy import asc, desc, select from app import db from app.dao.dao_utils import VersionOptions, autocommit, version_class @@ -46,24 +46,29 @@ def dao_redact_template(template, user_id): def dao_get_template_by_id_and_service_id(template_id, service_id, version=None): if version is not None: - return TemplateHistory.query.filter_by( + stmt = select(TemplateHistory).filter_by( id=template_id, hidden=False, service_id=service_id, version=version - ).one() - return Template.query.filter_by( + ) + return db.session.execute(stmt).scalars().one() + stmt = select(Template).filter_by( id=template_id, hidden=False, service_id=service_id - ).one() + ) + return db.session.execute(stmt).scalars().one() def dao_get_template_by_id(template_id, version=None): if version is not None: - return TemplateHistory.query.filter_by(id=template_id, version=version).one() - return Template.query.filter_by(id=template_id).one() + stmt = select(TemplateHistory).filter_by(id=template_id, version=version) + return db.session.execute(stmt).scalars().one() + stmt = select(Template).filter_by(id=template_id) + return db.session.execute(stmt).scalars().one() def dao_get_all_templates_for_service(service_id, template_type=None): if template_type is not None: - return ( - Template.query.filter_by( + stmt = ( + select(Template) + .filter_by( service_id=service_id, template_type=template_type, hidden=False, @@ -73,26 +78,27 @@ def dao_get_all_templates_for_service(service_id, template_type=None): asc(Template.name), asc(Template.template_type), ) - .all() ) - - return ( - Template.query.filter_by(service_id=service_id, hidden=False, archived=False) + return db.session.execute(stmt).scalars().all() + stmt = ( + select(Template) + .filter_by(service_id=service_id, hidden=False, archived=False) .order_by( asc(Template.name), asc(Template.template_type), ) - .all() ) + return db.session.execute(stmt).scalars().all() def dao_get_template_versions(service_id, template_id): - return ( - TemplateHistory.query.filter_by( + stmt = ( + select(TemplateHistory) + .filter_by( service_id=service_id, id=template_id, hidden=False, ) .order_by(desc(TemplateHistory.version)) - .all() ) + return db.session.execute(stmt).scalars().all() From b4a45da4d413e4ca8c388f9cb3307016712d0ce6 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 08:50:04 -0700 Subject: [PATCH 09/17] fix template folder dao --- app/dao/organization_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 34afd98e0..8bb10f659 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -69,7 +69,7 @@ def dao_create_organization(organization): @autocommit def dao_update_organization(organization_id, **kwargs): domains = kwargs.pop("domains", None) - stmt = update(Organization).where(id=organization_id).values(**kwargs) + stmt = update(Organization).where(Organization.id==organization_id).values(**kwargs) num_updated = db.session.execute(stmt).rowcount if isinstance(domains, list): From c8a758aff63ea998d547c6e7e57c53405e7e77b1 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 08:52:50 -0700 Subject: [PATCH 10/17] fix template folder dao --- app/dao/organization_dao.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index 8bb10f659..f699e33bc 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -69,7 +69,9 @@ def dao_create_organization(organization): @autocommit def dao_update_organization(organization_id, **kwargs): domains = kwargs.pop("domains", None) - stmt = update(Organization).where(Organization.id==organization_id).values(**kwargs) + stmt = ( + update(Organization).where(Organization.id == organization_id).values(**kwargs) + ) num_updated = db.session.execute(stmt).rowcount if isinstance(domains, list): From 68b9d8a484beb43e6486052df4a819dba8b48c08 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 09:21:06 -0700 Subject: [PATCH 11/17] fix template folder dao --- .ds.baseline | 4 +- tests/app/dao/test_organization_dao.py | 29 +++-- tests/app/dao/test_services_dao.py | 174 +++++++++++++++---------- 3 files changed, 126 insertions(+), 81 deletions(-) diff --git a/.ds.baseline b/.ds.baseline index 1c279e018..329763bf5 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -239,7 +239,7 @@ "filename": "tests/app/dao/test_services_dao.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 265, + "line_number": 266, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2024-09-27T16:42:53Z" + "generated_at": "2024-10-14T16:21:01Z" } diff --git a/tests/app/dao/test_organization_dao.py b/tests/app/dao/test_organization_dao.py index edffdd1d4..fb2e01d85 100644 --- a/tests/app/dao/test_organization_dao.py +++ b/tests/app/dao/test_organization_dao.py @@ -1,6 +1,7 @@ import uuid import pytest +from sqlalchemy import select from sqlalchemy.exc import IntegrityError, SQLAlchemyError from app import db @@ -57,7 +58,8 @@ def test_get_organization_by_id_gets_correct_organization(notify_db_session): def test_update_organization(notify_db_session): create_organization() - organization = Organization.query.one() + stmt = select(Organization) + organization = db.session.execute(stmt).scalars().one() user = create_user() email_branding = create_email_branding() @@ -78,7 +80,8 @@ def test_update_organization(notify_db_session): dao_update_organization(organization.id, **data) - organization = Organization.query.one() + stmt = select(Organization) + organization = db.session.execute(stmt).scalars().one() for attribute, value in data.items(): assert getattr(organization, attribute) == value @@ -102,7 +105,8 @@ def test_update_organization_domains_lowercases( ): create_organization() - organization = Organization.query.one() + stmt = select(Organization) + organization = db.session.execute(stmt).scalars().one() # Seed some domains dao_update_organization(organization.id, domains=["123", "456"]) @@ -121,7 +125,8 @@ def test_update_organization_domains_lowercases_integrity_error( ): create_organization() - organization = Organization.query.one() + stmt = select(Organization) + organization = db.session.execute(stmt).scalars().one() # Seed some domains dao_update_organization(organization.id, domains=["123", "456"]) @@ -175,11 +180,11 @@ def test_update_organization_updates_the_service_org_type_if_org_type_is_provide assert sample_organization.organization_type == OrganizationType.FEDERAL assert sample_service.organization_type == OrganizationType.FEDERAL + stmt = select(Service.get_history_model()).filter_by( + id=sample_service.id, version=2 + ) assert ( - Service.get_history_model() - .query.filter_by(id=sample_service.id, version=2) - .one() - .organization_type + db.session.execute(stmt).scalars().one().organization_type == OrganizationType.FEDERAL ) @@ -229,11 +234,11 @@ def test_add_service_to_organization(sample_service, sample_organization): assert sample_organization.services[0].id == sample_service.id assert sample_service.organization_type == sample_organization.organization_type + stmt = select(Service.get_history_model()).filter_by( + id=sample_service.id, version=2 + ) assert ( - Service.get_history_model() - .query.filter_by(id=sample_service.id, version=2) - .one() - .organization_type + db.session.execute(stmt).scalars().one().organization_type == sample_organization.organization_type ) assert sample_service.organization_id == sample_organization.id diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index e590eb5b4..487df6a29 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -6,6 +6,7 @@ from unittest.mock import Mock import pytest import sqlalchemy from freezegun import freeze_time +from sqlalchemy import func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound @@ -91,7 +92,7 @@ from tests.app.db import ( def test_create_service(notify_db_session): user = create_user() - assert Service.query.count() == 0 + assert service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -101,7 +102,7 @@ def test_create_service(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 + assert service_query_count() == 1 service_db = Service.query.one() assert service_db.name == "service_name" assert service_db.id == service.id @@ -120,7 +121,7 @@ def test_create_service_with_organization(notify_db_session): organization_type=OrganizationType.STATE, domains=["local-authority.gov.uk"], ) - assert Service.query.count() == 0 + assert service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -130,9 +131,9 @@ def test_create_service_with_organization(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 + assert service_query_count() == 1 service_db = Service.query.one() - organization = Organization.query.get(organization.id) + organization = db.session.get(Organization, organization.id) assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -151,7 +152,7 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): organization_type=OrganizationType.STATE, domains=["local-authority.gov.uk"], ) - assert Service.query.count() == 0 + assert service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -161,9 +162,9 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 + assert service_query_count() == 1 service_db = Service.query.one() - organization = Organization.query.get(organization.id) + organization = db.session.get(Organization, organization.id) assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -183,7 +184,7 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): def test_cannot_create_two_services_with_same_name(notify_db_session): user = create_user() - assert Service.query.count() == 0 + assert service_query_count() == 0 service1 = Service( name="service_name", email_from="email_from1", @@ -209,7 +210,7 @@ def test_cannot_create_two_services_with_same_name(notify_db_session): def test_cannot_create_two_services_with_same_email_from(notify_db_session): user = create_user() - assert Service.query.count() == 0 + assert service_query_count() == 0 service1 = Service( name="service_name1", email_from="email_from", @@ -235,7 +236,7 @@ def test_cannot_create_two_services_with_same_email_from(notify_db_session): def test_cannot_create_service_with_no_user(notify_db_session): user = create_user() - assert Service.query.count() == 0 + assert service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -258,7 +259,7 @@ def test_should_add_user_to_service(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert user in Service.query.first().users + assert user in service_query_first().users new_user = User( name="Test User", email_address="new_user@digital.fake.gov", @@ -267,7 +268,7 @@ def test_should_add_user_to_service(notify_db_session): ) save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) - assert new_user in Service.query.first().users + assert new_user in service_query_first().users def test_dao_add_user_to_service_sets_folder_permissions(sample_user, sample_service): @@ -314,7 +315,8 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a other_service_folder = create_template_folder(other_service) folder_permissions = [str(other_service_folder.id)] - assert ServiceUser.query.count() == 2 + stmt = select(ServiceUser) + assert db.session.execute(stmt).scalar() == 2 with pytest.raises(IntegrityError) as e: dao_add_user_to_service( @@ -326,7 +328,8 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value) ) - assert ServiceUser.query.count() == 2 + stmt = select(ServiceUser) + assert db.session.execute(stmt).scalar() == 2 def test_should_remove_user_from_service(notify_db_session): @@ -347,9 +350,9 @@ def test_should_remove_user_from_service(notify_db_session): ) save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) - assert new_user in Service.query.first().users + assert new_user in service_query_first().users dao_remove_user_from_service(service, new_user) - assert new_user not in Service.query.first().users + assert new_user not in service_query_first().users def test_should_remove_user_from_service_exception(notify_db_session): @@ -668,8 +671,8 @@ def test_removing_all_permission_returns_service_with_no_permissions(notify_db_s def test_create_service_creates_a_history_record_with_current_data(notify_db_session): user = create_user() - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert service_query_count() == 0 + assert service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -678,10 +681,10 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 - assert Service.get_history_model().query.count() == 1 + assert service_query_count() == 1 + assert service_history_query_count() == 1 - service_from_db = Service.query.first() + service_from_db = service_query_first() service_history = Service.get_history_model().query.first() assert service_from_db.id == service_history.id @@ -692,10 +695,25 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses assert service_from_db.created_by.id == service_history.created_by_id +def service_query_count(): + stmt = select(func.count()).select_from(Service) + return db.session.execute(stmt).scalar() or 0 + + +def service_query_first(): + stmt = select(Service) + return db.session.execute(stmt).scalars().first() + + +def service_history_query_count(): + stmt = select(func.count()).select_from(Service.get_history_model()) + return db.session.execute(stmt).scalar() or 0 + + def test_update_service_creates_a_history_record_with_current_data(notify_db_session): user = create_user() - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert service_query_count() == 0 + assert service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -705,17 +723,17 @@ def test_update_service_creates_a_history_record_with_current_data(notify_db_ses ) dao_create_service(service, user) - assert Service.query.count() == 1 - assert Service.query.first().version == 1 - assert Service.get_history_model().query.count() == 1 + assert service_query_count() == 1 + assert service_query_first().version == 1 + assert service_history_query_count() == 1 service.name = "updated_service_name" dao_update_service(service) - assert Service.query.count() == 1 - assert Service.get_history_model().query.count() == 2 + assert service_query_count() == 1 + assert service_history_query_count() == 2 - service_from_db = Service.query.first() + service_from_db = service_query_first() assert service_from_db.version == 2 @@ -736,8 +754,8 @@ def test_update_service_permission_creates_a_history_record_with_current_data( notify_db_session, ): user = create_user() - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert service_query_count() == 0 + assert service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -755,17 +773,17 @@ def test_update_service_permission_creates_a_history_record_with_current_data( ], ) - assert Service.query.count() == 1 + assert service_query_count() == 1 service.permissions.append( ServicePermission(service_id=service.id, permission=ServicePermissionType.EMAIL) ) dao_update_service(service) - assert Service.query.count() == 1 - assert Service.get_history_model().query.count() == 2 + assert service_query_count() == 1 + assert service_history_query_count() == 2 - service_from_db = Service.query.first() + service_from_db = service_query_first() assert service_from_db.version == 2 @@ -784,10 +802,10 @@ def test_update_service_permission_creates_a_history_record_with_current_data( service.permissions.remove(permission) dao_update_service(service) - assert Service.query.count() == 1 - assert Service.get_history_model().query.count() == 3 + assert service_query_count() == 1 + assert service_history_query_count() == 3 - service_from_db = Service.query.first() + service_from_db = service_query_first() assert service_from_db.version == 3 _assert_service_permissions( service.permissions, @@ -810,8 +828,8 @@ def test_update_service_permission_creates_a_history_record_with_current_data( def test_create_service_and_history_is_transactional(notify_db_session): user = create_user() - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert service_query_count() == 0 + assert service_history_query_count() == 0 service = Service( name=None, email_from="email_from", @@ -828,8 +846,8 @@ def test_create_service_and_history_is_transactional(notify_db_session): in str(seeei) ) - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert service_query_count() == 0 + assert service_history_query_count() == 0 def test_delete_service_and_associated_objects(notify_db_session): @@ -846,7 +864,8 @@ def test_delete_service_and_associated_objects(notify_db_session): create_invited_user(service=service) user.organizations = [organization] - assert ServicePermission.query.count() == len( + stmt = select(ServicePermission) + assert db.session.execute(stmt).scalar() == len( ( ServicePermissionType.SMS, ServicePermissionType.EMAIL, @@ -855,21 +874,41 @@ def test_delete_service_and_associated_objects(notify_db_session): ) delete_service_and_all_associated_db_objects(service) - assert VerifyCode.query.count() == 0 - assert ApiKey.query.count() == 0 - assert ApiKey.get_history_model().query.count() == 0 - assert Template.query.count() == 0 - assert TemplateHistory.query.count() == 0 - assert Job.query.count() == 0 - assert Notification.query.count() == 0 - assert Permission.query.count() == 0 - assert User.query.count() == 0 - assert InvitedUser.query.count() == 0 - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 - assert ServicePermission.query.count() == 0 + stmt = select(VerifyCode) + assert db.session.execute(stmt).scalar() is None + stmt = select(ApiKey) + assert db.session.execute(stmt).scalar() is None + stmt = select(ApiKey.get_history_model()) + assert db.session.execute(stmt).scalar() is None + + stmt = select(Template) + assert db.session.execute(stmt).scalar() is None + + stmt = select(TemplateHistory) + assert db.session.execute(stmt).scalar() is None + + stmt = select(Job) + assert db.session.execute(stmt).scalar() is None + + stmt = select(Notification) + assert db.session.execute(stmt).scalar() is None + + stmt = select(Permission) + assert db.session.execute(stmt).scalar() is None + + stmt = select(User) + assert db.session.execute(stmt).scalar() is None + + stmt = select(InvitedUser) + assert db.session.execute(stmt).scalar() is None + stmt = select(ServicePermission) + assert db.session.execute(stmt).scalar() is None + + assert service_query_count() == 0 + assert service_history_query_count() == 0 # the organization hasn't been deleted - assert Organization.query.count() == 1 + stmt = select(Organization) + assert db.session.execute(stmt).scalar() == 1 def test_add_existing_user_to_another_service_doesnot_change_old_permissions( @@ -956,9 +995,10 @@ def test_fetch_stats_filters_on_service(notify_db_session): def test_fetch_stats_ignores_historical_notification_data(sample_template): create_notification_history(template=sample_template) - - assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 1 + stmt = select(Notification) + assert db.session.execute(stmt).scalar() is None + stmt = select(NotificationHistory) + assert db.session.execute(stmt).scalar() == 1 stats = dao_fetch_todays_stats_for_service(sample_template.service_id) assert len(stats) == 0 @@ -1316,7 +1356,7 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key( def test_dao_suspend_service_with_no_api_keys(notify_db_session): service = create_service() dao_suspend_service(service.id) - service = Service.query.get(service.id) + service = db.session.get(Service, service.id) assert not service.active assert service.name == service.name assert service.api_keys == [] @@ -1329,11 +1369,11 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys( service = create_service() api_key = create_api_key(service=service) dao_suspend_service(service.id) - service = Service.query.get(service.id) + service = db.session.get(Service, service.id) assert not service.active assert service.name == service.name - api_key = ApiKey.query.get(api_key.id) + api_key = db.session.get(ApiKey, api_key.id) assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) @@ -1344,13 +1384,13 @@ def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revok service = create_service() api_key = create_api_key(service=service) dao_suspend_service(service.id) - service = Service.query.get(service.id) + service = db.session.get(Service, service.id) assert not service.active dao_resume_service(service.id) - assert Service.query.get(service.id).active + assert db.session.get(Service, service.id).active - api_key = ApiKey.query.get(api_key.id) + api_key = db.session.get(ApiKey, api_key.id) assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) From 1eb46b39b1b09f44ea781c68029d6d93bc581a73 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 09:32:01 -0700 Subject: [PATCH 12/17] fix template folder dao --- tests/app/dao/test_services_dao.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 487df6a29..8640df2a5 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -315,7 +315,7 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a other_service_folder = create_template_folder(other_service) folder_permissions = [str(other_service_folder.id)] - stmt = select(ServiceUser) + stmt = select(func.count()).select_from(ServiceUser) assert db.session.execute(stmt).scalar() == 2 with pytest.raises(IntegrityError) as e: @@ -328,7 +328,7 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value) ) - stmt = select(ServiceUser) + stmt = select(func.count()).select_from(ServiceUser) assert db.session.execute(stmt).scalar() == 2 @@ -864,7 +864,7 @@ def test_delete_service_and_associated_objects(notify_db_session): create_invited_user(service=service) user.organizations = [organization] - stmt = select(ServicePermission) + stmt = select(func.count()).select_from(ServicePermission) assert db.session.execute(stmt).scalar() == len( ( ServicePermissionType.SMS, @@ -907,7 +907,7 @@ def test_delete_service_and_associated_objects(notify_db_session): assert service_query_count() == 0 assert service_history_query_count() == 0 # the organization hasn't been deleted - stmt = select(Organization) + stmt = select(func.count()).select_from(Organization) assert db.session.execute(stmt).scalar() == 1 @@ -997,7 +997,7 @@ def test_fetch_stats_ignores_historical_notification_data(sample_template): create_notification_history(template=sample_template) stmt = select(Notification) assert db.session.execute(stmt).scalar() is None - stmt = select(NotificationHistory) + stmt = select(func.count()).select_from(NotificationHistory) assert db.session.execute(stmt).scalar() == 1 stats = dao_fetch_todays_stats_for_service(sample_template.service_id) From ef5a45d6a255eb1b47c718708515deecca302fbb Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 10:28:44 -0700 Subject: [PATCH 13/17] fix template folder dao --- .ds.baseline | 4 +- tests/app/dao/test_services_dao.py | 67 +++++++++++++----------------- 2 files changed, 32 insertions(+), 39 deletions(-) diff --git a/.ds.baseline b/.ds.baseline index 329763bf5..1a5a9727b 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -239,7 +239,7 @@ "filename": "tests/app/dao/test_services_dao.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 266, + "line_number": 269, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2024-10-14T16:21:01Z" + "generated_at": "2024-10-14T17:28:40Z" } diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 8640df2a5..c6d19c322 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -103,7 +103,8 @@ def test_create_service(notify_db_session): ) dao_create_service(service, user) assert service_query_count() == 1 - service_db = Service.query.one() + stmt = select(Service) + service_db = db.session.execute(stmt).scalars().one() assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -132,7 +133,8 @@ def test_create_service_with_organization(notify_db_session): ) dao_create_service(service, user) assert service_query_count() == 1 - service_db = Service.query.one() + stmt = select(Service) + service_db = db.session.execute(stmt).scalars().one() organization = db.session.get(Organization, organization.id) assert service_db.name == "service_name" assert service_db.id == service.id @@ -163,7 +165,8 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): ) dao_create_service(service, user) assert service_query_count() == 1 - service_db = Service.query.one() + stmt = select(Service) + service_db = db.session.execute(stmt).scalars().one() organization = db.session.get(Organization, organization.id) assert service_db.name == "service_name" assert service_db.id == service.id @@ -385,11 +388,12 @@ def test_should_remove_user_from_service_exception(notify_db_session): def test_removing_a_user_from_a_service_deletes_their_permissions( sample_user, sample_service ): - assert len(Permission.query.all()) == 7 + stmt = select(Permission) + assert len(db.session.execute(stmt).scalars().all()) == 7 dao_remove_user_from_service(sample_service, sample_user) - assert Permission.query.all() == [] + assert db.session.execute(stmt).scalars().all() == [] def test_removing_a_user_from_a_service_deletes_their_folder_permissions_for_that_service( @@ -685,7 +689,9 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses assert service_history_query_count() == 1 service_from_db = service_query_first() - service_history = Service.get_history_model().query.first() + stmt = select(Service.get_history_model()) + + service_history = db.session.execute(stmt).scalars().first() assert service_from_db.id == service_history.id assert service_from_db.name == service_history.name @@ -737,17 +743,10 @@ def test_update_service_creates_a_history_record_with_current_data(notify_db_ses assert service_from_db.version == 2 - assert ( - Service.get_history_model().query.filter_by(name="service_name").one().version - == 1 - ) - assert ( - Service.get_history_model() - .query.filter_by(name="updated_service_name") - .one() - .version - == 2 - ) + stmt = select(Service.get_history_model()).filter_by(name="service_name") + assert db.session.execute(stmt).scalars().one().version == 1 + stmt = select(Service.get_history_model()).filter_by(name="updated_service_name") + assert db.session.execute(stmt).scalars().one().version == 2 def test_update_service_permission_creates_a_history_record_with_current_data( @@ -815,12 +814,12 @@ def test_update_service_permission_creates_a_history_record_with_current_data( ), ) - history = ( - Service.get_history_model() - .query.filter_by(name="service_name") + stmt = ( + select(Service.get_history_model()) + .filter_by(name="service_name") .order_by("version") - .all() ) + history = db.session.execute(stmt).scalars().all() assert len(history) == 3 assert history[2].version == 3 @@ -926,9 +925,8 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_create_service(service_one, user) assert user.id == service_one.users[0].id - test_user_permissions = Permission.query.filter_by( - service=service_one, user=user - ).all() + stmt = select(Permission).filter_by(service=service_one, user=user) + test_user_permissions = db.session.execute(stmt).scalars().all() assert len(test_user_permissions) == 7 other_user = User( @@ -948,14 +946,11 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_create_service(service_two, other_user) assert other_user.id == service_two.users[0].id - other_user_permissions = Permission.query.filter_by( - service=service_two, user=other_user - ).all() + stmt = select(Permission).filter_by(service=service_two, user=other_user) + other_user_permissions = db.session.execute(stmt).scalars().all() assert len(other_user_permissions) == 7 - - other_user_service_one_permissions = Permission.query.filter_by( - service=service_one, user=other_user - ).all() + stmt = select(Permission).filter_by(service=service_one, user=other_user) + other_user_service_one_permissions = db.session.execute(stmt).scalars().all() assert len(other_user_service_one_permissions) == 0 # adding the other_user to service_one should leave all other_user permissions on service_two intact @@ -965,14 +960,12 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_add_user_to_service(service_one, other_user, permissions=permissions) - other_user_service_one_permissions = Permission.query.filter_by( - service=service_one, user=other_user - ).all() + stmt = select(Permission).filter_by(service=service_one, user=other_user) + other_user_service_one_permissions = db.session.execute(stmt).scalars().all() assert len(other_user_service_one_permissions) == 2 - other_user_service_two_permissions = Permission.query.filter_by( - service=service_two, user=other_user - ).all() + stmt = select(Permission).filter_by(service=service_two, user=other_user) + other_user_service_two_permissions = db.session.execute(stmt).scalars().all() assert len(other_user_service_two_permissions) == 7 From 1c42508be72cdf0781939dcebb2c0a07b3098679 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 10:46:51 -0700 Subject: [PATCH 14/17] fix template folder dao --- .ds.baseline | 4 +- tests/app/dao/test_services_dao.py | 241 ++++++++++------------ tests/app/dao/test_template_folder_dao.py | 6 +- tests/app/dao/test_templates_dao.py | 67 +++--- 4 files changed, 154 insertions(+), 164 deletions(-) diff --git a/.ds.baseline b/.ds.baseline index 1a5a9727b..d8f938338 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -239,7 +239,7 @@ "filename": "tests/app/dao/test_services_dao.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 269, + "line_number": 265, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2024-10-14T17:28:40Z" + "generated_at": "2024-10-14T17:46:47Z" } diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index c6d19c322..e590eb5b4 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -6,7 +6,6 @@ from unittest.mock import Mock import pytest import sqlalchemy from freezegun import freeze_time -from sqlalchemy import func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound @@ -92,7 +91,7 @@ from tests.app.db import ( def test_create_service(notify_db_session): user = create_user() - assert service_query_count() == 0 + assert Service.query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -102,9 +101,8 @@ def test_create_service(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert service_query_count() == 1 - stmt = select(Service) - service_db = db.session.execute(stmt).scalars().one() + assert Service.query.count() == 1 + service_db = Service.query.one() assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -122,7 +120,7 @@ def test_create_service_with_organization(notify_db_session): organization_type=OrganizationType.STATE, domains=["local-authority.gov.uk"], ) - assert service_query_count() == 0 + assert Service.query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -132,10 +130,9 @@ def test_create_service_with_organization(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert service_query_count() == 1 - stmt = select(Service) - service_db = db.session.execute(stmt).scalars().one() - organization = db.session.get(Organization, organization.id) + assert Service.query.count() == 1 + service_db = Service.query.one() + organization = Organization.query.get(organization.id) assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -154,7 +151,7 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): organization_type=OrganizationType.STATE, domains=["local-authority.gov.uk"], ) - assert service_query_count() == 0 + assert Service.query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -164,10 +161,9 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert service_query_count() == 1 - stmt = select(Service) - service_db = db.session.execute(stmt).scalars().one() - organization = db.session.get(Organization, organization.id) + assert Service.query.count() == 1 + service_db = Service.query.one() + organization = Organization.query.get(organization.id) assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -187,7 +183,7 @@ def test_fetch_service_by_id_with_api_keys(notify_db_session): def test_cannot_create_two_services_with_same_name(notify_db_session): user = create_user() - assert service_query_count() == 0 + assert Service.query.count() == 0 service1 = Service( name="service_name", email_from="email_from1", @@ -213,7 +209,7 @@ def test_cannot_create_two_services_with_same_name(notify_db_session): def test_cannot_create_two_services_with_same_email_from(notify_db_session): user = create_user() - assert service_query_count() == 0 + assert Service.query.count() == 0 service1 = Service( name="service_name1", email_from="email_from", @@ -239,7 +235,7 @@ def test_cannot_create_two_services_with_same_email_from(notify_db_session): def test_cannot_create_service_with_no_user(notify_db_session): user = create_user() - assert service_query_count() == 0 + assert Service.query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -262,7 +258,7 @@ def test_should_add_user_to_service(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert user in service_query_first().users + assert user in Service.query.first().users new_user = User( name="Test User", email_address="new_user@digital.fake.gov", @@ -271,7 +267,7 @@ def test_should_add_user_to_service(notify_db_session): ) save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) - assert new_user in service_query_first().users + assert new_user in Service.query.first().users def test_dao_add_user_to_service_sets_folder_permissions(sample_user, sample_service): @@ -318,8 +314,7 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a other_service_folder = create_template_folder(other_service) folder_permissions = [str(other_service_folder.id)] - stmt = select(func.count()).select_from(ServiceUser) - assert db.session.execute(stmt).scalar() == 2 + assert ServiceUser.query.count() == 2 with pytest.raises(IntegrityError) as e: dao_add_user_to_service( @@ -331,8 +326,7 @@ def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value) ) - stmt = select(func.count()).select_from(ServiceUser) - assert db.session.execute(stmt).scalar() == 2 + assert ServiceUser.query.count() == 2 def test_should_remove_user_from_service(notify_db_session): @@ -353,9 +347,9 @@ def test_should_remove_user_from_service(notify_db_session): ) save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) - assert new_user in service_query_first().users + assert new_user in Service.query.first().users dao_remove_user_from_service(service, new_user) - assert new_user not in service_query_first().users + assert new_user not in Service.query.first().users def test_should_remove_user_from_service_exception(notify_db_session): @@ -388,12 +382,11 @@ def test_should_remove_user_from_service_exception(notify_db_session): def test_removing_a_user_from_a_service_deletes_their_permissions( sample_user, sample_service ): - stmt = select(Permission) - assert len(db.session.execute(stmt).scalars().all()) == 7 + assert len(Permission.query.all()) == 7 dao_remove_user_from_service(sample_service, sample_user) - assert db.session.execute(stmt).scalars().all() == [] + assert Permission.query.all() == [] def test_removing_a_user_from_a_service_deletes_their_folder_permissions_for_that_service( @@ -675,8 +668,8 @@ def test_removing_all_permission_returns_service_with_no_permissions(notify_db_s def test_create_service_creates_a_history_record_with_current_data(notify_db_session): user = create_user() - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -685,13 +678,11 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses created_by=user, ) dao_create_service(service, user) - assert service_query_count() == 1 - assert service_history_query_count() == 1 + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 1 - service_from_db = service_query_first() - stmt = select(Service.get_history_model()) - - service_history = db.session.execute(stmt).scalars().first() + service_from_db = Service.query.first() + service_history = Service.get_history_model().query.first() assert service_from_db.id == service_history.id assert service_from_db.name == service_history.name @@ -701,25 +692,10 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses assert service_from_db.created_by.id == service_history.created_by_id -def service_query_count(): - stmt = select(func.count()).select_from(Service) - return db.session.execute(stmt).scalar() or 0 - - -def service_query_first(): - stmt = select(Service) - return db.session.execute(stmt).scalars().first() - - -def service_history_query_count(): - stmt = select(func.count()).select_from(Service.get_history_model()) - return db.session.execute(stmt).scalar() or 0 - - def test_update_service_creates_a_history_record_with_current_data(notify_db_session): user = create_user() - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -729,32 +705,39 @@ def test_update_service_creates_a_history_record_with_current_data(notify_db_ses ) dao_create_service(service, user) - assert service_query_count() == 1 - assert service_query_first().version == 1 - assert service_history_query_count() == 1 + assert Service.query.count() == 1 + assert Service.query.first().version == 1 + assert Service.get_history_model().query.count() == 1 service.name = "updated_service_name" dao_update_service(service) - assert service_query_count() == 1 - assert service_history_query_count() == 2 + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 2 - service_from_db = service_query_first() + service_from_db = Service.query.first() assert service_from_db.version == 2 - stmt = select(Service.get_history_model()).filter_by(name="service_name") - assert db.session.execute(stmt).scalars().one().version == 1 - stmt = select(Service.get_history_model()).filter_by(name="updated_service_name") - assert db.session.execute(stmt).scalars().one().version == 2 + assert ( + Service.get_history_model().query.filter_by(name="service_name").one().version + == 1 + ) + assert ( + Service.get_history_model() + .query.filter_by(name="updated_service_name") + .one() + .version + == 2 + ) def test_update_service_permission_creates_a_history_record_with_current_data( notify_db_session, ): user = create_user() - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 service = Service( name="service_name", email_from="email_from", @@ -772,17 +755,17 @@ def test_update_service_permission_creates_a_history_record_with_current_data( ], ) - assert service_query_count() == 1 + assert Service.query.count() == 1 service.permissions.append( ServicePermission(service_id=service.id, permission=ServicePermissionType.EMAIL) ) dao_update_service(service) - assert service_query_count() == 1 - assert service_history_query_count() == 2 + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 2 - service_from_db = service_query_first() + service_from_db = Service.query.first() assert service_from_db.version == 2 @@ -801,10 +784,10 @@ def test_update_service_permission_creates_a_history_record_with_current_data( service.permissions.remove(permission) dao_update_service(service) - assert service_query_count() == 1 - assert service_history_query_count() == 3 + assert Service.query.count() == 1 + assert Service.get_history_model().query.count() == 3 - service_from_db = service_query_first() + service_from_db = Service.query.first() assert service_from_db.version == 3 _assert_service_permissions( service.permissions, @@ -814,12 +797,12 @@ def test_update_service_permission_creates_a_history_record_with_current_data( ), ) - stmt = ( - select(Service.get_history_model()) - .filter_by(name="service_name") + history = ( + Service.get_history_model() + .query.filter_by(name="service_name") .order_by("version") + .all() ) - history = db.session.execute(stmt).scalars().all() assert len(history) == 3 assert history[2].version == 3 @@ -827,8 +810,8 @@ def test_update_service_permission_creates_a_history_record_with_current_data( def test_create_service_and_history_is_transactional(notify_db_session): user = create_user() - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 service = Service( name=None, email_from="email_from", @@ -845,8 +828,8 @@ def test_create_service_and_history_is_transactional(notify_db_session): in str(seeei) ) - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 def test_delete_service_and_associated_objects(notify_db_session): @@ -863,8 +846,7 @@ def test_delete_service_and_associated_objects(notify_db_session): create_invited_user(service=service) user.organizations = [organization] - stmt = select(func.count()).select_from(ServicePermission) - assert db.session.execute(stmt).scalar() == len( + assert ServicePermission.query.count() == len( ( ServicePermissionType.SMS, ServicePermissionType.EMAIL, @@ -873,41 +855,21 @@ def test_delete_service_and_associated_objects(notify_db_session): ) delete_service_and_all_associated_db_objects(service) - stmt = select(VerifyCode) - assert db.session.execute(stmt).scalar() is None - stmt = select(ApiKey) - assert db.session.execute(stmt).scalar() is None - stmt = select(ApiKey.get_history_model()) - assert db.session.execute(stmt).scalar() is None - - stmt = select(Template) - assert db.session.execute(stmt).scalar() is None - - stmt = select(TemplateHistory) - assert db.session.execute(stmt).scalar() is None - - stmt = select(Job) - assert db.session.execute(stmt).scalar() is None - - stmt = select(Notification) - assert db.session.execute(stmt).scalar() is None - - stmt = select(Permission) - assert db.session.execute(stmt).scalar() is None - - stmt = select(User) - assert db.session.execute(stmt).scalar() is None - - stmt = select(InvitedUser) - assert db.session.execute(stmt).scalar() is None - stmt = select(ServicePermission) - assert db.session.execute(stmt).scalar() is None - - assert service_query_count() == 0 - assert service_history_query_count() == 0 + assert VerifyCode.query.count() == 0 + assert ApiKey.query.count() == 0 + assert ApiKey.get_history_model().query.count() == 0 + assert Template.query.count() == 0 + assert TemplateHistory.query.count() == 0 + assert Job.query.count() == 0 + assert Notification.query.count() == 0 + assert Permission.query.count() == 0 + assert User.query.count() == 0 + assert InvitedUser.query.count() == 0 + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 + assert ServicePermission.query.count() == 0 # the organization hasn't been deleted - stmt = select(func.count()).select_from(Organization) - assert db.session.execute(stmt).scalar() == 1 + assert Organization.query.count() == 1 def test_add_existing_user_to_another_service_doesnot_change_old_permissions( @@ -925,8 +887,9 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_create_service(service_one, user) assert user.id == service_one.users[0].id - stmt = select(Permission).filter_by(service=service_one, user=user) - test_user_permissions = db.session.execute(stmt).scalars().all() + test_user_permissions = Permission.query.filter_by( + service=service_one, user=user + ).all() assert len(test_user_permissions) == 7 other_user = User( @@ -946,11 +909,14 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_create_service(service_two, other_user) assert other_user.id == service_two.users[0].id - stmt = select(Permission).filter_by(service=service_two, user=other_user) - other_user_permissions = db.session.execute(stmt).scalars().all() + other_user_permissions = Permission.query.filter_by( + service=service_two, user=other_user + ).all() assert len(other_user_permissions) == 7 - stmt = select(Permission).filter_by(service=service_one, user=other_user) - other_user_service_one_permissions = db.session.execute(stmt).scalars().all() + + other_user_service_one_permissions = Permission.query.filter_by( + service=service_one, user=other_user + ).all() assert len(other_user_service_one_permissions) == 0 # adding the other_user to service_one should leave all other_user permissions on service_two intact @@ -960,12 +926,14 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( dao_add_user_to_service(service_one, other_user, permissions=permissions) - stmt = select(Permission).filter_by(service=service_one, user=other_user) - other_user_service_one_permissions = db.session.execute(stmt).scalars().all() + other_user_service_one_permissions = Permission.query.filter_by( + service=service_one, user=other_user + ).all() assert len(other_user_service_one_permissions) == 2 - stmt = select(Permission).filter_by(service=service_two, user=other_user) - other_user_service_two_permissions = db.session.execute(stmt).scalars().all() + other_user_service_two_permissions = Permission.query.filter_by( + service=service_two, user=other_user + ).all() assert len(other_user_service_two_permissions) == 7 @@ -988,10 +956,9 @@ def test_fetch_stats_filters_on_service(notify_db_session): def test_fetch_stats_ignores_historical_notification_data(sample_template): create_notification_history(template=sample_template) - stmt = select(Notification) - assert db.session.execute(stmt).scalar() is None - stmt = select(func.count()).select_from(NotificationHistory) - assert db.session.execute(stmt).scalar() == 1 + + assert Notification.query.count() == 0 + assert NotificationHistory.query.count() == 1 stats = dao_fetch_todays_stats_for_service(sample_template.service_id) assert len(stats) == 0 @@ -1349,7 +1316,7 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key( def test_dao_suspend_service_with_no_api_keys(notify_db_session): service = create_service() dao_suspend_service(service.id) - service = db.session.get(Service, service.id) + service = Service.query.get(service.id) assert not service.active assert service.name == service.name assert service.api_keys == [] @@ -1362,11 +1329,11 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys( service = create_service() api_key = create_api_key(service=service) dao_suspend_service(service.id) - service = db.session.get(Service, service.id) + service = Service.query.get(service.id) assert not service.active assert service.name == service.name - api_key = db.session.get(ApiKey, api_key.id) + api_key = ApiKey.query.get(api_key.id) assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) @@ -1377,13 +1344,13 @@ def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revok service = create_service() api_key = create_api_key(service=service) dao_suspend_service(service.id) - service = db.session.get(Service, service.id) + service = Service.query.get(service.id) assert not service.active dao_resume_service(service.id) - assert db.session.get(Service, service.id).active + assert Service.query.get(service.id).active - api_key = db.session.get(ApiKey, api_key.id) + api_key = ApiKey.query.get(api_key.id) assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) diff --git a/tests/app/dao/test_template_folder_dao.py b/tests/app/dao/test_template_folder_dao.py index 17b03e5df..2a872e775 100644 --- a/tests/app/dao/test_template_folder_dao.py +++ b/tests/app/dao/test_template_folder_dao.py @@ -1,3 +1,5 @@ +from sqlalchemy import select + from app import db from app.dao.service_user_dao import dao_get_service_user from app.dao.template_folder_dao import ( @@ -17,5 +19,5 @@ def test_dao_delete_template_folder_deletes_user_folder_permissions( dao_update_template_folder(folder) dao_delete_template_folder(folder) - - assert db.session.query(user_folder_permissions).all() == [] + stmt = select(user_folder_permissions) + assert db.session.execute(stmt).scalars().all() == [] diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index bfe0e59d1..8da454d30 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -2,8 +2,10 @@ from datetime import datetime import pytest from freezegun import freeze_time +from sqlalchemy import func, select from sqlalchemy.orm.exc import NoResultFound +from app import db from app.dao.templates_dao import ( dao_create_template, dao_get_all_templates_for_service, @@ -17,6 +19,16 @@ from app.models import Template, TemplateHistory, TemplateRedacted from tests.app.db import create_template +def template_query_count(): + stmt = select(func.count()).select_from(Template) + return db.session.execute(stmt).scalar or 0 + + +def template_history_query_count(): + stmt = select(func.count()).select_from(TemplateHistory) + return db.session.execute(stmt).scalar or 0 + + @pytest.mark.parametrize( "template_type, subject", [ @@ -37,7 +49,7 @@ def test_create_template(sample_service, sample_user, template_type, subject): template = Template(**data) dao_create_template(template) - assert Template.query.count() == 1 + assert template_query_count() == 1 assert len(dao_get_all_templates_for_service(sample_service.id)) == 1 assert ( dao_get_all_templates_for_service(sample_service.id)[0].name @@ -50,11 +62,13 @@ def test_create_template(sample_service, sample_user, template_type, subject): def test_create_template_creates_redact_entry(sample_service): - assert TemplateRedacted.query.count() == 0 + stmt = select(func.count()).select_from(TemplateRedacted) + assert db.session.execute(stmt).scalar() is None template = create_template(sample_service) - redacted = TemplateRedacted.query.one() + stmt = select(TemplateRedacted) + redacted = db.session.execute(stmt).scalars().one() assert redacted.template_id == template.id assert redacted.redact_personalisation is False assert redacted.updated_by_id == sample_service.created_by_id @@ -79,7 +93,8 @@ def test_update_template(sample_service, sample_user): def test_redact_template(sample_template): - redacted = TemplateRedacted.query.one() + stmt = select(TemplateRedacted) + redacted = db.session.execute(stmt).scalars().one() assert redacted.template_id == sample_template.id assert redacted.redact_personalisation is False @@ -96,7 +111,7 @@ def test_get_all_templates_for_service(service_factory): service_1 = service_factory.get("service 1", email_from="service.1") service_2 = service_factory.get("service 2", email_from="service.2") - assert Template.query.count() == 2 + assert template_query_count() == 2 assert len(dao_get_all_templates_for_service(service_1.id)) == 1 assert len(dao_get_all_templates_for_service(service_2.id)) == 1 @@ -119,7 +134,7 @@ def test_get_all_templates_for_service(service_factory): content="Template content", ) - assert Template.query.count() == 5 + assert template_query_count() == 5 assert len(dao_get_all_templates_for_service(service_1.id)) == 3 assert len(dao_get_all_templates_for_service(service_2.id)) == 2 @@ -144,7 +159,7 @@ def test_get_all_templates_for_service_is_alphabetised(sample_service): service=sample_service, ) - assert Template.query.count() == 3 + assert template_query_count() == 3 assert ( dao_get_all_templates_for_service(sample_service.id)[0].name == "Sample Template 1" @@ -171,7 +186,7 @@ def test_get_all_templates_for_service_is_alphabetised(sample_service): def test_get_all_returns_empty_list_if_no_templates(sample_service): - assert Template.query.count() == 0 + assert template_query_count() == 0 assert len(dao_get_all_templates_for_service(sample_service.id)) == 0 @@ -257,8 +272,8 @@ def test_get_template_by_id_and_service_returns_none_if_no_template( def test_create_template_creates_a_history_record_with_current_data( sample_service, sample_user ): - assert Template.query.count() == 0 - assert TemplateHistory.query.count() == 0 + assert template_query_count() == 0 + assert template_history_query_count() == 0 data = { "name": "Sample Template", "template_type": TemplateType.EMAIL, @@ -270,10 +285,12 @@ def test_create_template_creates_a_history_record_with_current_data( template = Template(**data) dao_create_template(template) - assert Template.query.count() == 1 + assert template_query_count() == 1 - template_from_db = Template.query.first() - template_history = TemplateHistory.query.first() + stmt = select(Template) + template_from_db = db.session.execute(stmt).scalars().first() + stmt = select(TemplateHistory) + template_history = db.session.execute(stmt).scalars().first() assert template_from_db.id == template_history.id assert template_from_db.name == template_history.name @@ -286,8 +303,8 @@ def test_create_template_creates_a_history_record_with_current_data( def test_update_template_creates_a_history_record_with_current_data( sample_service, sample_user ): - assert Template.query.count() == 0 - assert TemplateHistory.query.count() == 0 + assert template_query_count() == 0 + assert template_history_query_count() == 0 data = { "name": "Sample Template", "template_type": TemplateType.EMAIL, @@ -301,22 +318,26 @@ def test_update_template_creates_a_history_record_with_current_data( created = dao_get_all_templates_for_service(sample_service.id)[0] assert created.name == "Sample Template" - assert Template.query.count() == 1 - assert Template.query.first().version == 1 - assert TemplateHistory.query.count() == 1 + assert template_query_count() == 1 + stmt = select(Template) + assert db.session.execute(stmt).scalars().first().version == 1 + assert template_history_query_count() == 1 created.name = "new name" dao_update_template(created) - assert Template.query.count() == 1 - assert TemplateHistory.query.count() == 2 + assert template_query_count() == 1 + assert template_history_query_count() == 2 - template_from_db = Template.query.first() + stmt = select(Template) + template_from_db = db.session.execute(stmt).scalars().first() assert template_from_db.version == 2 - assert TemplateHistory.query.filter_by(name="Sample Template").one().version == 1 - assert TemplateHistory.query.filter_by(name="new name").one().version == 2 + stmt = select(TemplateHistory).filter_by(name="Sample Template") + assert db.session.execute(stmt).scalars().one().version == 1 + stmt = select(TemplateHistory).filter_by(name="new name") + assert db.session.execute(stmt).scalars().one().version == 2 def test_get_template_history_version(sample_user, sample_service, sample_template): From 758e12c901e77e9f0884824ff031e040b0273969 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 10:55:10 -0700 Subject: [PATCH 15/17] fix template folder dao --- tests/app/dao/test_templates_dao.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 8da454d30..5fe603a64 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -21,12 +21,12 @@ from tests.app.db import create_template def template_query_count(): stmt = select(func.count()).select_from(Template) - return db.session.execute(stmt).scalar or 0 + return db.session.execute(stmt).scalar() or 0 def template_history_query_count(): stmt = select(func.count()).select_from(TemplateHistory) - return db.session.execute(stmt).scalar or 0 + return db.session.execute(stmt).scalar() or 0 @pytest.mark.parametrize( From 4a7e4c79a83d5fdcea353cb5ff782666f43c0c56 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 14 Oct 2024 11:03:17 -0700 Subject: [PATCH 16/17] fix template folder dao --- tests/app/dao/test_templates_dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 5fe603a64..734a29c0a 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -63,7 +63,7 @@ def test_create_template(sample_service, sample_user, template_type, subject): def test_create_template_creates_redact_entry(sample_service): stmt = select(func.count()).select_from(TemplateRedacted) - assert db.session.execute(stmt).scalar() is None + assert db.session.execute(stmt).scalar() == 0 template = create_template(sample_service) From 829d9020d8716e0874068bfe66bb3b42a251efd9 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 28 Oct 2024 13:03:50 -0700 Subject: [PATCH 17/17] code review feedback --- app/dao/organization_dao.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/dao/organization_dao.py b/app/dao/organization_dao.py index f699e33bc..668ac6c25 100644 --- a/app/dao/organization_dao.py +++ b/app/dao/organization_dao.py @@ -23,7 +23,6 @@ def dao_count_organizations_with_live_services(): Service.count_as_live.is_(True), ) ) - # TODO Need distinct here? return db.session.execute(stmt).scalar() or 0