diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 61b997688..c37dae011 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -39,6 +39,13 @@ def update_fact_notification_status(process_day, notification_type, service_id): start_date = get_london_midnight_in_utc(process_day) end_date = get_london_midnight_in_utc(process_day + timedelta(days=1)) + # delete any existing rows in case some no longer exist e.g. if all messages are sent + FactNotificationStatus.query.filter( + FactNotificationStatus.bst_date == process_day, + FactNotificationStatus.notification_type == notification_type, + FactNotificationStatus.service_id == service_id, + ).delete() + # query notifications or notification_history for the day, depending on their data retention service = Service.query.get(service_id) source_table = get_notification_table_to_use( @@ -71,30 +78,22 @@ def update_fact_notification_status(process_day, notification_type, service_id): source_table.status ) - stmt = insert(FactNotificationStatus.__table__).from_select( - [ - FactNotificationStatus.bst_date, - FactNotificationStatus.template_id, - FactNotificationStatus.service_id, - FactNotificationStatus.job_id, - FactNotificationStatus.notification_type, - FactNotificationStatus.key_type, - FactNotificationStatus.notification_status, - FactNotificationStatus.notification_count - ], - query + db.session.connection().execute( + insert(FactNotificationStatus.__table__).from_select( + [ + FactNotificationStatus.bst_date, + FactNotificationStatus.template_id, + FactNotificationStatus.service_id, + FactNotificationStatus.job_id, + FactNotificationStatus.notification_type, + FactNotificationStatus.key_type, + FactNotificationStatus.notification_status, + FactNotificationStatus.notification_count + ], + query + ) ) - stmt = stmt.on_conflict_do_update( - constraint="ft_notification_status_pkey", - set_={ - FactNotificationStatus.notification_count: stmt.excluded.notification_count, - FactNotificationStatus.updated_at: datetime.utcnow() - } - ) - - db.session.connection().execute(stmt) - def fetch_notification_status_for_service_by_month(start_date, end_date, service_id): return db.session.query( diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 1e83161f7..3b5f5b701 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -599,9 +599,10 @@ def test_create_nightly_notification_status_for_service_and_day(notify_db_sessio def test_create_nightly_notification_status_for_service_and_day_overwrites_old_data(notify_db_session): first_service = create_service(service_name='First Service') first_template = create_template(service=first_service) - create_notification(template=first_template, status='delivered') - process_day = date.today() + + # first run: one notification, expect one row (just one status) + notification = create_notification(template=first_template, status='sending') create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms') new_fact_data = FactNotificationStatus.query.order_by( @@ -611,8 +612,11 @@ def test_create_nightly_notification_status_for_service_and_day_overwrites_old_d assert len(new_fact_data) == 1 assert new_fact_data[0].notification_count == 1 + assert new_fact_data[0].notification_status == 'sending' - create_notification(template=first_template, status='delivered') + # second run: status changed, still expect one row (one status) + notification.status = 'delivered' + create_notification(template=first_template, status='created') create_nightly_notification_status_for_service_and_day(str(process_day), first_service.id, 'sms') updated_fact_data = FactNotificationStatus.query.order_by( @@ -620,8 +624,11 @@ def test_create_nightly_notification_status_for_service_and_day_overwrites_old_d FactNotificationStatus.notification_type ).all() - assert len(updated_fact_data) == 1 - assert updated_fact_data[0].notification_count == 2 + assert len(updated_fact_data) == 2 + assert updated_fact_data[0].notification_count == 1 + assert updated_fact_data[0].notification_status == 'created' + assert updated_fact_data[1].notification_count == 1 + assert updated_fact_data[1].notification_status == 'delivered' # the job runs at 12:30am London time. 04/01 is in BST.