more code review feedback

This commit is contained in:
Kenneth Kehl
2024-12-20 08:09:19 -08:00
parent a77343ebc7
commit 3c0621f472
22 changed files with 125 additions and 133 deletions

View File

@@ -30,7 +30,7 @@ from notifications_utils.recipients import (
def dao_get_last_date_template_was_used(template_id, service_id):
last_date_from_notifications = (
db.session.query(functions.max(Notification.created_at))
.filter(
.where(
Notification.service_id == service_id,
Notification.template_id == template_id,
Notification.key_type != KeyType.TEST,
@@ -43,7 +43,7 @@ def dao_get_last_date_template_was_used(template_id, service_id):
last_date = (
db.session.query(functions.max(FactNotificationStatus.local_date))
.filter(
.where(
FactNotificationStatus.template_id == template_id,
FactNotificationStatus.key_type != KeyType.TEST,
)
@@ -126,9 +126,7 @@ def update_notification_status_by_id(
notification_id, status, sent_by=None, provider_response=None, carrier=None
):
stmt = (
select(Notification)
.with_for_update()
.filter(Notification.id == notification_id)
select(Notification).with_for_update().where(Notification.id == notification_id)
)
notification = db.session.execute(stmt).scalars().first()
@@ -173,7 +171,7 @@ def update_notification_status_by_id(
@autocommit
def update_notification_status_by_reference(reference, status):
# this is used to update emails
stmt = select(Notification).filter(Notification.reference == reference)
stmt = select(Notification).where(Notification.reference == reference)
notification = db.session.execute(stmt).scalars().first()
if not notification:
@@ -271,7 +269,7 @@ def get_notification_by_id(notification_id, service_id=None, _raise=False):
if service_id:
filters.append(Notification.service_id == service_id)
stmt = select(Notification).filter(*filters)
stmt = select(Notification).where(*filters)
return (
db.session.execute(stmt).scalars().one()
@@ -307,7 +305,7 @@ def get_notifications_for_service(
if older_than is not None:
older_than_created_at = (
db.session.query(Notification.created_at)
.filter(Notification.id == older_than)
.where(Notification.id == older_than)
.as_scalar()
)
filters.append(Notification.created_at < older_than_created_at)
@@ -457,7 +455,7 @@ def move_notifications_to_notification_history(
deleted += delete_count_per_call
# Deleting test Notifications, test notifications are not persisted to NotificationHistory
stmt = delete(Notification).filter(
stmt = delete(Notification).where(
Notification.notification_type == notification_type,
Notification.service_id == service_id,
Notification.created_at < timestamp_to_delete_backwards_from,
@@ -471,7 +469,7 @@ def move_notifications_to_notification_history(
@autocommit
def dao_delete_notifications_by_id(notification_id):
db.session.query(Notification).filter(Notification.id == notification_id).delete(
db.session.query(Notification).where(Notification.id == notification_id).delete(
synchronize_session="fetch"
)
@@ -487,7 +485,7 @@ def dao_timeout_notifications(cutoff_time, limit=100000):
stmt = (
select(Notification)
.filter(
.where(
Notification.created_at < cutoff_time,
Notification.status.in_(current_statuses),
Notification.notification_type.in_(
@@ -500,7 +498,7 @@ def dao_timeout_notifications(cutoff_time, limit=100000):
stmt = (
update(Notification)
.filter(Notification.id.in_([n.id for n in notifications]))
.where(Notification.id.in_([n.id for n in notifications]))
.values({"status": new_status, "updated_at": updated_at})
)
db.session.execute(stmt)
@@ -513,7 +511,7 @@ def dao_timeout_notifications(cutoff_time, limit=100000):
def dao_update_notifications_by_reference(references, update_dict):
stmt = (
update(Notification)
.filter(Notification.reference.in_(references))
.where(Notification.reference.in_(references))
.values(update_dict)
)
result = db.session.execute(stmt)
@@ -523,7 +521,7 @@ def dao_update_notifications_by_reference(references, update_dict):
if updated_count != len(references):
stmt = (
update(NotificationHistory)
.filter(NotificationHistory.reference.in_(references))
.where(NotificationHistory.reference.in_(references))
.values(update_dict)
)
result = db.session.execute(stmt)
@@ -586,7 +584,7 @@ def dao_get_notifications_by_recipient_or_reference(
results = (
db.session.query(Notification)
.filter(*filters)
.where(*filters)
.order_by(desc(Notification.created_at))
.paginate(page=page, per_page=page_size, count=False, error_out=error_out)
)
@@ -594,7 +592,7 @@ def dao_get_notifications_by_recipient_or_reference(
def dao_get_notification_by_reference(reference):
stmt = select(Notification).filter(Notification.reference == reference)
stmt = select(Notification).where(Notification.reference == reference)
return db.session.execute(stmt).scalars().one()
@@ -602,10 +600,10 @@ def dao_get_notification_history_by_reference(reference):
try:
# This try except is necessary because in test keys and research mode does not create notification history.
# Otherwise we could just search for the NotificationHistory object
stmt = select(Notification).filter(Notification.reference == reference)
stmt = select(Notification).where(Notification.reference == reference)
return db.session.execute(stmt).scalars().one()
except NoResultFound:
stmt = select(NotificationHistory).filter(
stmt = select(NotificationHistory).where(
NotificationHistory.reference == reference
)
return db.session.execute(stmt).scalars().one()
@@ -648,7 +646,7 @@ def dao_get_notifications_processing_time_stats(start_date, end_date):
def dao_get_last_notification_added_for_job_id(job_id):
stmt = (
select(Notification)
.filter(Notification.job_id == job_id)
.where(Notification.job_id == job_id)
.order_by(Notification.job_row_number.desc())
)
last_notification_added = db.session.execute(stmt).scalars().first()
@@ -659,7 +657,7 @@ def dao_get_last_notification_added_for_job_id(job_id):
def notifications_not_yet_sent(should_be_sending_after_seconds, notification_type):
older_than_date = utc_now() - timedelta(seconds=should_be_sending_after_seconds)
stmt = select(Notification).filter(
stmt = select(Notification).where(
Notification.created_at <= older_than_date,
Notification.notification_type == notification_type,
Notification.status == NotificationStatus.CREATED,
@@ -691,7 +689,7 @@ def get_service_ids_with_notifications_before(notification_type, timestamp):
return {
row.service_id
for row in db.session.query(Notification.service_id)
.filter(
.where(
Notification.notification_type == notification_type,
Notification.created_at < timestamp,
)
@@ -705,7 +703,7 @@ def get_service_ids_with_notifications_on_date(notification_type, date):
notification_table_query = db.session.query(
Notification.service_id.label("service_id")
).filter(
).where(
Notification.notification_type == notification_type,
# using >= + < is much more efficient than date(created_at)
Notification.created_at >= start_date,
@@ -716,7 +714,7 @@ def get_service_ids_with_notifications_on_date(notification_type, date):
# provided the task to populate it has run before they were archived.
ft_status_table_query = db.session.query(
FactNotificationStatus.service_id.label("service_id")
).filter(
).where(
FactNotificationStatus.notification_type == notification_type,
FactNotificationStatus.local_date == date,
)