fix more sqlalchemy

.ds.baseline

@@ -305,7 +305,7 @@
         "filename": "tests/app/service/test_rest.py",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 1284,
+        "line_number": 1285,
         "is_secret": false
       }
     ],
@@ -384,5 +384,5 @@
       }
     ]
   },
-  "generated_at": "2024-10-31T21:25:32Z"
+  "generated_at": "2024-11-14T22:52:47Z"
 }

tests/app/celery/test_reporting_tasks.py

@@ -4,7 +4,7 @@
 
 import pytest
 from freezegun import freeze_time
-from sqlalchemy import select
+from sqlalchemy import func, select
 
 from app import db
 from app.celery.reporting_tasks import (
@@ -363,9 +363,12 @@ def test_create_nightly_billing_for_day_use_BST(
         rate_multiplier=1.0,
         billable_units=4,
     )
-
-    assert Notification.query.count() == 3
-    assert FactBilling.query.count() == 0
+    stmt = select(func.count()).select_from(Notification)
+    count = db.session.execute(stmt).scalar() or 0
+    assert count == 3
+    stmt = select(func.count()).select_from(FactBilling)
+    count = db.session.execute(stmt).scalar() or 0
+    assert count == 0
 
     create_nightly_billing_for_day("2018-03-25")
     records = FactBilling.query.order_by(FactBilling.local_date).all()

tests/app/celery/test_tasks.py

@@ -412,7 +412,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(
         encryption.encrypt(notification),
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert persisted_notification.template_id == sample_template_with_placeholders.id
     assert (
@@ -431,6 +431,11 @@ def test_should_send_template_to_correct_sms_task_and_persist(
     )
 
 
+def _get_notification_query_one():
+    stmt = select(Notification)
+    return db.session.execute(stmt).scalars().one()
+
+
 def test_should_save_sms_if_restricted_service_and_valid_number(
     notify_db_session, mocker
 ):
@@ -451,7 +456,7 @@ def test_should_save_sms_if_restricted_service_and_valid_number(
         encrypt_notification,
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert persisted_notification.template_id == template.id
     assert persisted_notification.template_version == template.version
@@ -490,7 +495,7 @@ def test_save_email_should_save_default_email_reply_to_text_on_notification(
         encryption.encrypt(notification),
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.reply_to_text == "reply_to@digital.fake.gov"
 
 
@@ -510,7 +515,7 @@ def test_save_sms_should_save_default_sms_sender_notification_reply_to_text_on(
         encryption.encrypt(notification),
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.reply_to_text == "12345"
 
 
@@ -534,6 +539,11 @@ def test_should_not_save_sms_if_restricted_service_and_invalid_number(
     assert _get_notification_query_count() == 0
 
 
+def _get_notification_query_all():
+    stmt = select(Notification)
+    return db.session.execute(stmt).scalars().all()
+
+
 def _get_notification_query_count():
     stmt = select(func.count()).select_from(Notification)
     return db.session.execute(stmt).scalar() or 0
@@ -577,7 +587,7 @@ def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, mocker)
         notification_id,
         encryption.encrypt(notification),
     )
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert persisted_notification.job_id == sample_job.id
     assert persisted_notification.template_id == sample_job.template.id
@@ -642,7 +652,7 @@ def test_should_use_email_template_and_persist(
             encryption.encrypt(notification),
         )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert (
         persisted_notification.template_id == sample_email_template_with_placeholders.id
@@ -689,7 +699,7 @@ def test_save_email_should_use_template_version_from_job_not_latest(
         encryption.encrypt(notification),
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert persisted_notification.template_id == sample_email_template.id
     assert persisted_notification.template_version == version_on_notification
@@ -718,7 +728,7 @@ def test_should_use_email_template_subject_placeholders(
         notification_id,
         encryption.encrypt(notification),
     )
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert (
         persisted_notification.template_id == sample_email_template_with_placeholders.id
@@ -759,7 +769,7 @@ def test_save_email_uses_the_reply_to_text_when_provided(sample_email_template,
         encryption.encrypt(notification),
         sender_id=other_email_reply_to.id,
     )
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.notification_type == NotificationType.EMAIL
     assert persisted_notification.reply_to_text == "other@example.com"
 
@@ -784,7 +794,7 @@ def test_save_email_uses_the_default_reply_to_text_if_sender_id_is_none(
         encryption.encrypt(notification),
         sender_id=None,
     )
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.notification_type == NotificationType.EMAIL
     assert persisted_notification.reply_to_text == "default@example.com"
 
@@ -803,7 +813,7 @@ def test_should_use_email_template_and_persist_without_personalisation(
         notification_id,
         encryption.encrypt(notification),
     )
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.to == "1"
     assert persisted_notification.template_id == sample_email_template.id
     assert persisted_notification.created_at >= now
@@ -936,7 +946,7 @@ def test_save_sms_uses_sms_sender_reply_to_text(mocker, notify_db_session):
         encryption.encrypt(notification),
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.reply_to_text == "+12028675309"
 
 
@@ -962,7 +972,7 @@ def test_save_sms_uses_non_default_sms_sender_reply_to_text_if_provided(
         sender_id=new_sender.id,
     )
 
-    persisted_notification = Notification.query.one()
+    persisted_notification = _get_notification_query_one()
     assert persisted_notification.reply_to_text == "new-sender"
 
 
@@ -1476,12 +1486,12 @@ def test_save_api_email_or_sms(mocker, sample_service, notification_type):
 
     encrypted = encryption.encrypt(data)
 
-    assert len(Notification.query.all()) == 0
+    assert len(_get_notification_query_all()) == 0
     if notification_type == NotificationType.EMAIL:
         save_api_email(encrypted_notification=encrypted)
     else:
         save_api_sms(encrypted_notification=encrypted)
-    notifications = Notification.query.all()
+    notifications = _get_notification_query_all()
     assert len(notifications) == 1
     assert str(notifications[0].id) == data["id"]
     assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
@@ -1529,20 +1539,20 @@ def test_save_api_email_dont_retry_if_notification_already_exists(
         expected_queue = QueueNames.SEND_SMS
 
     encrypted = encryption.encrypt(data)
-    assert len(Notification.query.all()) == 0
+    assert len(_get_notification_query_all()) == 0
 
     if notification_type == NotificationType.EMAIL:
         save_api_email(encrypted_notification=encrypted)
     else:
         save_api_sms(encrypted_notification=encrypted)
-    notifications = Notification.query.all()
+    notifications = _get_notification_query_all()
     assert len(notifications) == 1
     # call the task again with the same notification
     if notification_type == NotificationType.EMAIL:
         save_api_email(encrypted_notification=encrypted)
     else:
         save_api_sms(encrypted_notification=encrypted)
-    notifications = Notification.query.all()
+    notifications = _get_notification_query_all()
     assert len(notifications) == 1
     assert str(notifications[0].id) == data["id"]
     assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
@@ -1606,7 +1616,7 @@ def test_save_tasks_use_cached_service_and_template(
     ]
 
     # But we save 2 notifications and enqueue 2 tasks
-    assert len(Notification.query.all()) == 2
+    assert len(_get_notification_query_all()) == 2
     assert len(delivery_mock.call_args_list) == 2
 
 
@@ -1667,12 +1677,12 @@ def create_encrypted_notification():
             }
         )
 
-    assert len(Notification.query.all()) == 0
+    assert len(_get_notification_query_all()) == 0
 
     for _ in range(3):
         task_function(encrypted_notification=create_encrypted_notification())
 
     assert service_dict_mock.call_args_list == [call(str(template.service_id))]
 
-    assert len(Notification.query.all()) == 3
+    assert len(_get_notification_query_all()) == 3
     assert len(mock_provider_task.call_args_list) == 3

tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py

@@ -43,11 +43,21 @@ def test_move_notifications_does_nothing_if_notification_history_row_already_exi
     )
 
     assert _get_notification_count() == 0
-    history = NotificationHistory.query.all()
+    history = _get_notification_history_query_all()
     assert len(history) == 1
     assert history[0].status == NotificationStatus.DELIVERED
 
 
+def _get_notification_query_all():
+    stmt = select(Notification)
+    return db.session.execute(stmt).scalars().all()
+
+
+def _get_notification_history_query_all():
+    stmt = select(NotificationHistory)
+    return db.session.execute(stmt).scalars().all()
+
+
 def _get_notification_count():
     stmt = select(func.count()).select_from(Notification)
     return db.session.execute(stmt).scalar() or 0
@@ -76,8 +86,18 @@ def test_move_notifications_only_moves_notifications_older_than_provided_timesta
     )
     assert result == 1
 
-    assert Notification.query.one().id == new_notification.id
-    assert NotificationHistory.query.one().id == old_notification_id
+    assert _get_notification_query_one().id == new_notification.id
+    assert _get_notification_history_query_one().id == old_notification_id
+
+
+def _get_notification_query_one():
+    stmt = select(Notification)
+    return db.session.execute(stmt).scalars().one()
+
+
+def _get_notification_history_query_one():
+    stmt = select(NotificationHistory)
+    return db.session.execute(stmt).scalars().one()
 
 
 def test_move_notifications_keeps_calling_until_no_more_to_delete_and_then_returns_total_deleted(
@@ -123,7 +143,9 @@ def test_move_notifications_only_moves_for_given_notification_type(sample_servic
     )
     assert result == 1
     assert {x.notification_type for x in Notification.query} == {NotificationType.EMAIL}
-    assert NotificationHistory.query.one().notification_type == NotificationType.SMS
+    assert (
+        _get_notification_history_query_one().notification_type == NotificationType.SMS
+    )
 
 
 def test_move_notifications_only_moves_for_given_service(notify_db_session):
@@ -146,8 +168,8 @@ def test_move_notifications_only_moves_for_given_service(notify_db_session):
     )
     assert result == 1
 
-    assert NotificationHistory.query.one().service_id == service.id
-    assert Notification.query.one().service_id == other_service.id
+    assert _get_notification_history_query_one().service_id == service.id
+    assert _get_notification_query_one().service_id == other_service.id
 
 
 def test_move_notifications_just_deletes_test_key_notifications(sample_template):
@@ -258,8 +280,8 @@ def test_insert_notification_history_delete_notifications(sample_email_template)
         timestamp_to_delete_backwards_from=utc_now() - timedelta(days=1),
     )
     assert del_count == 8
-    notifications = Notification.query.all()
-    history_rows = NotificationHistory.query.all()
+    notifications = _get_notification_query_all()
+    history_rows = _get_notification_history_query_all()
     assert len(history_rows) == 8
     assert ids_to_move == sorted([x.id for x in history_rows])
     assert len(notifications) == 3
@@ -293,8 +315,8 @@ def test_insert_notification_history_delete_notifications_more_notifications_tha
     )
 
     assert del_count == 1
-    notifications = Notification.query.all()
-    history_rows = NotificationHistory.query.all()
+    notifications = _get_notification_query_all()
+    history_rows = _get_notification_history_query_all()
     assert len(history_rows) == 1
     assert len(notifications) == 2
 
@@ -324,8 +346,8 @@ def test_insert_notification_history_delete_notifications_only_insert_delete_for
     )
 
     assert del_count == 1
-    notifications = Notification.query.all()
-    history_rows = NotificationHistory.query.all()
+    notifications = _get_notification_query_all()
+    history_rows = _get_notification_history_query_all()
     assert len(notifications) == 1
     assert len(history_rows) == 1
     assert notifications[0].id == notification_to_stay.id
@@ -361,8 +383,8 @@ def test_insert_notification_history_delete_notifications_insert_for_key_type(
     )
 
     assert del_count == 2
-    notifications = Notification.query.all()
-    history_rows = NotificationHistory.query.all()
+    notifications = _get_notification_query_all()
+    history_rows = _get_notification_history_query_all()
     assert len(notifications) == 1
     assert with_test_key.id == notifications[0].id
     assert len(history_rows) == 2

tests/app/dao/test_annual_billing_dao.py

@@ -1,6 +1,8 @@
 import pytest
 from freezegun import freeze_time
+from sqlalchemy import select
 
+from app import db
 from app.dao.annual_billing_dao import (
     dao_create_or_update_annual_billing_for_year,
     dao_get_free_sms_fragment_limit_for_year,
@@ -87,7 +89,7 @@ def test_set_default_free_allowance_for_service(
 
     set_default_free_allowance_for_service(service=service, year_start=year)
 
-    annual_billing = AnnualBilling.query.all()
+    annual_billing = db.session.execute(select(AnnualBilling)).scalars().all()
 
     assert len(annual_billing) == 1
     assert annual_billing[0].service_id == service.id
@@ -109,7 +111,7 @@ def test_set_default_free_allowance_for_service_using_correct_year(
 @freeze_time("2021-04-01 14:02:00")
 def test_set_default_free_allowance_for_service_updates_existing_year(sample_service):
     set_default_free_allowance_for_service(service=sample_service, year_start=None)
-    annual_billing = AnnualBilling.query.all()
+    annual_billing = db.session.execute(select(AnnualBilling)).scalars().all()
     assert not sample_service.organization_type
     assert len(annual_billing) == 1
     assert annual_billing[0].service_id == sample_service.id
@@ -118,7 +120,7 @@ def test_set_default_free_allowance_for_service_updates_existing_year(sample_ser
     sample_service.organization_type = OrganizationType.FEDERAL
 
     set_default_free_allowance_for_service(service=sample_service, year_start=None)
-    annual_billing = AnnualBilling.query.all()
+    annual_billing = db.session.execute(select(AnnualBilling)).scalars().all()
     assert len(annual_billing) == 1
     assert annual_billing[0].service_id == sample_service.id
     assert annual_billing[0].free_sms_fragment_limit == 150000