diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index a810ff0db..df8e653ee 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -1,9 +1,10 @@ from datetime import timedelta -from sqlalchemy import Date, case, func +from sqlalchemy import Date, case, cast, func, select, union_all from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.orm import aliased from sqlalchemy.sql.expression import extract, literal -from sqlalchemy.types import DateTime, Integer +from sqlalchemy.types import DateTime, Integer, Text from app import db from app.dao.dao_utils import autocommit @@ -14,6 +15,9 @@ NotificationAllTimeView, Service, Template, + TemplateFolder, + User, + template_folder_map, ) from app.utils import ( get_midnight_in_utc, @@ -126,36 +130,47 @@ def fetch_notification_status_for_service_for_day(fetch_day, service_id): def fetch_notification_status_for_service_for_today_and_7_previous_days( - service_id, by_template=False, limit_days=7 -): + service_id: str, by_template: bool = False, limit_days: int = 7 +) -> list[dict | None]: start_date = midnight_n_days_ago(limit_days) - now = utc_now() - stats_for_7_days = db.session.query( - FactNotificationStatus.notification_type.cast(db.Text).label( - "notification_type" - ), - FactNotificationStatus.notification_status.cast(db.Text).label("status"), + now = get_midnight_in_utc(utc_now()) + + # Query for the last 7 days + stats_for_7_days = select( + cast(FactNotificationStatus.notification_type, Text).label("notification_type"), + cast(FactNotificationStatus.notification_status, Text).label("status"), *( - [FactNotificationStatus.template_id.label("template_id")] + [ + FactNotificationStatus.template_id.label("template_id"), + FactNotificationStatus.local_date.label("date_used"), + ] if by_template else [] ), FactNotificationStatus.notification_count.label("count"), - ).filter( + ).where( FactNotificationStatus.service_id == service_id, FactNotificationStatus.local_date >= start_date, FactNotificationStatus.key_type != KeyType.TEST, ) + # Query for today's stats stats_for_today = ( - db.session.query( - Notification.notification_type.cast(db.Text), - Notification.status.cast(db.Text), - *([Notification.template_id] if by_template else []), + select( + cast(Notification.notification_type, Text), + cast(Notification.status, Text), + *( + [ + Notification.template_id, + literal(now).label("date_used"), + ] + if by_template + else [] + ), func.count().label("count"), ) - .filter( - Notification.created_at >= get_midnight_in_utc(now), + .where( + Notification.created_at >= now, Notification.service_id == service_id, Notification.key_type != KeyType.TEST, ) @@ -166,31 +181,67 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days( ) ) - all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery() + # Combine the queries using union_all + all_stats_union = union_all(stats_for_7_days, stats_for_today).subquery() + all_stats_alias = aliased(all_stats_union, name="all_stats") - query = db.session.query( + # Final query with optional template joins + query = select( *( [ + TemplateFolder.name.label("folder"), Template.name.label("template_name"), - False, # TODO: this is related to is_precompiled_letter - all_stats_table.c.template_id, + False, # TODO: Handle `is_precompiled_letter` + template_folder_map.c.template_folder_id, + all_stats_alias.c.template_id, + User.name.label("created_by"), + Template.created_by_id, + func.max(all_stats_alias.c.date_used).label( + "last_used" + ), # Get the most recent date ] if by_template else [] ), - all_stats_table.c.notification_type, - all_stats_table.c.status, - func.cast(func.sum(all_stats_table.c.count), Integer).label("count"), + all_stats_alias.c.notification_type, + all_stats_alias.c.status, + cast(func.sum(all_stats_alias.c.count), Integer).label("count"), ) if by_template: - query = query.filter(all_stats_table.c.template_id == Template.id) + query = ( + query.join(Template, all_stats_alias.c.template_id == Template.id) + .join(User, Template.created_by_id == User.id) + .outerjoin( + template_folder_map, Template.id == template_folder_map.c.template_id + ) + .outerjoin( + TemplateFolder, + TemplateFolder.id == template_folder_map.c.template_folder_id, + ) + ) + + # Group by all necessary fields except date_used + query = query.group_by( + *( + [ + TemplateFolder.name, + Template.name, + all_stats_alias.c.template_id, + User.name, + template_folder_map.c.template_folder_id, + Template.created_by_id, + ] + if by_template + else [] + ), + all_stats_alias.c.notification_type, + all_stats_alias.c.status, + ) - return 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() + # Execute the query using Flask-SQLAlchemy's session + result = db.session.execute(query) + return result.mappings().all() def fetch_notification_status_totals_for_all_services(start_date, end_date): diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 03a319ecc..cf4482caa 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -23,7 +23,7 @@ def get_template_statistics_for_service_by_day(service_id): try: whole_days = int(whole_days) except ValueError: - error = "{} is not an integer".format(whole_days) + error = f"{whole_days} is not an integer" message = {"whole_days": [error]} raise InvalidRequest(message, status_code=400) @@ -41,6 +41,11 @@ def get_template_statistics_for_service_by_day(service_id): "count": row.count, "template_id": str(row.template_id), "template_name": row.template_name, + "template_folder_id": row.template_folder_id, + "template_folder": row.folder, + "created_by_id": row.created_by_id, + "created_by": row.created_by, + "last_used": row.last_used, "template_type": row.notification_type, "status": row.status, } diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index dc46de45d..ccb0c5a06 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -1,5 +1,4 @@ from datetime import date, datetime, timedelta -from unittest import mock from uuid import UUID import pytest @@ -26,6 +25,7 @@ create_notification, create_service, create_template, + create_template_folder, ) @@ -252,15 +252,18 @@ def test_fetch_notification_status_by_template_for_service_for_today_and_7_previ notify_db_session, ): service_1 = create_service(service_name="service_1") + test_folder = create_template_folder(service=service_1, name="Test_Folder_For_This") sms_template = create_template( template_name="sms Template 1", service=service_1, template_type=TemplateType.SMS, + folder=test_folder, ) sms_template_2 = create_template( template_name="sms Template 2", service=service_1, template_type=TemplateType.SMS, + folder=test_folder, ) email_template = create_template( service=service_1, template_type=TemplateType.EMAIL @@ -329,82 +332,152 @@ def test_fetch_notification_status_by_template_for_service_for_today_and_7_previ by_template=True, ) - assert [ - ( - "email Template Name", - False, - mock.ANY, - NotificationType.EMAIL, - NotificationStatus.DELIVERED, - 1, - ), - ( - "email Template Name", - False, - mock.ANY, - NotificationType.EMAIL, - NotificationStatus.DELIVERED, - 3, - ), - ( - "sms Template 1", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.CREATED, - 1, - ), - ( - "sms Template Name", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.CREATED, - 1, - ), - ( - "sms Template 1", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.DELIVERED, - 1, - ), - ( - "sms Template 2", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.DELIVERED, - 1, - ), - ( - "sms Template Name", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.DELIVERED, - 8, - ), - ( - "sms Template Name", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.DELIVERED, - 10, - ), - ( - "sms Template Name", - False, - mock.ANY, - NotificationType.SMS, - NotificationStatus.DELIVERED, - 11, - ), - ] == sorted( - results, key=lambda x: (x.notification_type, x.status, x.template_name, x.count) - ) + expected = [ + { + "folder": None, + "template_name": "email Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 31, 0, 0), + "notification_type": NotificationType.EMAIL, + "status": NotificationStatus.DELIVERED, + "count": 1, + }, + { + "folder": None, + "template_name": "email Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 29, 0, 0), + "notification_type": NotificationType.EMAIL, + "status": NotificationStatus.DELIVERED, + "count": 3, + }, + { + "folder": None, + "template_name": "sms Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 29, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.CREATED, + "count": 1, + }, + { + "folder": "Test_Folder_For_This", + "template_name": "sms Template 1", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 31, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.CREATED, + "count": 1, + }, + { + "folder": None, + "template_name": "sms Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 29, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.DELIVERED, + "count": 10, + }, + { + "folder": "Test_Folder_For_This", + "template_name": "sms Template 2", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 31, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.DELIVERED, + "count": 1, + }, + { + "folder": None, + "template_name": "sms Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 25, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.DELIVERED, + "count": 8, + }, + { + "folder": "Test_Folder_For_This", + "template_name": "sms Template 1", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 31, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.DELIVERED, + "count": 1, + }, + { + "folder": None, + "template_name": "sms Template Name", + "_no_label": False, + "created_by": "Test User", + "last_used": datetime(2018, 10, 29, 0, 0), + "notification_type": NotificationType.SMS, + "status": NotificationStatus.DELIVERED, + "count": 11, + }, + ] + + expected = [ + [ + str(row[k]) if k != "last_used" else row[k].strftime("%Y-%m-%d") + for k in ( + "folder", + "template_name", + "created_by", + "last_used", + "notification_type", + "status", + "count", + ) + ] + for row in sorted( + expected, + key=lambda x: ( + str(x["notification_type"]), + str(x["status"]), + x["folder"] if x["folder"] is not None else "", + x["template_name"], + x["count"], + x["last_used"], + ), + ) + ] + + results = [ + [ + str(row[k]) if k != "last_used" else row[k].strftime("%Y-%m-%d") + for k in ( + "folder", + "template_name", + "created_by", + "last_used", + "notification_type", + "status", + "count", + ) + ] + for row in sorted( + results, + key=lambda x: ( + x.notification_type, + x.status, + x.folder if x.folder is not None else "", + x.template_name, + x.count, + x.last_used, + ), + ) + ] + + assert expected == results @pytest.mark.parametrize( diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 1ae65b22e..958a2df6c 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -79,6 +79,11 @@ def test_get_template_statistics_for_service_by_day_goes_to_db( template_name=sample_template.name, notification_type=sample_template.template_type, status=NotificationStatus.CREATED, + template_folder_id="123456", + folder="Some_Folder", + created_by_id="987654", + created_by="Mr. Nobody", + last_used="0/0/0", ) ], ) @@ -95,6 +100,11 @@ def test_get_template_statistics_for_service_by_day_goes_to_db( "template_name": sample_template.name, "template_type": sample_template.template_type, "status": NotificationStatus.CREATED, + "template_folder_id": "123456", + "template_folder": "Some_Folder", + "created_by_id": "987654", + "created_by": "Mr. Nobody", + "last_used": "0/0/0", } ] # dao only called for 2nd, since redis returned values for first call