diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index c880830c8..0cd744acd 100644 --- 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 diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4b9b6c7e9..5522940e9 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -453,19 +453,6 @@ def dao_timeout_notifications(timeout_period_in_seconds): return technical_failure_notifications, temporary_failure_notifications -def get_total_sent_notifications_in_date_range(start_date, end_date, notification_type): - result = db.session.query( - func.count(NotificationHistory.id).label('count') - ).filter( - NotificationHistory.key_type != KEY_TYPE_TEST, - NotificationHistory.created_at >= start_date, - NotificationHistory.created_at <= end_date, - NotificationHistory.notification_type == notification_type - ).scalar() - - return result or 0 - - def is_delivery_slow_for_provider( created_at, provider, diff --git a/app/performance_platform/total_sent_notifications.py b/app/performance_platform/total_sent_notifications.py index 14695e9e9..6dc1d039a 100644 --- 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 }, diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 179aa95ff..249e65072 100644 --- 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) ]) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 91df8ec72..e831c5d3e 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -25,7 +25,6 @@ from app.dao.notifications_dao import ( get_notification_with_personalisation, get_notifications_for_job, get_notifications_for_service, - get_total_sent_notifications_in_date_range, is_delivery_slow_for_provider, set_scheduled_notification_to_processed, update_notification_status_by_id, @@ -58,7 +57,6 @@ from tests.app.conftest import ( sample_template as create_sample_template, sample_service, sample_job, - sample_notification_history as create_notification_history, ) from tests.app.db import ( create_job, @@ -1106,116 +1104,6 @@ def test_should_exclude_test_key_notifications_by_default( assert len(all_notifications) == 1 -@pytest.mark.parametrize('notification_type', ['sms', 'email']) -def test_get_total_sent_notifications_in_date_range_returns_only_in_date_range( - notify_db, - notify_db_session, - sample_template, - notification_type -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - notification_type=notification_type, - status='delivered' - ) - - start_date = datetime(2000, 3, 30, 0, 0, 0, 0) - with freeze_time(start_date): - notification_history(created_at=start_date + timedelta(hours=3)) - notification_history(created_at=start_date + timedelta(hours=5, minutes=10)) - notification_history(created_at=start_date + timedelta(hours=11, minutes=59)) - - end_date = datetime(2000, 3, 31, 0, 0, 0, 0) - notification_history(created_at=end_date + timedelta(seconds=1)) - notification_history(created_at=end_date + timedelta(minutes=10)) - - total_count = get_total_sent_notifications_in_date_range(start_date, end_date, notification_type) - assert total_count == 3 - - -@pytest.mark.parametrize('notification_type', ['sms', 'email']) -def test_get_total_sent_notifications_in_date_range_excludes_test_key_notifications( - notify_db, - notify_db_session, - sample_template, - notification_type -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - notification_type=notification_type, - status='delivered' - ) - - start_date = datetime(2000, 3, 30, 0, 0, 0, 0) - end_date = datetime(2000, 3, 31, 0, 0, 0, 0) - with freeze_time(start_date): - notification_history(key_type=KEY_TYPE_TEAM) - notification_history(key_type=KEY_TYPE_TEAM) - notification_history(key_type=KEY_TYPE_NORMAL) - notification_history(key_type=KEY_TYPE_TEST) - - total_count = get_total_sent_notifications_in_date_range(start_date, end_date, notification_type) - assert total_count == 3 - - -def test_get_total_sent_notifications_for_sms_excludes_email_counts( - notify_db, - notify_db_session, - sample_template -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - status='delivered' - ) - - start_date = datetime(2000, 3, 30, 0, 0, 0, 0) - end_date = datetime(2000, 3, 31, 0, 0, 0, 0) - with freeze_time(start_date): - notification_history(notification_type='email') - notification_history(notification_type='email') - notification_history(notification_type='sms') - notification_history(notification_type='sms') - notification_history(notification_type='sms') - - total_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms') - assert total_count == 3 - - -def test_get_total_sent_notifications_for_email_excludes_sms_counts( - notify_db, - notify_db_session, - sample_template -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - status='delivered' - ) - - start_date = datetime(2000, 3, 30, 0, 0, 0, 0) - end_date = datetime(2000, 3, 31, 0, 0, 0, 0) - with freeze_time(start_date): - notification_history(notification_type='email') - notification_history(notification_type='email') - notification_history(notification_type='sms') - notification_history(notification_type='sms') - notification_history(notification_type='sms') - - total_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email') - assert total_count == 2 - - @pytest.mark.parametrize( "normal_sending,slow_sending,normal_delivered,slow_delivered,threshold,expected_result", [ diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 0b727a726..fecf8d696 100644 --- 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=3) + create_ft_notification_status(bst_date="2019-03-27", service=sample_email_template.service, + template=sample_email_template, count=5) + create_ft_notification_status(bst_date="2019-03-27", service=sample_letter_template.service, + template=sample_letter_template, count=7) + + 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 diff --git a/tests/app/performance_platform/test_total_sent_notifications.py b/tests/app/performance_platform/test_total_sent_notifications.py index e00631abd..0945903cc 100644 --- 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), - }