Update the nightly task that send performance platform statistics to use ft_notification_status rather than notification_history.

The previous query was including all notifications regardless of notification_status. I don't think that's right, it shouldn't include things like technical-failure or validation-failed. Thoughts? I also need to remove the query that's no longer being used.
2025-12-20 23:41:17 -05:00 · 2019-03-29 14:21:05 +00:00
parent 29df5730cb
commit 1806f092f3
5 changed files with 96 additions and 57 deletions
--- a/app/dao/fact_notification_status_dao.py
+++ b/app/dao/fact_notification_status_dao.py
@@ -394,3 +394,15 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id):
    else:
        query = stats
    return query.all()
+
+
+def get_total_sent_notifications_for_day_and_type(day, notification_type):
+    result = db.session.query(
+        func.sum(FactNotificationStatus.notification_count).label('count')
+    ).filter(
+        FactNotificationStatus.notification_type == notification_type,
+        FactNotificationStatus.key_type != KEY_TYPE_TEST,
+        FactNotificationStatus.bst_date == day,
+    ).scalar()
+
+    return result or 0
--- a/app/performance_platform/total_sent_notifications.py
+++ b/app/performance_platform/total_sent_notifications.py
@@ -1,8 +1,5 @@
-from datetime import timedelta
-
 from app import performance_platform_client
-from app.dao.notifications_dao import get_total_sent_notifications_in_date_range
-from app.utils import get_london_midnight_in_utc
+from app.dao.fact_notification_status_dao import get_total_sent_notifications_for_day_and_type


 def send_total_notifications_sent_for_day_stats(date, notification_type, count):
@@ -18,15 +15,12 @@ def send_total_notifications_sent_for_day_stats(date, notification_type, count):


 def get_total_sent_notifications_for_day(day):
