From 008c3a8d68c911f177351c99ca533b91a5d74ba0 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 13 Jun 2023 12:57:51 -0700 Subject: [PATCH] initial --- app/celery/provider_tasks.py | 13 ++- app/dao/fact_notification_status_dao.py | 84 ++++++++++---------- app/dao/jobs_dao.py | 21 ++--- app/dao/notifications_dao.py | 7 +- app/delivery/send_to_providers.py | 5 +- app/service/rest.py | 3 + tests/app/delivery/test_send_to_providers.py | 16 ++-- 7 files changed, 76 insertions(+), 73 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 2b3ddee32..9f94b58c6 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -1,6 +1,5 @@ from datetime import datetime, timedelta from time import time -from zoneinfo import ZoneInfo from flask import current_app from sqlalchemy.orm.exc import NoResultFound @@ -18,8 +17,8 @@ from app.dao.notifications_dao import ( from app.delivery import send_to_providers from app.exceptions import NotificationTechnicalFailureException from app.models import ( + NOTIFICATION_DELIVERED, NOTIFICATION_FAILED, - NOTIFICATION_SENT, NOTIFICATION_TECHNICAL_FAILURE, ) @@ -37,15 +36,15 @@ def check_sms_delivery_receipt(self, message_id, notification_id, sent_at): """ status, provider_response = aws_cloudwatch_client.check_sms(message_id, notification_id, sent_at) if status == 'success': - status = NOTIFICATION_SENT + status = NOTIFICATION_DELIVERED else: status = NOTIFICATION_FAILED update_notification_status_by_id(notification_id, status, provider_response=provider_response) current_app.logger.info(f"Updated notification {notification_id} with response '{provider_response}'") - if status == NOTIFICATION_SENT: + if status == NOTIFICATION_DELIVERED: insert_notification_history_delete_notifications_by_id(notification_id) - current_app.logger.info(f"Archived notification {notification_id} that was successfully sent") + current_app.logger.info(f"Archived notification {notification_id} that was successfully delivered") @notify_celery.task(bind=True, name="deliver_sms", max_retries=48, default_retry_delay=300) @@ -58,9 +57,9 @@ def deliver_sms(self, notification_id): if not notification: raise NoResultFound() message_id = send_to_providers.send_sms_to_provider(notification) - # We have to put it in the default US/Eastern timezone. From zones west of there, the delay + # We have to put it in UTC. For other timezones, the delay # will be ignored and it will fire immediately (although this probably only affects developer testing) - my_eta = datetime.now(ZoneInfo('US/Eastern')) + timedelta(seconds=300) + my_eta = datetime.utcnow() + timedelta(seconds=300) check_sms_delivery_receipt.apply_async( [message_id, notification_id, now], eta=my_eta, diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 857256cd3..338de8ab6 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -108,17 +108,17 @@ def fetch_notification_status_for_service_for_day(fetch_day, service_id): return db.session.query( # return current month as a datetime so the data has the same shape as the ft_notification_status query literal(fetch_day.replace(day=1), type_=DateTime).label('month'), - Notification.notification_type, - Notification.status.label('notification_status'), + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status.label('notification_status'), func.count().label('count') ).filter( - Notification.created_at >= get_midnight_in_utc(fetch_day), - Notification.created_at < get_midnight_in_utc(fetch_day + timedelta(days=1)), - Notification.service_id == service_id, - Notification.key_type != KEY_TYPE_TEST + NotificationAllTimeView.created_at >= get_midnight_in_utc(fetch_day), + NotificationAllTimeView.created_at < get_midnight_in_utc(fetch_day + timedelta(days=1)), + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.key_type != KEY_TYPE_TEST ).group_by( - Notification.notification_type, - Notification.status + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status ).all() @@ -137,18 +137,18 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_ ) stats_for_today = db.session.query( - Notification.notification_type.cast(db.Text), - Notification.status, - *([Notification.template_id] if by_template else []), + NotificationAllTimeView.notification_type.cast(db.Text), + NotificationAllTimeView.status, + *([NotificationAllTimeView.template_id] if by_template else []), func.count().label('count') ).filter( - Notification.created_at >= get_midnight_in_utc(now), - Notification.service_id == service_id, - Notification.key_type != KEY_TYPE_TEST + NotificationAllTimeView.created_at >= get_midnight_in_utc(now), + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.key_type != KEY_TYPE_TEST ).group_by( - Notification.notification_type, - *([Notification.template_id] if by_template else []), - Notification.status + NotificationAllTimeView.notification_type, + *([NotificationAllTimeView.template_id] if by_template else []), + NotificationAllTimeView.status ) all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery() @@ -167,12 +167,14 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_ if by_template: query = query.filter(all_stats_table.c.template_id == Template.id) - return query.group_by( + x = query.group_by( *([Template.name, all_stats_table.c.template_id] if by_template else []), all_stats_table.c.notification_type, all_stats_table.c.status, ).all() + return x + def fetch_notification_status_totals_for_all_services(start_date, end_date): stats = db.session.query( @@ -191,16 +193,16 @@ def fetch_notification_status_totals_for_all_services(start_date, end_date): today = get_midnight_in_utc(datetime.utcnow()) if start_date <= datetime.utcnow().date() <= end_date: stats_for_today = db.session.query( - Notification.notification_type.cast(db.Text).label('notification_type'), - Notification.status, - Notification.key_type, + NotificationAllTimeView.notification_type.cast(db.Text).label('notification_type'), + NotificationAllTimeView.status, + NotificationAllTimeView.key_type, func.count().label('count') ).filter( - Notification.created_at >= today + NotificationAllTimeView.created_at >= today ).group_by( - Notification.notification_type.cast(db.Text), - Notification.status, - Notification.key_type, + NotificationAllTimeView.notification_type.cast(db.Text), + NotificationAllTimeView.status, + NotificationAllTimeView.key_type, ) all_stats_table = stats.union_all(stats_for_today).subquery() query = db.session.query( @@ -267,19 +269,19 @@ def fetch_stats_for_all_services_by_date_range(start_date, end_date, include_fro if start_date <= datetime.utcnow().date() <= end_date: today = get_midnight_in_utc(datetime.utcnow()) subquery = db.session.query( - Notification.notification_type.cast(db.Text).label('notification_type'), - Notification.status.label('status'), - Notification.service_id.label('service_id'), + NotificationAllTimeView.notification_type.cast(db.Text).label('notification_type'), + NotificationAllTimeView.status.label('status'), + NotificationAllTimeView.service_id.label('service_id'), func.count(Notification.id).label('count') ).filter( - Notification.created_at >= today + NotificationAllTimeView.created_at >= today ).group_by( - Notification.notification_type, - Notification.status, - Notification.service_id + NotificationAllTimeView.notification_type, + NotificationAllTimeView.status, + NotificationAllTimeView.service_id ) if not include_from_test_key: - subquery = subquery.filter(Notification.key_type != KEY_TYPE_TEST) + subquery = subquery.filter(NotificationAllTimeView.key_type != KEY_TYPE_TEST) subquery = subquery.subquery() stats_for_today = db.session.query( @@ -358,24 +360,24 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): if start_date <= datetime.utcnow() <= end_date: today = get_midnight_in_utc(datetime.utcnow()) - month = get_month_from_utc_column(Notification.created_at) + month = get_month_from_utc_column(NotificationAllTimeView.created_at) stats_for_today = db.session.query( - Notification.template_id.label('template_id'), + NotificationAllTimeView.template_id.label('template_id'), Template.name.label('name'), Template.template_type.label('template_type'), extract('month', month).label('month'), extract('year', month).label('year'), func.count().label('count') ).join( - Template, Notification.template_id == Template.id, + Template, NotificationAllTimeView.template_id == Template.id, ).filter( - Notification.created_at >= today, - Notification.service_id == service_id, - Notification.key_type != KEY_TYPE_TEST, - Notification.status != NOTIFICATION_CANCELLED + NotificationAllTimeView.created_at >= today, + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.key_type != KEY_TYPE_TEST, + NotificationAllTimeView.status != NOTIFICATION_CANCELLED ).group_by( - Notification.template_id, + NotificationAllTimeView.template_id, Template.hidden, Template.name, Template.template_type, diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 8fd7f22c0..0c21e2a9c 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -11,7 +11,7 @@ from app.models import ( JOB_STATUS_SCHEDULED, FactNotificationStatus, Job, - Notification, + NotificationAllTimeView, ServiceDataRetention, Template, ) @@ -20,12 +20,12 @@ from app.utils import midnight_n_days_ago def dao_get_notification_outcomes_for_job(service_id, job_id): notification_statuses = db.session.query( - func.count(Notification.status).label('count'), Notification.status + func.count(NotificationAllTimeView.status).label('count'), NotificationAllTimeView.status ).filter( - Notification.service_id == service_id, - Notification.job_id == job_id + NotificationAllTimeView.service_id == service_id, + NotificationAllTimeView.job_id == job_id ).group_by( - Notification.status + NotificationAllTimeView.status ).all() if not notification_statuses: @@ -185,12 +185,12 @@ def find_jobs_with_missing_rows(): Job.job_status == JOB_STATUS_FINISHED, Job.processing_finished < ten_minutes_ago, Job.processing_finished > yesterday, - Job.id == Notification.job_id, + Job.id == NotificationAllTimeView.job_id, ).group_by( Job ).having( - func.count(Notification.id) != Job.notification_count + func.count(NotificationAllTimeView.id) != Job.notification_count ) return jobs_with_rows_missing.all() @@ -202,11 +202,12 @@ def find_missing_row_for_job(job_id, job_size): ).subquery() query = db.session.query( - Notification.job_row_number, + NotificationAllTimeView.job_row_number, expected_row_numbers.c.row.label('missing_row') ).outerjoin( - Notification, and_(expected_row_numbers.c.row == Notification.job_row_number, Notification.job_id == job_id) + NotificationAllTimeView, and_(expected_row_numbers.c.row == NotificationAllTimeView.job_row_number, + NotificationAllTimeView.job_id == job_id) ).filter( - Notification.job_row_number == None # noqa + NotificationAllTimeView.job_row_number == None # noqa ) return query.all() diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 40138eb5c..d49e72a0e 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -31,6 +31,7 @@ from app.models import ( SMS_TYPE, FactNotificationStatus, Notification, + NotificationAllTimeView, NotificationHistory, ) from app.utils import ( @@ -162,16 +163,16 @@ def dao_update_notification(notification): def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page_size=None): if page_size is None: page_size = current_app.config['PAGE_SIZE'] - query = Notification.query.filter_by(service_id=service_id, job_id=job_id) + query = NotificationAllTimeView.query.filter_by(service_id=service_id, job_id=job_id) query = _filter_query(query, filter_dict) - return query.order_by(asc(Notification.job_row_number)).paginate( + return query.order_by(asc(NotificationAllTimeView.job_row_number)).paginate( page=page, per_page=page_size ) def dao_get_notification_count_for_job_id(*, job_id): - return Notification.query.filter_by(job_id=job_id).count() + return NotificationAllTimeView.query.filter_by(job_id=job_id).count() def get_notification_with_personalisation(service_id, notification_id, key_type): diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index cd4a766a3..79cefc2fb 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -28,7 +28,6 @@ from app.models import ( EMAIL_TYPE, KEY_TYPE_TEST, NOTIFICATION_SENDING, - NOTIFICATION_SENT, NOTIFICATION_STATUS_TYPES_COMPLETED, NOTIFICATION_TECHNICAL_FAILURE, SMS_TYPE, @@ -137,9 +136,7 @@ def update_notification_to_sending(notification, provider): notification.sent_at = datetime.utcnow() notification.sent_by = provider.name if notification.status not in NOTIFICATION_STATUS_TYPES_COMPLETED: - # We currently have no callback method for SMS deliveries - # TODO create celery task to request SMS delivery receipts from cloudwatch api - notification.status = NOTIFICATION_SENT if notification.notification_type == "sms" else NOTIFICATION_SENDING + notification.status = NOTIFICATION_SENDING dao_update_notification(notification) diff --git a/app/service/rest.py b/app/service/rest.py index 43d3ec63a..3d2b14073 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -515,10 +515,13 @@ def get_detailed_service(service_id, today_only=False): def get_service_statistics(service_id, today_only, limit_days=7): # today_only flag is used by the send page to work out if the service will exceed their daily usage by sending a job if today_only: + print("TODAY ONLY") stats = dao_fetch_todays_stats_for_service(service_id) else: + print("PAST WEEK") stats = fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, limit_days=limit_days) + print(f"GET_SERVICE_STATISTICS returns {statistics.format_statistics(stats)}") return statistics.format_statistics(stats) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 21e854c26..a4a264f19 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -98,7 +98,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( notification = Notification.query.filter_by(id=db_notification.id).one() - assert notification.status == 'sent' + assert notification.status == 'sending' assert notification.sent_at <= datetime.utcnow() assert notification.sent_by == 'sns' assert notification.billable_units == 1 @@ -207,7 +207,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( assert persisted_notification.template_id == expected_template_id assert persisted_notification.template_version == version_on_notification assert persisted_notification.template_version != t.version - assert persisted_notification.status == 'sent' + assert persisted_notification.status == 'sending' assert not persisted_notification.personalisation @@ -240,7 +240,7 @@ def test_should_call_send_sms_response_task_if_research_mode( persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) assert persisted_notification.to == sample_notification.to assert persisted_notification.template_id == sample_notification.template_id - assert persisted_notification.status == 'sent' + assert persisted_notification.status == 'sending' assert persisted_notification.sent_at <= datetime.utcnow() assert persisted_notification.sent_by == 'sns' assert not persisted_notification.personalisation @@ -254,7 +254,7 @@ def test_should_have_sending_status_if_fake_callback_function_fails(sample_notif send_to_providers.send_sms_to_provider( sample_notification ) - assert sample_notification.status == 'sent' + assert sample_notification.status == 'sending' assert sample_notification.sent_by == 'sns' @@ -534,7 +534,7 @@ def test_should_not_update_notification_if_research_mode_on_exception( @pytest.mark.parametrize("starting_status, expected_status", [ ("delivered", "delivered"), - ("created", "sent"), + ("created", "sending"), ("technical-failure", "technical-failure"), ]) def test_update_notification_to_sending_does_not_update_status_from_a_final_status( @@ -556,11 +556,11 @@ def __update_notification(notification_to_update, research_mode, expected_status @pytest.mark.parametrize('research_mode,key_type, billable_units, expected_status', [ (True, KEY_TYPE_NORMAL, 0, 'delivered'), - (False, KEY_TYPE_NORMAL, 1, 'sent'), + (False, KEY_TYPE_NORMAL, 1, 'sending'), (False, KEY_TYPE_TEST, 0, 'sending'), (True, KEY_TYPE_TEST, 0, 'sending'), (True, KEY_TYPE_TEAM, 0, 'delivered'), - (False, KEY_TYPE_TEAM, 1, 'sent') + (False, KEY_TYPE_TEAM, 1, 'sending') ]) def test_should_update_billable_units_and_status_according_to_research_mode_and_key_type( sample_template, @@ -631,7 +631,7 @@ def test_should_send_sms_to_international_providers( international=True ) - assert notification_international.status == 'sent' + assert notification_international.status == 'sending' assert notification_international.sent_by == 'sns'