From 9df498106f8b6be03f7a6a93ff8c22d84ab6ec8d Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 29 Aug 2017 16:26:55 +0100 Subject: [PATCH 1/7] Add one_off argument to create_notification test function One off messages have no API key or job ID. If one_off is set to False, an API key will automatically be added. --- tests/app/db.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/app/db.py b/tests/app/db.py index 8283428c9..e61270b81 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -123,7 +123,8 @@ def create_notification( international=False, phone_prefix=None, scheduled_for=None, - normalised_to=None + normalised_to=None, + one_off=False, ): if created_at is None: created_at = datetime.utcnow() @@ -132,7 +133,7 @@ def create_notification( sent_at = sent_at or datetime.utcnow() updated_at = updated_at or datetime.utcnow() - if job is None and api_key is None: + if not one_off and (job is None and api_key is None): # we didn't specify in test - lets create it api_key = ApiKey.query.filter(ApiKey.service == template.service, ApiKey.key_type == key_type).first() if not api_key: From 132d65bc75c69d44f38f44750f544823d63112e6 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 29 Aug 2017 16:35:30 +0100 Subject: [PATCH 2/7] Add query to get processing time stats for performance platform We are only interested in API notifications, not including test messages. Letters are not included. --- app/dao/notifications_dao.py | 36 ++++++++ tests/app/dao/notification_dao/__init__.py | 0 .../test_notification_dao.py | 0 ...t_notification_dao_performance_platform.py | 87 +++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 tests/app/dao/notification_dao/__init__.py rename tests/app/dao/{ => notification_dao}/test_notification_dao.py (100%) create mode 100644 tests/app/dao/notification_dao/test_notification_dao_performance_platform.py diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4514d4310..dca73188a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -16,6 +16,8 @@ from notifications_utils.recipients import ( from werkzeug.datastructures import MultiDict from sqlalchemy import (desc, func, or_, and_, asc) from sqlalchemy.orm import joinedload +from sqlalchemy.sql.expression import case +from sqlalchemy.sql import functions from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES from app import db, create_uuid @@ -519,3 +521,37 @@ def set_scheduled_notification_to_processed(notification_id): {'pending': False} ) db.session.commit() + + +def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, end_date): + """ + SELECT + count(notifications), + sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END) + FROM notifications + WHERE + created_at > 'START DATE' AND + created_at < 'END DATE' AND + api_key_id IS NOT NULL AND + key_type != 'test' AND + notification_type != 'letter'; + """ + under_10_secs = Notification.sent_at - Notification.created_at <= timedelta(seconds=10) + sum_column = functions.sum( + case( + [ + (under_10_secs, 1) + ], + else_=0 + ) + ) + return db.session.query( + func.count(Notification.id).label('messages_total'), + sum_column.label('messages_within_10_secs') + ).filter( + Notification.created_at >= start_date, + Notification.created_at < end_date, + Notification.api_key_id.isnot(None), + Notification.key_type != KEY_TYPE_TEST, + Notification.notification_type != LETTER_TYPE + ).one() diff --git a/tests/app/dao/notification_dao/__init__.py b/tests/app/dao/notification_dao/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py similarity index 100% rename from tests/app/dao/test_notification_dao.py rename to tests/app/dao/notification_dao/test_notification_dao.py diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py new file mode 100644 index 000000000..d3fc254e8 --- /dev/null +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -0,0 +1,87 @@ +from datetime import date, datetime, timedelta + +from freezegun import freeze_time + +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_perfomance_platform +from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST + +from tests.app.db import create_notification + + +def test_get_total_notifications_filters_on_date(sample_template): + create_notification(sample_template, created_at=datetime(2016, 10, 17, 10, 0)) + create_notification(sample_template, created_at=datetime(2016, 10, 18, 10, 0)) + create_notification(sample_template, created_at=datetime(2016, 10, 19, 10, 0)) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 + + +def test_get_total_notifications_filters_on_date_at_midnight(sample_template): + create_notification(sample_template, created_at=datetime(2016, 10, 17, 23, 59, 59)) + create_notification(sample_template, created_at=datetime(2016, 10, 18, 0, 0)) + create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) + create_notification(sample_template, created_at=datetime(2016, 10, 19, 0, 0)) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 2 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_only_counts_api_notifications(sample_template, sample_job, sample_api_key): + create_notification(sample_template, one_off=True) + create_notification(sample_template, one_off=True) + create_notification(sample_template, job=sample_job) + create_notification(sample_template, job=sample_job) + create_notification(sample_template, api_key=sample_api_key) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_ignores_test_keys(sample_template): + # Creating multiple templates with normal and team keys but only 1 template + # with a test key to test that the count ignores letters + create_notification(sample_template, key_type=KEY_TYPE_NORMAL) + create_notification(sample_template, key_type=KEY_TYPE_NORMAL) + create_notification(sample_template, key_type=KEY_TYPE_TEAM) + create_notification(sample_template, key_type=KEY_TYPE_TEAM) + create_notification(sample_template, key_type=KEY_TYPE_TEST) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 4 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_ignores_letters( + sample_template, + sample_email_template, + sample_letter_template +): + # Creating multiple sms and email templates but only 1 letter template to + # test that the count ignores letters + create_notification(sample_template) + create_notification(sample_template) + create_notification(sample_email_template) + create_notification(sample_email_template) + create_notification(sample_letter_template) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 4 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_messages_within_10_seconds(sample_template): + created_at = datetime.utcnow() + + create_notification(sample_template, sent_at=created_at + timedelta(seconds=5)) + create_notification(sample_template, sent_at=created_at + timedelta(seconds=10)) + create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) + + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 3 + assert result.messages_within_10_secs == 2 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): + create_notification(sample_template, status='created', sent_at=None) + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 + assert result.messages_within_10_secs == 0 From a1a5fdedb117abd0234d1330e60817e955d673f0 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 30 Aug 2017 14:36:16 +0100 Subject: [PATCH 3/7] Send results of processing-time query to performance platform --- app/celery/scheduled_tasks.py | 43 +++++++++-------- app/performance_platform/processing_time.py | 36 ++++++++++++++ tests/app/celery/test_scheduled_tasks.py | 7 +-- .../test_processing_time.py | 47 +++++++++++++++++++ 4 files changed, 111 insertions(+), 22 deletions(-) create mode 100644 app/performance_platform/processing_time.py create mode 100644 tests/app/performance_platform/test_processing_time.py diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 5614c361f..bf10ceec7 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery -from app.performance_platform import total_sent_notifications +from app.performance_platform import total_sent_notifications, processing_time from app import performance_platform_client from app.dao.date_util import get_month_start_and_end_date_in_utc from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago @@ -176,27 +176,32 @@ def timeout_notifications(): @statsd(namespace="tasks") def send_daily_performance_platform_stats(): if performance_platform_client.active: - count_dict = total_sent_notifications.get_total_sent_notifications_yesterday() - email_sent_count = count_dict.get('email').get('count') - sms_sent_count = count_dict.get('sms').get('count') - start_date = count_dict.get('start_date') + send_total_sent_notifications_to_performance_platform() + processing_time.send_processing_time_to_performance_platform() - current_app.logger.info( - "Attempting to update performance platform for date {} with email count {} and sms count {}" - .format(start_date, email_sent_count, sms_sent_count) - ) - total_sent_notifications.send_total_notifications_sent_for_day_stats( - start_date, - 'sms', - sms_sent_count - ) +def send_total_sent_notifications_to_performance_platform(): + count_dict = total_sent_notifications.get_total_sent_notifications_yesterday() + email_sent_count = count_dict.get('email').get('count') + sms_sent_count = count_dict.get('sms').get('count') + start_date = count_dict.get('start_date') - total_sent_notifications.send_total_notifications_sent_for_day_stats( - start_date, - 'email', - email_sent_count - ) + current_app.logger.info( + "Attempting to update performance platform for date {} with email count {} and sms count {}" + .format(start_date, email_sent_count, sms_sent_count) + ) + + total_sent_notifications.send_total_notifications_sent_for_day_stats( + start_date, + 'sms', + sms_sent_count + ) + + total_sent_notifications.send_total_notifications_sent_for_day_stats( + start_date, + 'email', + email_sent_count + ) @notify_celery.task(name='switch-current-sms-provider-on-slow-delivery') diff --git a/app/performance_platform/processing_time.py b/app/performance_platform/processing_time.py new file mode 100644 index 000000000..e9811a939 --- /dev/null +++ b/app/performance_platform/processing_time.py @@ -0,0 +1,36 @@ +from datetime import datetime + +from flask import current_app + +from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_perfomance_platform +from app import performance_platform_client + + +def send_processing_time_to_performance_platform(): + today = datetime.utcnow() + start_date = get_midnight_for_day_before(today) + end_date = get_london_midnight_in_utc(today) + + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, end_date) + + current_app.logger.info( + 'Sending processing-time to performance platform for date {}. Total: {}, under 10 secs {}'.format( + start_date, result.messages_total, result.messages_within_10_secs + ) + ) + + send_processing_time_data(start_date, 'messages-total', result.messages_total) + send_processing_time_data(start_date, 'messages-within-10-secs', result.messages_within_10_secs) + + +def send_processing_time_data(date, status, count): + payload = performance_platform_client.format_payload( + dataset='processing-time', + date=date, + group_name='status', + group_value=status, + count=count + ) + + performance_platform_client.send_stats_to_performance_platform(payload) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index a05ea18f9..c77785e53 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -29,7 +29,8 @@ from app.celery.scheduled_tasks import ( switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, timeout_notifications, - populate_monthly_billing) + populate_monthly_billing, + send_total_sent_notifications_to_performance_platform) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.config import QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id @@ -285,7 +286,7 @@ 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_daily_performance_stats_calls_with_correct_totals( +def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals( notify_db, notify_db_session, sample_template, @@ -319,7 +320,7 @@ def test_send_daily_performance_stats_calls_with_correct_totals( new_callable=PropertyMock ) as mock_active: mock_active.return_value = True - send_daily_performance_platform_stats() + send_total_sent_notifications_to_performance_platform() perf_mock.assert_has_calls([ call(get_london_midnight_in_utc(yesterday), 'sms', 2), diff --git a/tests/app/performance_platform/test_processing_time.py b/tests/app/performance_platform/test_processing_time.py new file mode 100644 index 000000000..07f78859e --- /dev/null +++ b/tests/app/performance_platform/test_processing_time.py @@ -0,0 +1,47 @@ +from datetime import datetime, timedelta + +from freezegun import freeze_time + +from tests.app.db import create_notification +from app.performance_platform.processing_time import ( + send_processing_time_to_performance_platform, + send_processing_time_data +) + + +@freeze_time('2016-10-18T02:00') +def test_send_processing_time_to_performance_platform_generates_correct_calls(mocker, sample_template): + send_mock = mocker.patch('app.performance_platform.processing_time.send_processing_time_data') + + created_at = datetime.utcnow() - timedelta(days=1) + + create_notification(sample_template, created_at=created_at, sent_at=created_at + timedelta(seconds=5)) + create_notification(sample_template, created_at=created_at, sent_at=created_at + timedelta(seconds=15)) + create_notification(sample_template, created_at=datetime.utcnow() - timedelta(days=2)) + + send_processing_time_to_performance_platform() + + send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-total', 2) + send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-within-10-secs', 1) + + +def test_send_processing_time_to_performance_platform_creates_correct_call_to_perf_platform(mocker): + send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa + + send_processing_time_data( + date=datetime(2016, 10, 15, 23, 0, 0), + status='foo', + count=142 + ) + + assert send_stats.call_count == 1 + + request_args = send_stats.call_args[0][0] + assert request_args['dataType'] == 'processing-time' + assert request_args['service'] == 'govuk-notify' + assert request_args['period'] == 'day' + assert request_args['status'] == 'foo' + assert request_args['_timestamp'] == '2016-10-16T00:00:00' + assert request_args['count'] == 142 + expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeWZvb3Byb2Nlc3NpbmctdGltZWRheQ==' + assert request_args['_id'] == expected_base64_id From 49a6bfc06bf9f31e5142adf7befbebc1135c9317 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 30 Aug 2017 16:02:30 +0100 Subject: [PATCH 4/7] Send '0', not 'null', to perf platform if no notifications are sent --- app/dao/notifications_dao.py | 6 +++--- .../test_notification_dao_performance_platform.py | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index dca73188a..c1b22301c 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -527,7 +527,7 @@ def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, """ SELECT count(notifications), - sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END) + coalesce(sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END), 0) FROM notifications WHERE created_at > 'START DATE' AND @@ -537,14 +537,14 @@ def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, notification_type != 'letter'; """ under_10_secs = Notification.sent_at - Notification.created_at <= timedelta(seconds=10) - sum_column = functions.sum( + sum_column = functions.coalesce(functions.sum( case( [ (under_10_secs, 1) ], else_=0 ) - ) + ), 0) return db.session.query( func.count(Notification.id).label('messages_total'), sum_column.label('messages_within_10_secs') diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py index d3fc254e8..6d3e6839a 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -85,3 +85,10 @@ def test_get_total_notifications_counts_messages_that_have_not_sent(sample_templ result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 1 assert result.messages_within_10_secs == 0 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_returns_zero_if_no_data(notify_db_session): + result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 0 + assert result.messages_within_10_secs == 0 From ea2f838510a90d4ac3b5ee50138972723a1580e6 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 31 Aug 2017 11:10:54 +0100 Subject: [PATCH 5/7] Fix typo --- app/performance_platform/processing_time.py | 4 ++-- ...st_notification_dao_performance_platform.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/performance_platform/processing_time.py b/app/performance_platform/processing_time.py index e9811a939..320c0d454 100644 --- a/app/performance_platform/processing_time.py +++ b/app/performance_platform/processing_time.py @@ -3,7 +3,7 @@ from datetime import datetime from flask import current_app from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc -from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_perfomance_platform +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_performance_platform from app import performance_platform_client @@ -12,7 +12,7 @@ def send_processing_time_to_performance_platform(): start_date = get_midnight_for_day_before(today) end_date = get_london_midnight_in_utc(today) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, end_date) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date) current_app.logger.info( 'Sending processing-time to performance platform for date {}. Total: {}, under 10 secs {}'.format( diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py index 6d3e6839a..38d7096fe 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -2,7 +2,7 @@ from datetime import date, datetime, timedelta from freezegun import freeze_time -from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_perfomance_platform +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_performance_platform from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST from tests.app.db import create_notification @@ -12,7 +12,7 @@ def test_get_total_notifications_filters_on_date(sample_template): create_notification(sample_template, created_at=datetime(2016, 10, 17, 10, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 18, 10, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 19, 10, 0)) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 1 @@ -21,7 +21,7 @@ def test_get_total_notifications_filters_on_date_at_midnight(sample_template): create_notification(sample_template, created_at=datetime(2016, 10, 18, 0, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) create_notification(sample_template, created_at=datetime(2016, 10, 19, 0, 0)) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 2 @@ -32,7 +32,7 @@ def test_get_total_notifications_only_counts_api_notifications(sample_template, create_notification(sample_template, job=sample_job) create_notification(sample_template, job=sample_job) create_notification(sample_template, api_key=sample_api_key) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 1 @@ -45,7 +45,7 @@ def test_get_total_notifications_ignores_test_keys(sample_template): create_notification(sample_template, key_type=KEY_TYPE_TEAM) create_notification(sample_template, key_type=KEY_TYPE_TEAM) create_notification(sample_template, key_type=KEY_TYPE_TEST) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 4 @@ -62,7 +62,7 @@ def test_get_total_notifications_ignores_letters( create_notification(sample_email_template) create_notification(sample_email_template) create_notification(sample_letter_template) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 4 @@ -74,7 +74,7 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa create_notification(sample_template, sent_at=created_at + timedelta(seconds=10)) create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 3 assert result.messages_within_10_secs == 2 @@ -82,13 +82,13 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa @freeze_time('2016-10-18T10:00') def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): create_notification(sample_template, status='created', sent_at=None) - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 1 assert result.messages_within_10_secs == 0 @freeze_time('2016-10-18T10:00') def test_get_total_notifications_returns_zero_if_no_data(notify_db_session): - result = dao_get_total_notifications_sent_per_day_for_perfomance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) assert result.messages_total == 0 assert result.messages_within_10_secs == 0 From 72de309b2676333ac4b46367d7fd52d9bd951d57 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 31 Aug 2017 12:44:06 +0100 Subject: [PATCH 6/7] Make perf platform processing stats query the NotificationHistory table --- app/dao/notifications_dao.py | 22 ++++----- tests/app/conftest.py | 26 ++++++++-- ...t_notification_dao_performance_platform.py | 48 +++++++++++++++++++ 3 files changed, 81 insertions(+), 15 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index c1b22301c..17e388422 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -44,7 +44,6 @@ from app.models import ( from app.dao.dao_utils import transactional from app.statsd_decorators import statsd -from app.utils import get_london_month_from_utc_column def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -523,12 +522,12 @@ def set_scheduled_notification_to_processed(notification_id): db.session.commit() -def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, end_date): +def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date): """ SELECT - count(notifications), + count(notification_history), coalesce(sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END), 0) - FROM notifications + FROM notification_history WHERE created_at > 'START DATE' AND created_at < 'END DATE' AND @@ -536,7 +535,7 @@ def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, key_type != 'test' AND notification_type != 'letter'; """ - under_10_secs = Notification.sent_at - Notification.created_at <= timedelta(seconds=10) + under_10_secs = NotificationHistory.sent_at - NotificationHistory.created_at <= timedelta(seconds=10) sum_column = functions.coalesce(functions.sum( case( [ @@ -545,13 +544,14 @@ def dao_get_total_notifications_sent_per_day_for_perfomance_platform(start_date, else_=0 ) ), 0) + return db.session.query( - func.count(Notification.id).label('messages_total'), + func.count(NotificationHistory.id).label('messages_total'), sum_column.label('messages_within_10_secs') ).filter( - Notification.created_at >= start_date, - Notification.created_at < end_date, - Notification.api_key_id.isnot(None), - Notification.key_type != KEY_TYPE_TEST, - Notification.notification_type != LETTER_TYPE + NotificationHistory.created_at >= start_date, + NotificationHistory.created_at < end_date, + NotificationHistory.api_key_id.isnot(None), + NotificationHistory.key_type != KEY_TYPE_TEST, + NotificationHistory.notification_type != LETTER_TYPE ).one() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 86437e7a1..91954dc1b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -136,24 +136,30 @@ def sample_service( restricted=False, limit=1000, email_from=None, - permissions=[SMS_TYPE, EMAIL_TYPE] + permissions=[SMS_TYPE, EMAIL_TYPE], + research_mode=None ): if user is None: user = create_user() if email_from is None: email_from = service_name.lower().replace(' ', '.') + data = { 'name': service_name, 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, 'created_by': user, - 'letter_contact_block': 'London,\nSW1A 1AA', + 'letter_contact_block': 'London,\nSW1A 1AA' } service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) dao_create_service(service, user, service_permissions=permissions) + + if research_mode: + service.research_mode = research_mode + else: if user not in service.users: dao_add_user_to_service(service, user) @@ -206,6 +212,7 @@ def sample_template( }) template = Template(**data) dao_create_template(template) + return template @@ -631,14 +638,22 @@ def sample_notification_history( status='created', created_at=None, notification_type=None, - key_type=KEY_TYPE_NORMAL + key_type=KEY_TYPE_NORMAL, + sent_at=None, + api_key=None ): if created_at is None: created_at = datetime.utcnow() + if sent_at is None: + sent_at = datetime.utcnow() + if notification_type is None: notification_type = sample_template.template_type + if not api_key: + api_key = create_api_key(sample_template.service, key_type=key_type) + notification_history = NotificationHistory( id=uuid.uuid4(), service=sample_template.service, @@ -647,7 +662,10 @@ def sample_notification_history( status=status, created_at=created_at, notification_type=notification_type, - key_type=key_type + key_type=key_type, + api_key=api_key, + api_key_id=api_key and api_key.id, + sent_at=sent_at ) notify_db.session.add(notification_history) notify_db.session.commit() diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py index 38d7096fe..dc11ac8e2 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -6,13 +6,20 @@ from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_f from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST from tests.app.db import create_notification +from tests.app.conftest import ( + sample_notification_history, + sample_service, + sample_template +) def test_get_total_notifications_filters_on_date(sample_template): create_notification(sample_template, created_at=datetime(2016, 10, 17, 10, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 18, 10, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 19, 10, 0)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 @@ -21,7 +28,9 @@ def test_get_total_notifications_filters_on_date_at_midnight(sample_template): create_notification(sample_template, created_at=datetime(2016, 10, 18, 0, 0)) create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) create_notification(sample_template, created_at=datetime(2016, 10, 19, 0, 0)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 2 @@ -32,7 +41,9 @@ def test_get_total_notifications_only_counts_api_notifications(sample_template, create_notification(sample_template, job=sample_job) create_notification(sample_template, job=sample_job) create_notification(sample_template, api_key=sample_api_key) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 @@ -45,7 +56,9 @@ def test_get_total_notifications_ignores_test_keys(sample_template): create_notification(sample_template, key_type=KEY_TYPE_TEAM) create_notification(sample_template, key_type=KEY_TYPE_TEAM) create_notification(sample_template, key_type=KEY_TYPE_TEST) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 4 @@ -62,7 +75,9 @@ def test_get_total_notifications_ignores_letters( create_notification(sample_email_template) create_notification(sample_email_template) create_notification(sample_letter_template) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 4 @@ -75,6 +90,7 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 3 assert result.messages_within_10_secs == 2 @@ -82,7 +98,9 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa @freeze_time('2016-10-18T10:00') def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): create_notification(sample_template, status='created', sent_at=None) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 1 assert result.messages_within_10_secs == 0 @@ -90,5 +108,35 @@ def test_get_total_notifications_counts_messages_that_have_not_sent(sample_templ @freeze_time('2016-10-18T10:00') def test_get_total_notifications_returns_zero_if_no_data(notify_db_session): result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + assert result.messages_total == 0 assert result.messages_within_10_secs == 0 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_ignores_research_mode(notify_db, notify_db_session): + created_at = datetime.utcnow() + service = sample_service(notify_db, notify_db_session, research_mode=True) + template = sample_template(notify_db, notify_db_session, service=service) + + create_notification(template, status='created', sent_at=None) + + sample_notification_history( + notify_db, + notify_db_session, + template, + notification_type='email', + sent_at=created_at + timedelta(seconds=5) + ) + sample_notification_history( + notify_db, + notify_db_session, + template, + notification_type='sms', + sent_at=created_at + timedelta(seconds=5) + ) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + + assert result.messages_total == 2 + assert result.messages_within_10_secs == 2 From 96443f5d6e11a8df7e17da86acbb335cd964c720 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 31 Aug 2017 12:50:11 +0100 Subject: [PATCH 7/7] Combine test to query notifications within date range instead --- ...t_notification_dao_performance_platform.py | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py index dc11ac8e2..125223e2a 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -12,24 +12,17 @@ from tests.app.conftest import ( sample_template ) - -def test_get_total_notifications_filters_on_date(sample_template): - create_notification(sample_template, created_at=datetime(2016, 10, 17, 10, 0)) - create_notification(sample_template, created_at=datetime(2016, 10, 18, 10, 0)) - create_notification(sample_template, created_at=datetime(2016, 10, 19, 10, 0)) - - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) - - assert result.messages_total == 1 +BEGINNING_OF_DAY = date(2016, 10, 18) +END_OF_DAY = date(2016, 10, 19) -def test_get_total_notifications_filters_on_date_at_midnight(sample_template): +def test_get_total_notifications_filters_on_date_within_date_range(sample_template): create_notification(sample_template, created_at=datetime(2016, 10, 17, 23, 59, 59)) - create_notification(sample_template, created_at=datetime(2016, 10, 18, 0, 0)) + create_notification(sample_template, created_at=BEGINNING_OF_DAY) create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) - create_notification(sample_template, created_at=datetime(2016, 10, 19, 0, 0)) + create_notification(sample_template, created_at=END_OF_DAY) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 2 @@ -42,7 +35,7 @@ def test_get_total_notifications_only_counts_api_notifications(sample_template, create_notification(sample_template, job=sample_job) create_notification(sample_template, api_key=sample_api_key) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 1 @@ -57,7 +50,7 @@ def test_get_total_notifications_ignores_test_keys(sample_template): create_notification(sample_template, key_type=KEY_TYPE_TEAM) create_notification(sample_template, key_type=KEY_TYPE_TEST) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 4 @@ -76,7 +69,7 @@ def test_get_total_notifications_ignores_letters( create_notification(sample_email_template) create_notification(sample_letter_template) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 4 @@ -89,7 +82,7 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa create_notification(sample_template, sent_at=created_at + timedelta(seconds=10)) create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 3 assert result.messages_within_10_secs == 2 @@ -99,7 +92,7 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): create_notification(sample_template, status='created', sent_at=None) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 1 assert result.messages_within_10_secs == 0 @@ -107,7 +100,7 @@ def test_get_total_notifications_counts_messages_that_have_not_sent(sample_templ @freeze_time('2016-10-18T10:00') def test_get_total_notifications_returns_zero_if_no_data(notify_db_session): - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 0 assert result.messages_within_10_secs == 0 @@ -136,7 +129,7 @@ def test_get_total_notifications_counts_ignores_research_mode(notify_db, notify_ sent_at=created_at + timedelta(seconds=5) ) - result = dao_get_total_notifications_sent_per_day_for_performance_platform(date(2016, 10, 18), date(2016, 10, 19)) + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) assert result.messages_total == 2 assert result.messages_within_10_secs == 2