Merge pull request #1417 from GSA/clean_up_sqlalchemy

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
       }
     ],
@@ -341,15 +341,15 @@
         "filename": "tests/app/user/test_rest.py",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 108,
+        "line_number": 110,
         "is_secret": false
       },
       {
         "type": "Secret Keyword",
         "filename": "tests/app/user/test_rest.py",
         "hashed_secret": "0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33",
         "is_verified": false,
-        "line_number": 826,
+        "line_number": 864,
         "is_secret": false
       }
     ],
@@ -384,5 +384,5 @@
       }
     ]
   },
-  "generated_at": "2024-10-31T21:25:32Z"
+  "generated_at": "2024-12-19T19:09:50Z"
 }

Makefile

@@ -83,7 +83,7 @@ test: export NEW_RELIC_ENVIRONMENT=test
 test: ## Run tests and create coverage report
 	poetry run black .
 	poetry run flake8 .
-	poetry run isort --check-only ./app ./tests
+	poetry run isort ./app ./tests
 	poetry run coverage run --omit=*/migrations/*,*/tests/* -m pytest --maxfail=10
 
     ## TODO set this back to 95 asap

app/billing/rest.py

@@ -1,5 +1,6 @@
 from flask import Blueprint, jsonify, request
 
+from app import db
 from app.billing.billing_schemas import (
     create_or_update_free_sms_fragment_limit_schema,
     serialize_ft_billing_remove_emails,
@@ -60,7 +61,7 @@ def get_free_sms_fragment_limit(service_id):
     )
 
     if annual_billing is None:
-        service = Service.query.get(service_id)
+        service = db.session.get(Service, service_id)
         # An entry does not exist in annual_billing table for that service and year.
         # Set the annual billing to the default free allowance based on the organization type of the service.
 

app/celery/scheduled_tasks.py

@@ -2,10 +2,10 @@
 from datetime import datetime, timedelta
 
 from flask import current_app
-from sqlalchemy import between
+from sqlalchemy import between, select, union
 from sqlalchemy.exc import SQLAlchemyError
 
-from app import notify_celery, redis_store, zendesk_client
+from app import db, notify_celery, redis_store, zendesk_client
 from app.celery.tasks import (
     get_recipient_csv_and_template_and_sender_id,
     process_incomplete_jobs,
@@ -20,7 +20,7 @@
 from app.dao.invited_user_dao import expire_invitations_created_more_than_two_days_ago
 from app.dao.jobs_dao import (
     dao_set_scheduled_jobs_to_pending,
-    dao_update_job,
+    dao_update_job_status_to_error,
     find_jobs_with_missing_rows,
     find_missing_row_for_job,
 )
@@ -115,30 +115,34 @@ def check_job_status():
     end_minutes_ago = utc_now() - timedelta(minutes=END_MINUTES)
     start_minutes_ago = utc_now() - timedelta(minutes=START_MINUTES)
 
-    incomplete_in_progress_jobs = Job.query.filter(
+    incomplete_in_progress_jobs = select(Job).where(
         Job.job_status == JobStatus.IN_PROGRESS,
         between(Job.processing_started, start_minutes_ago, end_minutes_ago),
     )
-    incomplete_pending_jobs = Job.query.filter(
+    incomplete_pending_jobs = select(Job).where(
         Job.job_status == JobStatus.PENDING,
         Job.scheduled_for.isnot(None),
         between(Job.scheduled_for, start_minutes_ago, end_minutes_ago),
     )
-
-    jobs_not_complete_after_allotted_time = (
-        incomplete_in_progress_jobs.union(incomplete_pending_jobs)
-        .order_by(Job.processing_started, Job.scheduled_for)
-        .all()
+    jobs_not_completed_after_allotted_time = union(
+        incomplete_in_progress_jobs, incomplete_pending_jobs
+    )
+    jobs_not_completed_after_allotted_time = (
+        jobs_not_completed_after_allotted_time.order_by(
+            Job.processing_started, Job.scheduled_for
+        )
     )
 
+    jobs_not_complete_after_allotted_time = db.session.execute(
+        jobs_not_completed_after_allotted_time
+    ).all()
+
     # temporarily mark them as ERROR so that they don't get picked up by future check_job_status tasks
     # if they haven't been re-processed in time.
     job_ids = []
     for job in jobs_not_complete_after_allotted_time:
-        job.job_status = JobStatus.ERROR
-        dao_update_job(job)
+        dao_update_job_status_to_error(job)
         job_ids.append(str(job.id))
-
     if job_ids:
         current_app.logger.info("Job(s) {} have not completed.".format(job_ids))
         process_incomplete_jobs.apply_async([job_ids], queue=QueueNames.JOBS)
@@ -168,6 +172,7 @@ def replay_created_notifications():
 
 @notify_celery.task(name="check-for-missing-rows-in-completed-jobs")
 def check_for_missing_rows_in_completed_jobs():
+
     jobs = find_jobs_with_missing_rows()
     for job in jobs:
         (

app/commands.py

@@ -656,7 +656,7 @@ def populate_annual_billing_with_defaults(year, missing_services_only):
                     AnnualBilling.financial_year_start == year,
                 ),
             )
-            .filter(AnnualBilling.id == None)  # noqa
+            .where(AnnualBilling.id == None)  # noqa
         )
         active_services = db.session.execute(stmt).scalars().all()
     else:
@@ -665,7 +665,7 @@ def populate_annual_billing_with_defaults(year, missing_services_only):
     previous_year = year - 1
     services_with_zero_free_allowance = (
         db.session.query(AnnualBilling.service_id)
-        .filter(
+        .where(
             AnnualBilling.financial_year_start == previous_year,
             AnnualBilling.free_sms_fragment_limit == 0,
         )

app/dao/annual_billing_dao.py

@@ -29,8 +29,8 @@ def dao_create_or_update_annual_billing_for_year(
 def dao_get_annual_billing(service_id):
     stmt = (
         select(AnnualBilling)
-        .filter_by(
-            service_id=service_id,
+        .where(
+            AnnualBilling.service_id == service_id,
         )
         .order_by(AnnualBilling.financial_year_start)
     )
@@ -43,7 +43,7 @@ def dao_update_annual_billing_for_future_years(
 ):
     stmt = (
         update(AnnualBilling)
-        .filter(
+        .where(
             AnnualBilling.service_id == service_id,
             AnnualBilling.financial_year_start > financial_year_start,
         )
@@ -57,17 +57,18 @@ def dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start=No
     if not financial_year_start:
         financial_year_start = get_current_calendar_year_start_year()
 
-    stmt = select(AnnualBilling).filter_by(
-        service_id=service_id, financial_year_start=financial_year_start
+    stmt = select(AnnualBilling).where(
+        AnnualBilling.service_id == service_id,
+        AnnualBilling.financial_year_start == financial_year_start,
     )
     return db.session.execute(stmt).scalars().first()
 
 
 def dao_get_all_free_sms_fragment_limit(service_id):
     stmt = (
         select(AnnualBilling)
-        .filter_by(
-            service_id=service_id,
+        .where(
+            AnnualBilling.service_id == service_id,
         )
         .order_by(AnnualBilling.financial_year_start)
     )

app/dao/api_key_dao.py

@@ -1,7 +1,7 @@
 import uuid
 from datetime import timedelta
 
-from sqlalchemy import func, or_
+from sqlalchemy import func, or_, select
 
 from app import db
 from app.dao.dao_utils import autocommit, version_class
@@ -23,31 +23,61 @@ def save_model_api_key(api_key):
 @autocommit
 @version_class(ApiKey)
 def expire_api_key(service_id, api_key_id):
-    api_key = ApiKey.query.filter_by(id=api_key_id, service_id=service_id).one()
+    api_key = (
+        db.session.execute(
+            select(ApiKey).where(
+                ApiKey.id == api_key_id, ApiKey.service_id == service_id
+            )
+        )
+        .scalars()
+        .one()
+    )
     api_key.expiry_date = utc_now()
     db.session.add(api_key)
 
 
 def get_model_api_keys(service_id, id=None):
     if id:
-        return ApiKey.query.filter_by(
-            id=id, service_id=service_id, expiry_date=None
-        ).one()
+        return (
+            db.session.execute(
+                select(ApiKey).where(
+                    ApiKey.id == id,
+                    ApiKey.service_id == service_id,
+                    ApiKey.expiry_date == None,  # noqa
+                )
+            )
+            .scalars()
+            .one()
+        )
     seven_days_ago = utc_now() - timedelta(days=7)
-    return ApiKey.query.filter(
-        or_(
-            ApiKey.expiry_date == None,  # noqa
-            func.date(ApiKey.expiry_date) > seven_days_ago,  # noqa
-        ),
-        ApiKey.service_id == service_id,
-    ).all()
+    return (
+        db.session.execute(
+            select(ApiKey).where(
+                or_(
+                    ApiKey.expiry_date == None,  # noqa
+                    func.date(ApiKey.expiry_date) > seven_days_ago,  # noqa
+                ),
+                ApiKey.service_id == service_id,
+            )
+        )
+        .scalars()
+        .all()
+    )
 
 
 def get_unsigned_secrets(service_id):
     """
     This method can only be exposed to the Authentication of the api calls.
     """
-    api_keys = ApiKey.query.filter_by(service_id=service_id, expiry_date=None).all()
+    api_keys = (
+        db.session.execute(
+            select(ApiKey).where(
+                ApiKey.service_id == service_id, ApiKey.expiry_date == None  # noqa
+            )
+        )
+        .scalars()
+        .all()
+    )
     keys = [x.secret for x in api_keys]
     return keys
 
@@ -56,5 +86,13 @@ def get_unsigned_secret(key_id):
     """
     This method can only be exposed to the Authentication of the api calls.
     """
-    api_key = ApiKey.query.filter_by(id=key_id, expiry_date=None).one()
+    api_key = (
+        db.session.execute(
+            select(ApiKey).where(
+                ApiKey.id == key_id, ApiKey.expiry_date == None  # noqa
+            )
+        )
+        .scalars()
+        .one()
+    )
     return api_key.secret

app/dao/complaint_dao.py

@@ -33,7 +33,7 @@ def fetch_paginated_complaints(page=1):
 def fetch_complaints_by_service(service_id):
     stmt = (
         select(Complaint)
-        .filter_by(service_id=service_id)
+        .where(Complaint.service_id == service_id)
         .order_by(desc(Complaint.created_at))
     )
     return db.session.execute(stmt).scalars().all()
@@ -46,6 +46,6 @@ def fetch_count_of_complaints(start_date, end_date):
     stmt = (
         select(func.count())
         .select_from(Complaint)
-        .filter(Complaint.created_at >= start_date, Complaint.created_at < end_date)
+        .where(Complaint.created_at >= start_date, Complaint.created_at < end_date)
     )
     return db.session.execute(stmt).scalar() or 0

app/dao/email_branding_dao.py

@@ -1,18 +1,32 @@
+from sqlalchemy import select
+
 from app import db
 from app.dao.dao_utils import autocommit
 from app.models import EmailBranding
 
 
 def dao_get_email_branding_options():
-    return EmailBranding.query.all()
+    return db.session.execute(select(EmailBranding)).scalars().all()
 
 
 def dao_get_email_branding_by_id(email_branding_id):
-    return EmailBranding.query.filter_by(id=email_branding_id).one()
+    return (
+        db.session.execute(
+            select(EmailBranding).where(EmailBranding.id == email_branding_id)
+        )
+        .scalars()
+        .one()
+    )
 
 
 def dao_get_email_branding_by_name(email_branding_name):
-    return EmailBranding.query.filter_by(name=email_branding_name).first()
+    return (
+        db.session.execute(
+            select(EmailBranding).where(EmailBranding.name == email_branding_name)
+        )
+        .scalars()
+        .first()
+    )
 
 
 @autocommit