-    start_date = get_london_midnight_in_utc(day)
-    end_date = start_date + timedelta(days=1)
-
-    email_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email')
-    sms_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms')
-    letter_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'letter')
+    email_count = get_total_sent_notifications_for_day_and_type(day, 'email')
+    sms_count = get_total_sent_notifications_for_day_and_type(day, 'sms')
+    letter_count = get_total_sent_notifications_for_day_and_type(day, 'letter')

    return {
-        "start_date": start_date,
+        "start_date": day,
        "email": {
            "count": email_count
        },
--- a/tests/app/celery/test_nightly_tasks.py
+++ b/tests/app/celery/test_nightly_tasks.py
@@ -34,7 +34,6 @@ from app.models import (
    SMS_TYPE,
    EMAIL_TYPE
 )
-from app.utils import get_london_midnight_in_utc
 from tests.app.aws.test_s3 import single_s3_object_stub
 from tests.app.db import (
    create_notification,
@@ -42,7 +41,8 @@ from tests.app.db import (
    create_template,
    create_job,
    create_service_callback_api,
-    create_service_data_retention
+    create_service_data_retention,
+    create_ft_notification_status
 )

 from tests.app.conftest import datetime_in_past
@@ -253,29 +253,26 @@ def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mo

@freeze_time("2016-01-11 12:30:00")
 def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals(
-        notify_db,
        notify_db_session,
        sample_template,
        sample_email_template,
        mocker
 ):
-    sms = sample_template
-    email = sample_email_template
-
    perf_mock = mocker.patch(
        'app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats')  # noqa

-    create_notification(email, status='delivered')
-    create_notification(sms, status='delivered')
+    today = datetime.utcnow().date()
+    create_ft_notification_status(bst_date=today, notification_type='sms', service=sample_template.service,
+                                  template=sample_template)
+    create_ft_notification_status(bst_date=today, notification_type='email', service=sample_email_template.service,
+                                  template=sample_email_template)

    # Create some notifications for the day before
-    yesterday = datetime(2016, 1, 10, 15, 30, 0, 0)
-    with freeze_time(yesterday):
-        create_notification(sms, status='delivered')
-        create_notification(sms, status='delivered')
-        create_notification(email, status='delivered')
-        create_notification(email, status='delivered')
-        create_notification(email, status='delivered')
+    yesterday = today - timedelta(days=1)
+    create_ft_notification_status(bst_date=yesterday, notification_type='sms', service=sample_template.service,
+                                  template=sample_template, count=2)
+    create_ft_notification_status(bst_date=yesterday, notification_type='email', service=sample_email_template.service,
+                                  template=sample_email_template, count=3)

    with patch.object(
            PerformancePlatformClient,
@@ -286,8 +283,8 @@ def test_send_total_sent_notifications_to_performance_platform_calls_with_correc
        send_total_sent_notifications_to_performance_platform(yesterday)

        perf_mock.assert_has_calls([
-            call(get_london_midnight_in_utc(yesterday), 'sms', 2),
-            call(get_london_midnight_in_utc(yesterday), 'email', 3)
+            call(yesterday, 'sms', 2),
+            call(yesterday, 'email', 3)
        ])


--- a/tests/app/dao/test_fact_notification_status_dao.py
+++ b/tests/app/dao/test_fact_notification_status_dao.py
@@ -12,10 +12,12 @@ from app.dao.fact_notification_status_dao import (
    fetch_notification_status_for_service_for_today_and_7_previous_days,
    fetch_notification_status_totals_for_all_services,
    fetch_notification_statuses_for_job,
-    fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service
+    fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service,
+    get_total_sent_notifications_for_day_and_type
 )
 from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
 from freezegun import freeze_time
+
 from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job


@@ -526,3 +528,48 @@ def test_fetch_monthly_template_usage_for_service_does_not_include_test_notifica
    )

    assert len(results) == 0
+
+
+@pytest.mark.parametrize("notification_type, count",
+                         [("sms", 3),
+                          ("email", 5),
+                          ("letter", 7)])
+def test_get_total_sent_notifications_for_day_and_type_returns_right_notification_type(
+        notification_type, count, sample_template, sample_email_template, sample_letter_template
+):
+    create_ft_notification_status(bst_date="2019-03-27", service=sample_template.service, template=sample_template,
+                                  count=count)
+    create_ft_notification_status(bst_date="2019-03-27", service=sample_email_template.service,
+                                  template=sample_email_template, count=count)
+    create_ft_notification_status(bst_date="2019-03-27", service=sample_letter_template.service,
+                                  template=sample_letter_template, count=count)
+
+    result = get_total_sent_notifications_for_day_and_type(day='2019-03-27', notification_type=notification_type)
+
+    assert result == count
+
+
+@pytest.mark.parametrize("day",
+                         ["2019-01-27", "2019-04-02"])
+def test_get_total_sent_notifications_for_day_and_type_returns_total_for_right_day(
+    day, sample_template
+):
+    date = datetime.strptime(day, "%Y-%m-%d")
+    create_ft_notification_status(bst_date=date - timedelta(days=1), notification_type=sample_template.template_type,
+                                  service=sample_template.service, template=sample_template, count=1)
+    create_ft_notification_status(bst_date=date, notification_type=sample_template.template_type,
+                                  service=sample_template.service, template=sample_template, count=2)
+    create_ft_notification_status(bst_date=date + timedelta(days=1), notification_type=sample_template.template_type,
+                                  service=sample_template.service, template=sample_template, count=3)
+
+    total = get_total_sent_notifications_for_day_and_type(day, sample_template.template_type)
+
+    assert total == 2
+
+
+def test_get_total_sent_notifications_for_day_and_type_returns_zero_when_no_counts(
+    notify_db_session
+):
+    total = get_total_sent_notifications_for_day_and_type("2019-03-27", "sms")
+
+    assert total == 0
--- a/tests/app/performance_platform/test_total_sent_notifications.py
+++ b/tests/app/performance_platform/test_total_sent_notifications.py
@@ -1,14 +1,11 @@
-from datetime import datetime
+from datetime import datetime, timedelta

-from freezegun import freeze_time
-
-from app.utils import get_midnight_for_day_before
 from app.performance_platform.total_sent_notifications import (
    send_total_notifications_sent_for_day_stats,
    get_total_sent_notifications_for_day
 )

-from tests.app.db import create_template, create_notification
+from tests.app.db import create_template, create_ft_notification_status


 def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(mocker, client):
@@ -33,30 +30,31 @@ def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(
    assert request_args['_id'] == expected_base64_id


-@freeze_time("2016-01-11 12:30:00")
 def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(sample_service):
    sms = create_template(sample_service, template_type='sms')
    email = create_template(sample_service, template_type='email')
    letter = create_template(sample_service, template_type='letter')

-    create_notification(email, status='delivered')
-    create_notification(sms, status='delivered')
+    today = datetime.utcnow().date()
+    yesterday = today - timedelta(days=1)
+    create_ft_notification_status(bst_date=today, notification_type='sms',
+                                  service=sms.service, template=sms)
+    create_ft_notification_status(bst_date=today, notification_type='email',
+                                  service=email.service, template=email)
+    create_ft_notification_status(bst_date=today, notification_type='letter',
+                                  service=letter.service, template=letter)

-    # Create some notifications for the day before
-    yesterday = datetime(2016, 1, 10, 15, 30, 0, 0)
-    ereyesterday = datetime(2016, 1, 9, 15, 30, 0, 0)
-    with freeze_time(yesterday):
-        create_notification(letter, status='delivered')
-        create_notification(sms, status='delivered')
-        create_notification(sms, status='delivered')
-        create_notification(email, status='delivered')
-        create_notification(email, status='delivered')
-        create_notification(email, status='delivered')
+    create_ft_notification_status(bst_date=yesterday, notification_type='sms',
+                                  service=sms.service, template=sms, count=2)
+    create_ft_notification_status(bst_date=yesterday, notification_type='email',
+                                  service=email.service, template=email, count=3)
+    create_ft_notification_status(bst_date=yesterday, notification_type='letter',
+                                  service=letter.service, template=letter, count=1)

    total_count_dict = get_total_sent_notifications_for_day(yesterday)

    assert total_count_dict == {
-        "start_date": get_midnight_for_day_before(datetime.utcnow()),
+        "start_date": yesterday,
        "email": {
            "count": 3
        },
@@ -67,12 +65,3 @@ def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(sam
            "count": 1
        }
    }
-
-    another_day = get_total_sent_notifications_for_day(ereyesterday)
-
-    assert another_day == {
-        'email': {'count': 0},
-        'letter': {'count': 0},
-        'sms': {'count': 0},
-        'start_date': datetime(2016, 1, 9, 0, 0),
-    }