diff --git a/.ds.baseline b/.ds.baseline index c8d59174d..3779d8edb 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": 289, "is_secret": false } ], @@ -257,7 +257,7 @@ "filename": "tests/app/dao/test_users_dao.py", "hashed_secret": "f2c57870308dc87f432e5912d4de6f8e322721ba", "is_verified": false, - "line_number": 194, + "line_number": 199, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2024-10-14T17:46:47Z" + "generated_at": "2024-10-28T20:26:27Z" } diff --git a/app/dao/service_permissions_dao.py b/app/dao/service_permissions_dao.py index e459b6e56..0793b35b6 100644 --- a/app/dao/service_permissions_dao.py +++ b/app/dao/service_permissions_dao.py @@ -1,12 +1,14 @@ +from sqlalchemy import delete, select + from app import db from app.dao.dao_utils import autocommit from app.models import ServicePermission def dao_fetch_service_permissions(service_id): - return ServicePermission.query.filter( - ServicePermission.service_id == service_id - ).all() + + stmt = select(ServicePermission).filter(ServicePermission.service_id == service_id) + return db.session.execute(stmt).scalars().all() @autocommit @@ -16,9 +18,11 @@ def dao_add_service_permission(service_id, permission): def dao_remove_service_permission(service_id, permission): - deleted = ServicePermission.query.filter( + + stmt = delete(ServicePermission).where( ServicePermission.service_id == service_id, ServicePermission.permission == permission, - ).delete() + ) + result = db.session.execute(stmt) db.session.commit() - return deleted + return result.rowcount diff --git a/app/dao/service_sms_sender_dao.py b/app/dao/service_sms_sender_dao.py index 9224cf09d..82796b05f 100644 --- a/app/dao/service_sms_sender_dao.py +++ b/app/dao/service_sms_sender_dao.py @@ -1,4 +1,4 @@ -from sqlalchemy import desc +from sqlalchemy import desc, select from app import db from app.dao.dao_utils import autocommit @@ -17,17 +17,20 @@ def insert_service_sms_sender(service, sms_sender): def dao_get_service_sms_senders_by_id(service_id, service_sms_sender_id): - return ServiceSmsSender.query.filter_by( + stmt = select(ServiceSmsSender).filter_by( id=service_sms_sender_id, service_id=service_id, archived=False - ).one() + ) + return db.session.execute(stmt).scalars().one() def dao_get_sms_senders_by_service_id(service_id): - return ( - ServiceSmsSender.query.filter_by(service_id=service_id, archived=False) + + stmt = ( + select(ServiceSmsSender) + .filter_by(service_id=service_id, archived=False) .order_by(desc(ServiceSmsSender.is_default)) - .all() ) + return db.session.execute(stmt).scalars().all() @autocommit diff --git a/app/dao/service_user_dao.py b/app/dao/service_user_dao.py index 0b991a4fc..d60c92ba6 100644 --- a/app/dao/service_user_dao.py +++ b/app/dao/service_user_dao.py @@ -1,25 +1,23 @@ +from sqlalchemy import select + from app import db from app.dao.dao_utils import autocommit from app.models import ServiceUser, User def dao_get_service_user(user_id, service_id): - # TODO: This has been changed to account for the test case failure - # that used this method but have any service user to return. Somehow, this - # started to throw an error with one() method in sqlalchemy 2.0 unlike 1.4 - return ServiceUser.query.filter_by( - user_id=user_id, service_id=service_id - ).one_or_none() + stmt = select(ServiceUser).filter_by(user_id=user_id, service_id=service_id) + return db.session.execute(stmt).scalars().one_or_none() def dao_get_active_service_users(service_id): - query = ( - db.session.query(ServiceUser) + + stmt = ( + select(ServiceUser) .join(User, User.id == ServiceUser.user_id) .filter(User.state == "active", ServiceUser.service_id == service_id) ) - - return query.all() + return db.session.execute(stmt).scalars().all() def dao_get_service_users_by_user_id(user_id): diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 19755edfe..1f8956865 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -2,7 +2,7 @@ import uuid from datetime import timedelta from flask import current_app -from sqlalchemy import Float, cast, select +from sqlalchemy import Float, cast, delete, select from sqlalchemy.orm import joinedload from sqlalchemy.sql.expression import and_, asc, case, func @@ -51,34 +51,42 @@ from app.utils import ( def dao_fetch_all_services(only_active=False): - query = Service.query.order_by(asc(Service.created_at)).options( - joinedload(Service.users) - ) + + stmt = select(Service) if only_active: - query = query.filter(Service.active) + stmt = stmt.where(Service.active) - return query.all() + stmt = stmt.order_by(asc(Service.created_at)).options(joinedload(Service.users)) + + result = db.session.execute(stmt) + return result.unique().scalars().all() def get_services_by_partial_name(service_name): service_name = escape_special_characters(service_name) - return Service.query.filter(Service.name.ilike("%{}%".format(service_name))).all() + stmt = select(Service).where(Service.name.ilike("%{}%".format(service_name))) + result = db.session.execute(stmt) + return result.scalars().all() def dao_count_live_services(): - return Service.query.filter_by( - active=True, - restricted=False, - count_as_live=True, - ).count() + stmt = ( + select(func.count()) + .select_from(Service) + .where( + Service.active, Service.count_as_live, Service.restricted == False # noqa + ) + ) + result = db.session.execute(stmt) + return result.scalar() # Retrieves the count def dao_fetch_live_services_data(): year_start_date, year_end_date = get_current_calendar_year() most_recent_annual_billing = ( - db.session.query( + select( AnnualBilling.service_id, func.max(AnnualBilling.financial_year_start).label("year"), ) @@ -86,13 +94,17 @@ def dao_fetch_live_services_data(): .subquery() ) - this_year_ft_billing = FactBilling.query.filter( - FactBilling.local_date >= year_start_date, - FactBilling.local_date <= year_end_date, - ).subquery() + this_year_ft_billing = ( + select(FactBilling) + .filter( + FactBilling.local_date >= year_start_date, + FactBilling.local_date <= year_end_date, + ) + .subquery() + ) - data = ( - db.session.query( + stmt = ( + select( Service.id.label("service_id"), Service.name.label("service_name"), Organization.name.label("organization_name"), @@ -156,8 +168,9 @@ def dao_fetch_live_services_data(): AnnualBilling.free_sms_fragment_limit, ) .order_by(asc(Service.go_live_at)) - .all() ) + + data = db.session.execute(stmt).all() results = [] for row in data: existing_service = next( @@ -183,48 +196,55 @@ def dao_fetch_service_by_id(service_id, only_active=False): stmt = stmt.where(Service.active) result = db.session.execute(stmt) - return result.unique().scalars().one() + return result.unique().scalars().unique().one() def dao_fetch_service_by_inbound_number(number): - inbound_number = InboundNumber.query.filter( + stmt = select(InboundNumber).where( InboundNumber.number == number, InboundNumber.active - ).first() + ) + result = db.session.execute(stmt) + inbound_number = result.scalars().first() if not inbound_number: return None - return Service.query.filter(Service.id == inbound_number.service_id).first() + stmt = select(Service).where(Service.id == inbound_number.service_id) + result = db.session.execute(stmt) + return result.scalars().first() def dao_fetch_service_by_id_with_api_keys(service_id, only_active=False): - query = Service.query.filter_by(id=service_id).options(joinedload(Service.api_keys)) - + stmt = ( + select(Service).filter_by(id=service_id).options(joinedload(Service.api_keys)) + ) if only_active: - query = query.filter(Service.active) - - return query.one() + stmt = stmt.filter(Service.active) + return db.session.execute(stmt).scalars().unique().one() def dao_fetch_all_services_by_user(user_id, only_active=False): - query = ( - Service.query.filter(Service.users.any(id=user_id)) + + stmt = ( + select(Service) + .filter(Service.users.any(id=user_id)) .order_by(asc(Service.created_at)) .options(joinedload(Service.users)) ) - if only_active: - query = query.filter(Service.active) - - return query.all() + stmt = stmt.filter(Service.active) + return db.session.execute(stmt).scalars().unique().all() def dao_fetch_all_services_created_by_user(user_id): - query = Service.query.filter_by(created_by_id=user_id).order_by( - asc(Service.created_at) + + stmt = ( + select(Service) + .filter_by(created_by_id=user_id) + .order_by(asc(Service.created_at)) ) - return query.all() + return db.session.execute(stmt).scalars().all() @autocommit @@ -234,16 +254,15 @@ def dao_fetch_all_services_created_by_user(user_id): VersionOptions(Template, history_class=TemplateHistory, must_write_history=False), ) def dao_archive_service(service_id): - # have to eager load templates and api keys so that we don't flush when we loop through them - # to ensure that db.session still contains the models when it comes to creating history objects - service = ( - Service.query.options( + stmt = ( + select(Service) + .options( joinedload(Service.templates).subqueryload(Template.template_redacted), joinedload(Service.api_keys), ) .filter(Service.id == service_id) - .one() ) + service = db.session.execute(stmt).scalars().unique().one() service.active = False service.name = get_archived_db_column_value(service.name) @@ -259,11 +278,14 @@ def dao_archive_service(service_id): def dao_fetch_service_by_id_and_user(service_id, user_id): - return ( - Service.query.filter(Service.users.any(id=user_id), Service.id == service_id) + + stmt = ( + select(Service) + .filter(Service.users.any(id=user_id), Service.id == service_id) .options(joinedload(Service.users)) - .one() ) + result = db.session.execute(stmt).scalar_one() + return result @autocommit @@ -366,39 +388,40 @@ def dao_remove_user_from_service(service, user): def delete_service_and_all_associated_db_objects(service): - def _delete_commit(query): - query.delete(synchronize_session=False) + def _delete_commit(stmt): + db.session.execute(stmt) db.session.commit() - subq = db.session.query(Template.id).filter_by(service=service).subquery() - _delete_commit( - TemplateRedacted.query.filter(TemplateRedacted.template_id.in_(subq)) - ) + subq = select(Template.id).filter_by(service=service).subquery() - _delete_commit(ServiceSmsSender.query.filter_by(service=service)) - _delete_commit(ServiceEmailReplyTo.query.filter_by(service=service)) - _delete_commit(InvitedUser.query.filter_by(service=service)) - _delete_commit(Permission.query.filter_by(service=service)) - _delete_commit(NotificationHistory.query.filter_by(service=service)) - _delete_commit(Notification.query.filter_by(service=service)) - _delete_commit(Job.query.filter_by(service=service)) - _delete_commit(Template.query.filter_by(service=service)) - _delete_commit(TemplateHistory.query.filter_by(service_id=service.id)) - _delete_commit(ServicePermission.query.filter_by(service_id=service.id)) - _delete_commit(ApiKey.query.filter_by(service=service)) - _delete_commit(ApiKey.get_history_model().query.filter_by(service_id=service.id)) - _delete_commit(AnnualBilling.query.filter_by(service_id=service.id)) + stmt = delete(TemplateRedacted).filter(TemplateRedacted.template_id.in_(subq)) + _delete_commit(stmt) - verify_codes = VerifyCode.query.join(User).filter( - User.id.in_([x.id for x in service.users]) + _delete_commit(delete(ServiceSmsSender).filter_by(service=service)) + _delete_commit(delete(ServiceEmailReplyTo).filter_by(service=service)) + _delete_commit(delete(InvitedUser).filter_by(service=service)) + _delete_commit(delete(Permission).filter_by(service=service)) + _delete_commit(delete(NotificationHistory).filter_by(service=service)) + _delete_commit(delete(Notification).filter_by(service=service)) + _delete_commit(delete(Job).filter_by(service=service)) + _delete_commit(delete(Template).filter_by(service=service)) + _delete_commit(delete(TemplateHistory).filter_by(service_id=service.id)) + _delete_commit(delete(ServicePermission).filter_by(service_id=service.id)) + _delete_commit(delete(ApiKey).filter_by(service=service)) + _delete_commit(delete(ApiKey.get_history_model()).filter_by(service_id=service.id)) + _delete_commit(delete(AnnualBilling).filter_by(service_id=service.id)) + + stmt = ( + select(VerifyCode).join(User).filter(User.id.in_([x.id for x in service.users])) ) + verify_codes = db.session.execute(stmt).scalars().all() list(map(db.session.delete, verify_codes)) db.session.commit() users = [x for x in service.users] for user in users: user.organizations = [] service.users.remove(user) - _delete_commit(Service.get_history_model().query.filter_by(id=service.id)) + _delete_commit(delete(Service.get_history_model()).filter_by(id=service.id)) db.session.delete(service) db.session.commit() for user in users: @@ -409,8 +432,8 @@ def delete_service_and_all_associated_db_objects(service): def dao_fetch_todays_stats_for_service(service_id): today = utc_now().date() start_date = get_midnight_in_utc(today) - return ( - db.session.query( + stmt = ( + select( Notification.notification_type, Notification.status, func.count(Notification.id).label("count"), @@ -424,16 +447,16 @@ def dao_fetch_todays_stats_for_service(service_id): Notification.notification_type, Notification.status, ) - .all() ) + return db.session.execute(stmt).all() def dao_fetch_stats_for_service_from_days(service_id, start_date, end_date): start_date = get_midnight_in_utc(start_date) end_date = get_midnight_in_utc(end_date + timedelta(days=1)) - return ( - db.session.query( + stmt = ( + select( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), @@ -450,8 +473,8 @@ def dao_fetch_stats_for_service_from_days(service_id, start_date, end_date): NotificationAllTimeView.status, func.date_trunc("day", NotificationAllTimeView.created_at), ) - .all() ) + return db.session.execute(stmt).scalars().all() def dao_fetch_stats_for_service_from_days_for_user( @@ -460,13 +483,14 @@ def dao_fetch_stats_for_service_from_days_for_user( start_date = get_midnight_in_utc(start_date) end_date = get_midnight_in_utc(end_date + timedelta(days=1)) - return ( - db.session.query( + stmt = ( + select( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, func.date_trunc("day", NotificationAllTimeView.created_at).label("day"), func.count(NotificationAllTimeView.id).label("count"), ) + .select_from(NotificationAllTimeView) .filter( NotificationAllTimeView.service_id == service_id, NotificationAllTimeView.key_type != KeyType.TEST, @@ -479,8 +503,8 @@ def dao_fetch_stats_for_service_from_days_for_user( NotificationAllTimeView.status, func.date_trunc("day", NotificationAllTimeView.created_at), ) - .all() ) + return db.session.execute(stmt).scalars().all() def dao_fetch_todays_stats_for_all_services( @@ -491,7 +515,7 @@ def dao_fetch_todays_stats_for_all_services( end_date = get_midnight_in_utc(today + timedelta(days=1)) subquery = ( - db.session.query( + select( Notification.notification_type, Notification.status, Notification.service_id, @@ -510,8 +534,8 @@ def dao_fetch_todays_stats_for_all_services( subquery = subquery.subquery() - query = ( - db.session.query( + stmt = ( + select( Service.id.label("service_id"), Service.name, Service.restricted, @@ -526,9 +550,9 @@ def dao_fetch_todays_stats_for_all_services( ) if only_active: - query = query.filter(Service.active) + stmt = stmt.filter(Service.active) - return query.all() + return db.session.execute(stmt).all() @autocommit @@ -537,15 +561,13 @@ def dao_fetch_todays_stats_for_all_services( VersionOptions(Service), ) def dao_suspend_service(service_id): - # have to eager load api keys so that we don't flush when we loop through them - # to ensure that db.session still contains the models when it comes to creating history objects - service = ( - Service.query.options( - joinedload(Service.api_keys), - ) + + stmt = ( + select(Service) + .options(joinedload(Service.api_keys)) .filter(Service.id == service_id) - .one() ) + service = db.session.execute(stmt).scalars().unique().one() for api_key in service.api_keys: if not api_key.expiry_date: @@ -557,19 +579,22 @@ def dao_suspend_service(service_id): @autocommit @version_class(Service) def dao_resume_service(service_id): - service = Service.query.get(service_id) + service = db.session.get(Service, service_id) + service.active = True def dao_fetch_active_users_for_service(service_id): - query = User.query.filter(User.services.any(id=service_id), User.state == "active") - return query.all() + stmt = select(User).where(User.services.any(id=service_id), User.state == "active") + result = db.session.execute(stmt) + return result.scalars().all() def dao_find_services_sending_to_tv_numbers(start_date, end_date, threshold=500): - return ( - db.session.query( + + stmt = ( + select( Notification.service_id.label("service_id"), func.count(Notification.id).label("notification_count"), ) @@ -587,13 +612,13 @@ def dao_find_services_sending_to_tv_numbers(start_date, end_date, threshold=500) Notification.service_id, ) .having(func.count(Notification.id) > threshold) - .all() ) + return db.session.execute(stmt).all() def dao_find_services_with_high_failure_rates(start_date, end_date, threshold=10000): subquery = ( - db.session.query( + select( func.count(Notification.id).label("total_count"), Notification.service_id.label("service_id"), ) @@ -614,8 +639,8 @@ def dao_find_services_with_high_failure_rates(start_date, end_date, threshold=10 subquery = subquery.subquery() - query = ( - db.session.query( + stmt = ( + select( Notification.service_id.label("service_id"), func.count(Notification.id).label("permanent_failure_count"), subquery.c.total_count.label("total_count"), @@ -643,17 +668,19 @@ def dao_find_services_with_high_failure_rates(start_date, end_date, threshold=10 ) ) - return query.all() + return db.session.execute(stmt).all() def get_live_services_with_organization(): - query = ( - db.session.query( + + stmt = ( + select( Service.id.label("service_id"), Service.name.label("service_name"), Organization.id.label("organization_id"), Organization.name.label("organization_name"), ) + .select_from(Service) .outerjoin(Service.organization) .filter( Service.count_as_live.is_(True), @@ -663,14 +690,15 @@ def get_live_services_with_organization(): .order_by(Organization.name, Service.name) ) - return query.all() + return db.session.execute(stmt).all() def fetch_notification_stats_for_service_by_month_by_user( start_date, end_date, service_id, user_id ): - return ( - db.session.query( + + stmt = ( + select( func.date_trunc("month", NotificationAllTimeView.created_at).label("month"), NotificationAllTimeView.notification_type, (NotificationAllTimeView.status).label("notification_status"), @@ -688,8 +716,8 @@ def fetch_notification_stats_for_service_by_month_by_user( NotificationAllTimeView.notification_type, NotificationAllTimeView.status, ) - .all() ) + return db.session.execute(stmt).all() def get_specific_days_stats(data, start_date, days=None, end_date=None): diff --git a/tests/app/dao/test_service_sms_sender_dao.py b/tests/app/dao/test_service_sms_sender_dao.py index 9ca05e711..10bfd21f4 100644 --- a/tests/app/dao/test_service_sms_sender_dao.py +++ b/tests/app/dao/test_service_sms_sender_dao.py @@ -1,8 +1,10 @@ import uuid import pytest +from sqlalchemy import select from sqlalchemy.exc import SQLAlchemyError +from app import db from app.dao.service_sms_sender_dao import ( archive_sms_sender, dao_add_sms_sender_for_service, @@ -97,10 +99,8 @@ def test_dao_add_sms_sender_for_service(notify_db_session): is_default=False, inbound_number_id=None, ) - - service_sms_senders = ServiceSmsSender.query.order_by( - ServiceSmsSender.created_at - ).all() + stmt = select(ServiceSmsSender).order_by(ServiceSmsSender.created_at) + service_sms_senders = db.session.execute(stmt).scalars().all() assert len(service_sms_senders) == 2 assert service_sms_senders[0].sms_sender == "testing" assert service_sms_senders[0].is_default @@ -116,10 +116,8 @@ def test_dao_add_sms_sender_for_service_switches_default(notify_db_session): is_default=True, inbound_number_id=None, ) - - service_sms_senders = ServiceSmsSender.query.order_by( - ServiceSmsSender.created_at - ).all() + stmt = select(ServiceSmsSender).order_by(ServiceSmsSender.created_at) + service_sms_senders = db.session.execute(stmt).scalars().all() assert len(service_sms_senders) == 2 assert service_sms_senders[0].sms_sender == "testing" assert not service_sms_senders[0].is_default @@ -128,7 +126,8 @@ def test_dao_add_sms_sender_for_service_switches_default(notify_db_session): def test_dao_update_service_sms_sender(notify_db_session): service = create_service() - service_sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + stmt = select(ServiceSmsSender).filter_by(service_id=service.id) + service_sms_senders = db.session.execute(stmt).scalars().all() assert len(service_sms_senders) == 1 sms_sender_to_update = service_sms_senders[0] @@ -138,7 +137,8 @@ def test_dao_update_service_sms_sender(notify_db_session): is_default=True, sms_sender="updated", ) - sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + stmt = select(ServiceSmsSender).filter_by(service_id=service.id) + sms_senders = db.session.execute(stmt).scalars().all() assert len(sms_senders) == 1 assert sms_senders[0].is_default assert sms_senders[0].sms_sender == "updated" @@ -159,7 +159,8 @@ def test_dao_update_service_sms_sender_switches_default(notify_db_session): is_default=True, sms_sender="updated", ) - sms_senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + stmt = select(ServiceSmsSender).filter_by(service_id=service.id) + sms_senders = db.session.execute(stmt).scalars().all() expected = {("testing", False), ("updated", True)} results = {(sender.sms_sender, sender.is_default) for sender in sms_senders} @@ -190,7 +191,8 @@ def test_update_existing_sms_sender_with_inbound_number(notify_db_session): service = create_service() inbound_number = create_inbound_number(number="12345", service_id=service.id) - existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one() + stmt = select(ServiceSmsSender).filter_by(service_id=service.id) + existing_sms_sender = db.session.execute(stmt).scalars().one() sms_sender = update_existing_sms_sender_with_inbound_number( service_sms_sender=existing_sms_sender, sms_sender=inbound_number.number, @@ -206,7 +208,8 @@ def test_update_existing_sms_sender_with_inbound_number_raises_exception_if_inbo notify_db_session, ): service = create_service() - existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one() + stmt = select(ServiceSmsSender).filter_by(service_id=service.id) + existing_sms_sender = db.session.execute(stmt).scalars().one() with pytest.raises(expected_exception=SQLAlchemyError): update_existing_sms_sender_with_inbound_number( service_sms_sender=existing_sms_sender, diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index e590eb5b4..61fe99419 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 @@ -89,9 +90,32 @@ from tests.app.db import ( ) +def _get_service_query_count(): + stmt = select(func.count(Service.id)) + return db.session.execute(stmt).scalar() or 0 + + +def _get_service_history_query_count(): + stmt = select(func.count(Service.get_history_model().id)) + return db.session.execute(stmt).scalar() or 0 + + +def _get_first_service(): + stmt = select(Service).limit(1) + service = db.session.execute(stmt).scalars().first() + return service + + +def _get_service_by_id(service_id): + stmt = select(Service).filter(Service.id == service_id) + + service = db.session.execute(stmt).scalars().one() + return service + + def test_create_service(notify_db_session): user = create_user() - assert Service.query.count() == 0 + assert _get_service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -101,8 +125,8 @@ def test_create_service(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 - service_db = Service.query.one() + assert _get_service_query_count() == 1 + service_db = _get_first_service() assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == "email_from" @@ -120,7 +144,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 _get_service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -130,9 +154,9 @@ def test_create_service_with_organization(notify_db_session): created_by=user, ) dao_create_service(service, user) - assert Service.query.count() == 1 - service_db = Service.query.one() - organization = Organization.query.get(organization.id) + assert _get_service_query_count() == 1 + service_db = _get_first_service() + 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 +175,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 _get_service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -161,9 +185,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 - service_db = Service.query.one() - organization = Organization.query.get(organization.id) + assert _get_service_query_count() == 1 + service_db = _get_first_service() + 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 +207,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 _get_service_query_count() == 0 service1 = Service( name="service_name", email_from="email_from1", @@ -209,7 +233,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 _get_service_query_count() == 0 service1 = Service( name="service_name1", email_from="email_from", @@ -235,7 +259,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 _get_service_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -258,7 +282,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 _get_first_service().users new_user = User( name="Test User", email_address="new_user@digital.fake.gov", @@ -267,7 +291,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 _get_first_service().users def test_dao_add_user_to_service_sets_folder_permissions(sample_user, sample_service): @@ -314,7 +338,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(func.count(ServiceUser.service_id)) + assert db.session.execute(stmt).scalar() == 2 with pytest.raises(IntegrityError) as e: dao_add_user_to_service( @@ -326,7 +351,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(func.count(ServiceUser.service_id)) + assert db.session.execute(stmt).scalar() == 2 def test_should_remove_user_from_service(notify_db_session): @@ -347,9 +373,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 _get_first_service().users dao_remove_user_from_service(service, new_user) - assert new_user not in Service.query.first().users + assert new_user not in _get_first_service().users def test_should_remove_user_from_service_exception(notify_db_session): @@ -382,11 +408,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).all()) == 7 dao_remove_user_from_service(sample_service, sample_user) - assert Permission.query.all() == [] + assert db.session.execute(stmt).all() == [] def test_removing_a_user_from_a_service_deletes_their_folder_permissions_for_that_service( @@ -668,8 +695,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 _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -678,11 +705,12 @@ 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 _get_service_query_count() == 1 + assert _get_service_history_query_count() == 1 - service_from_db = Service.query.first() - service_history = Service.get_history_model().query.first() + service_from_db = _get_first_service() + 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 @@ -694,8 +722,8 @@ def test_create_service_creates_a_history_record_with_current_data(notify_db_ses 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 _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -705,39 +733,31 @@ 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 _get_service_query_count() == 1 + assert _get_first_service().version == 1 + assert _get_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 _get_service_query_count() == 1 + assert _get_service_history_query_count() == 2 - service_from_db = Service.query.first() + service_from_db = _get_first_service() 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( notify_db_session, ): user = create_user() - assert Service.query.count() == 0 - assert Service.get_history_model().query.count() == 0 + assert _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 service = Service( name="service_name", email_from="email_from", @@ -755,17 +775,17 @@ def test_update_service_permission_creates_a_history_record_with_current_data( ], ) - assert Service.query.count() == 1 + assert _get_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 _get_service_query_count() == 1 + assert _get_service_history_query_count() == 2 - service_from_db = Service.query.first() + service_from_db = _get_first_service() assert service_from_db.version == 2 @@ -784,10 +804,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 _get_service_query_count() == 1 + assert _get_service_history_query_count() == 3 - service_from_db = Service.query.first() + service_from_db = _get_first_service() assert service_from_db.version == 3 _assert_service_permissions( service.permissions, @@ -797,21 +817,20 @@ 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 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 _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 service = Service( name=None, email_from="email_from", @@ -828,8 +847,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 _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 def test_delete_service_and_associated_objects(notify_db_session): @@ -845,8 +864,8 @@ def test_delete_service_and_associated_objects(notify_db_session): create_notification(template=template, api_key=api_key) create_invited_user(service=service) user.organizations = [organization] - - assert ServicePermission.query.count() == len( + stmt = select(func.count(ServicePermission.service_id)) + assert db.session.execute(stmt).scalar() == len( ( ServicePermissionType.SMS, ServicePermissionType.EMAIL, @@ -855,21 +874,35 @@ 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 + + assert _get_service_query_count() == 0 + assert _get_service_history_query_count() == 0 + stmt = select(ServicePermission) + assert db.session.execute(stmt).scalar() is None + # the organization hasn't been deleted - assert Organization.query.count() == 1 + stmt = select(func.count(Organization.id)) + assert db.session.execute(stmt).scalar() == 1 def test_add_existing_user_to_another_service_doesnot_change_old_permissions( @@ -887,9 +920,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).all() assert len(test_user_permissions) == 7 other_user = User( @@ -909,14 +941,12 @@ 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).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).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 @@ -925,15 +955,12 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( permissions.append(Permission(permission=p)) 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).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).all() assert len(other_user_service_two_permissions) == 7 @@ -956,9 +983,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(func.count(Notification.id)) + assert db.session.execute(stmt).scalar() == 0 + stmt = select(func.count(NotificationHistory.id)) + assert db.session.execute(stmt).scalar() == 1 stats = dao_fetch_todays_stats_for_service(sample_template.service_id) assert len(stats) == 0 @@ -1316,7 +1344,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 = _get_service_by_id(service.id) assert not service.active assert service.name == service.name assert service.api_keys == [] @@ -1329,11 +1357,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 = _get_service_by_id(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 +1372,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 = _get_service_by_id(service.id) assert not service.active dao_resume_service(service.id) - assert Service.query.get(service.id).active + assert _get_service_by_id(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) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 85149b246..8f9f21fe3 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -71,8 +71,10 @@ def test_create_user(notify_db_session, phone_number, expected_phone_number): } user = User(**data) save_model_user(user, password="password", validated_email_access=True) - assert _get_user_query_count() == 1 - user_query = _get_user_query_first() + stmt = select(func.count(User.id)) + assert db.session.execute(stmt).scalar() == 1 + stmt = select(User) + user_query = db.session.execute(stmt).scalars().first() assert user_query.email_address == email assert user_query.id == user.id assert user_query.mobile_number == expected_phone_number @@ -84,7 +86,8 @@ def test_get_all_users(notify_db_session): create_user(email="1@test.com") create_user(email="2@test.com") - assert _get_user_query_count() == 2 + stmt = select(func.count(User.id)) + assert db.session.execute(stmt).scalar() == 2 assert len(get_user_by_id()) == 2 @@ -105,9 +108,10 @@ def test_get_user_invalid_id(notify_db_session): def test_delete_users(sample_user): - assert _get_user_query_count() == 1 + stmt = select(func.count(User.id)) + assert db.session.execute(stmt).scalar() == 1 delete_model_user(sample_user) - assert _get_user_query_count() == 0 + assert db.session.execute(stmt).scalar() == 0 def test_increment_failed_login_should_increment_failed_logins(sample_user): @@ -143,9 +147,10 @@ def test_get_user_by_email_is_case_insensitive(sample_user): def test_should_delete_all_verification_codes_more_than_one_day_old(sample_user): make_verify_code(sample_user, age=timedelta(hours=24), code="54321") make_verify_code(sample_user, age=timedelta(hours=24), code="54321") - assert _get_verify_code_query_count() == 2 + stmt = select(func.count(VerifyCode.id)) + assert db.session.execute(stmt).scalar() == 2 delete_codes_older_created_more_than_a_day_ago() - assert _get_verify_code_query_count() == 0 + assert db.session.execute(stmt).scalar() == 0 def test_should_not_delete_verification_codes_less_than_one_day_old(sample_user): @@ -153,8 +158,8 @@ def test_should_not_delete_verification_codes_less_than_one_day_old(sample_user) sample_user, age=timedelta(hours=23, minutes=59, seconds=59), code="12345" ) make_verify_code(sample_user, age=timedelta(hours=24), code="54321") - - assert _get_verify_code_query_count() == 2 + stmt = select(func.count(VerifyCode.id)) + assert db.session.execute(stmt).scalar() == 2 delete_codes_older_created_more_than_a_day_ago() stmt = select(VerifyCode) assert db.session.execute(stmt).scalars().one()._code == "12345"