Merge pull request #3538 from alphagov/fix-out-of-date-status-182116071

Fix out-of-date rows in ft_notification_status

app/dao/notifications_dao.py

@@ -13,7 +13,7 @@ from notifications_utils.recipients import (
     validate_and_format_email_address,
 )
 from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
-from sqlalchemy import and_, asc, desc, func, or_
+from sqlalchemy import and_, asc, desc, func, or_, union
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.sql import functions
@@ -807,14 +807,26 @@ def get_service_ids_with_notifications_on_date(notification_type, date):
     start_date = get_london_midnight_in_utc(date)
     end_date = get_london_midnight_in_utc(date + timedelta(days=1))
 
+    notification_table_query = db.session.query(
+        Notification.service_id.label('service_id')
+    ).filter(
+        Notification.notification_type == notification_type,
+        # using >= + < is much more efficient than date(created_at)
+        Notification.created_at >= start_date,
+        Notification.created_at < end_date,
+    )
+
+    # Looking at this table is more efficient for historical notifications,
+    # 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(
+        FactNotificationStatus.notification_type == notification_type,
+        FactNotificationStatus.bst_date == date,
+    )
+
     return {
-        row.service_id
+        row.service_id for row in db.session.query(union(
-        for row in db.session.query(
+            notification_table_query, ft_status_table_query
-            Notification.service_id
+        ).subquery()).distinct()
-        ).filter(
-            Notification.notification_type == notification_type,
-            # using >= + < is much more efficient than date(created_at)
-            Notification.created_at >= start_date,
-            Notification.created_at < end_date,
-        ).distinct()
     }

tests/app/dao/notification_dao/test_notification_dao.py

@@ -48,6 +48,7 @@ from app.models import (
     NotificationHistory,
 )
 from tests.app.db import (
+    create_ft_notification_status,
     create_job,
     create_notification,
     create_notification_history,
@@ -1741,3 +1742,13 @@ def test_get_service_ids_with_notifications_on_date_respects_gmt_bst(
     create_notification(template=sample_template, created_at=created_at_utc)
     service_ids = get_service_ids_with_notifications_on_date(SMS_TYPE, date_to_check)
     assert len(service_ids) == expected_count
+
+
+def test_get_service_ids_with_notifications_on_date_checks_ft_status(
+    sample_template,
+):
+    create_notification(template=sample_template, created_at='2022-01-01T09:30')
+    create_ft_notification_status(template=sample_template, bst_date='2022-01-02')
+
+    assert len(get_service_ids_with_notifications_on_date(SMS_TYPE, date(2022, 1, 1))) == 1
+    assert len(get_service_ids_with_notifications_on_date(SMS_TYPE, date(2022, 1, 2))) == 1