From ed52f41039484239203ad114b287b2efd0920111 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 12:21:08 +0000 Subject: [PATCH 1/7] Add, init and config performance platform client to prepare/upload daily notification stats --- app/__init__.py | 3 +- app/clients/performance_platform/__init__.py | 0 .../performance_platform_client.py | 57 +++++++++++++++ app/config.py | 5 ++ .../app/clients/test_performance_platform.py | 71 +++++++++++++++++++ 5 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 app/clients/performance_platform/__init__.py create mode 100644 app/clients/performance_platform/performance_platform_client.py create mode 100644 tests/app/clients/test_performance_platform.py diff --git a/app/__init__.py b/app/__init__.py index a13dacd8b..e9a528072 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,6 +1,5 @@ import os import uuid -import json from flask import Flask, _request_ctx_stack from flask import request, url_for, g, jsonify @@ -18,6 +17,7 @@ from app.clients.email.aws_ses import AwsSesClient from app.clients.sms.firetext import FiretextClient from app.clients.sms.loadtesting import LoadtestingClient from app.clients.sms.mmg import MMGClient +from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.encryption import Encryption @@ -34,6 +34,7 @@ aws_ses_client = AwsSesClient() encryption = Encryption() statsd_client = StatsdClient() redis_store = RedisClient() +performance_platform_client = PerformancePlatformClient() clients = Clients() diff --git a/app/clients/performance_platform/__init__.py b/app/clients/performance_platform/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py new file mode 100644 index 000000000..f9f777a1b --- /dev/null +++ b/app/clients/performance_platform/performance_platform_client.py @@ -0,0 +1,57 @@ +import base64 +import json +from requests import request + +from flask import current_app + + +class PerformancePlatformClient: + + def init_app(self, app): + self.active = app.config.get('PERFORMANCE_PLATFORM_ENABLED') + if self.active: + self.bearer_token = app.config.get('PERFORMANCE_PLATFORM_TOKEN') + self.performance_platform_url = current_app.config.get('PERFORMANCE_PLATFORM_URL') + + def send_performance_stats(self, date, channel, count, period): + if self.active: + payload = { + '_timestamp': date, + 'service': 'govuk-notify', + 'channel': channel, + 'count': count, + 'dataType': 'notifications', + 'period': period + } + self._add_id_for_payload(payload) + self._send_stats_to_performance_platform(payload) + + def _send_stats_to_performance_platform(self, payload): + headers = { + 'Content-Type': "application/json", + 'Authorization': 'Bearer {}'.format(self.bearer_token) + } + resp = request( + "POST", + self.performance_platform_url, + data=json.dumps(payload), + headers=headers + ) + + if resp.status_code != 200: + current_app.logger.error( + "Performance platform update request failed with {} '{}'".format( + resp.status_code, + resp.json()) + ) + + def _add_id_for_payload(self, payload): + payload_string = '{}{}{}{}{}'.format( + payload['_timestamp'], + payload['service'], + payload['channel'], + payload['dataType'], + payload['period'] + ) + _id = base64.b64encode(payload_string.encode('utf-8')) + payload.update({'_id': _id.decode('utf-8')}) diff --git a/app/config.py b/app/config.py index 8d10051ef..cedaf235c 100644 --- a/app/config.py +++ b/app/config.py @@ -50,6 +50,11 @@ class Config(object): REDIS_URL = os.getenv('REDIS_URL') REDIS_ENABLED = os.getenv('REDIS_ENABLED') == '1' + # Performance platform + PERFORMANCE_PLATFORM_ENABLED = os.getenv('PERFORMANCE_PLATFORM_ENABLED') == '1' + PERFORMANCE_PLATFORM_URL = os.getenv('PERFORMANCE_PLATFORM_URL') + PERFORMANCE_PLATFORM_TOKEN = os.getenv('PERFORMANCE_PLATFORM_TOKEN') + # Logging DEBUG = False LOGGING_STDOUT_JSON = os.getenv('LOGGING_STDOUT_JSON') == '1' diff --git a/tests/app/clients/test_performance_platform.py b/tests/app/clients/test_performance_platform.py new file mode 100644 index 000000000..b6caad468 --- /dev/null +++ b/tests/app/clients/test_performance_platform.py @@ -0,0 +1,71 @@ +import requests_mock +import pytest + +from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient + + +@pytest.fixture(scope='function') +def client(mocker): + client = PerformancePlatformClient() + current_app = mocker.Mock(config={ + 'PERFORMANCE_PLATFORM_ENABLED': True, + 'PERFORMANCE_PLATFORM_URL': 'performance-platform-url', + 'PERFORMANCE_PLATFORM_TOKEN': 'token' + }) + client.init_app(current_app) + return client + + +def test_should_not_call_if_not_enabled(notify_api, client, mocker): + mocker.patch.object(client, '_send_stats_to_performance_platform') + client.active = False + client.send_performance_stats( + date='2016-10-16T00:00:00+00:00', + channel='sms', + count=142, + period='day' + ) + + client._send_stats_to_performance_platform.assert_not_called() + + +def test_should_call_if_enabled(notify_api, client, mocker): + mocker.patch.object(client, '_send_stats_to_performance_platform') + client.send_performance_stats( + date='2016-10-16T00:00:00+00:00', + channel='sms', + count=142, + period='day' + ) + + assert client._send_stats_to_performance_platform.call_count == 1 + + +def test_send_platform_stats_creates_correct_call(notify_api, client): + with requests_mock.Mocker() as request_mock: + request_mock.post( + client.performance_platform_url, + json={}, + status_code=200 + ) + client.send_performance_stats( + date='2016-10-16T00:00:00+00:00', + channel='sms', + count=142, + period='day' + ) + + assert request_mock.call_count == 1 + + assert request_mock.request_history[0].url == client.performance_platform_url + assert request_mock.request_history[0].method == 'POST' + + request_args = request_mock.request_history[0].json() + assert request_args['dataType'] == 'notifications' + assert request_args['service'] == 'govuk-notify' + assert request_args['period'] == 'day' + assert request_args['channel'] == 'sms' + assert request_args['_timestamp'] == '2016-10-16T00:00:00+00:00' + assert request_args['count'] == 142 + expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMCswMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk=' + assert request_args['_id'] == expected_base64_id From 781bef557a0907c4c046821e836b5a24623899a1 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 12:21:28 +0000 Subject: [PATCH 2/7] Add date helper methods to be used as date range filters for notification counts --- app/utils.py | 12 ++++++++++++ tests/app/test_utils.py | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/app/test_utils.py diff --git a/app/utils.py b/app/utils.py index a556ccb6e..9c5c48e4b 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,3 +1,5 @@ +from datetime import datetime, timedelta + from flask import url_for from app.models import SMS_TYPE, EMAIL_TYPE from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate @@ -26,3 +28,13 @@ def get_template_instance(template, values): return { SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: PlainTextEmailTemplate }[template['template_type']](template, values) + + +def get_midnight_for_date(date): + midnight = datetime.combine(date, datetime.min.time()) + return midnight + + +def get_midnight_for_day_before(date): + day_before = date - timedelta(1) + return get_midnight_for_date(day_before) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py new file mode 100644 index 000000000..fcb071126 --- /dev/null +++ b/tests/app/test_utils.py @@ -0,0 +1,25 @@ +from datetime import datetime +import pytest + +from app.utils import ( + get_midnight_for_date, + get_midnight_for_day_before +) + + +@pytest.mark.parametrize('date, expected_date', [ + (datetime(2016, 1, 15, 0, 30), datetime(2016, 1, 15, 0, 0)), + (datetime(2016, 1, 15, 0, 0), datetime(2016, 1, 15, 0, 0)), + (datetime(2016, 1, 15, 11, 59), datetime(2016, 1, 15, 0, 0)), +]) +def test_get_midnight_for_today_returns_expected_date(date, expected_date): + assert get_midnight_for_date(date) == expected_date + + +@pytest.mark.parametrize('date, expected_date', [ + (datetime(2016, 1, 15, 0, 30), datetime(2016, 1, 14, 0, 0)), + (datetime(2016, 1, 15, 0, 0), datetime(2016, 1, 14, 0, 0)), + (datetime(2016, 1, 15, 11, 59), datetime(2016, 1, 14, 0, 0)), +]) +def test_get_midnight_for_day_before_returns_expected_date(date, expected_date): + assert get_midnight_for_day_before(date) == expected_date From b6cc5b00ba08bebca2f979138f4c9c5133119345 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 12:22:36 +0000 Subject: [PATCH 3/7] Add dao method to get sms and email notification counts for a date range and previous day --- app/dao/notifications_dao.py | 40 ++++- tests/app/conftest.py | 21 ++- tests/app/dao/test_notification_dao.py | 195 ++++++++++++++++++++++++- 3 files changed, 245 insertions(+), 11 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 8ab6b0dd0..cac613243 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,4 +1,3 @@ - import pytz from datetime import ( datetime, @@ -23,10 +22,16 @@ from app.models import ( NOTIFICATION_PENDING, NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, - KEY_TYPE_NORMAL, KEY_TYPE_TEST) + NOTIFICATION_STATUS_TYPES_COMPLETED, + KEY_TYPE_NORMAL, KEY_TYPE_TEST +) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd +from app.utils import ( + get_midnight_for_date, + get_midnight_for_day_before +) def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -408,3 +413,34 @@ def get_april_fools(year): """ return pytz.timezone('Europe/London').localize(datetime(year, 4, 1, 0, 0, 0)).astimezone(pytz.UTC).replace( tzinfo=None) + + +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.status.in_(NOTIFICATION_STATUS_TYPES_COMPLETED), + NotificationHistory.created_at >= start_date, + NotificationHistory.created_at <= end_date, + NotificationHistory.notification_type == notification_type + ).first() + + return 0 if result is None else result.count + + +def get_total_sent_notifications_yesterday(): + today = datetime.utcnow() + start_date = get_midnight_for_day_before(today) + end_date = get_midnight_for_date(today) + + return { + "start_date": start_date, + "end_date": end_date, + "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') + } + } diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 559537744..49afa591f 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -526,14 +526,21 @@ def mock_statsd_timing(mocker): @pytest.fixture(scope='function') -def sample_notification_history(notify_db, - notify_db_session, - sample_template, - status='created', - created_at=None): +def sample_notification_history( + notify_db, + notify_db_session, + sample_template, + status='created', + created_at=None, + notification_type=None, + key_type=KEY_TYPE_NORMAL +): if created_at is None: created_at = datetime.utcnow() + if notification_type is None: + notification_type = sample_template.template_type + notification_history = NotificationHistory( id=uuid.uuid4(), service=sample_template.service, @@ -541,8 +548,8 @@ def sample_notification_history(notify_db, template_version=sample_template.version, status=status, created_at=created_at, - notification_type=sample_template.template_type, - key_type=KEY_TYPE_NORMAL + notification_type=notification_type, + key_type=key_type ) notify_db.session.add(notification_history) notify_db.session.commit() diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index a6b75196f..14865ca1c 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -35,6 +35,8 @@ 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, + get_total_sent_notifications_yesterday, update_notification_status_by_id, update_notification_status_by_reference, dao_delete_notifications_and_history_by_id, @@ -44,8 +46,20 @@ from app.dao.notifications_dao import ( from app.dao.services_dao import dao_update_service -from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service, sample_job, - sample_api_key) +from app.utils import ( + get_midnight_for_date, + get_midnight_for_day_before +) + +from tests.app.conftest import ( + sample_notification, + sample_template, + sample_email_template, + sample_service, + sample_job, + sample_api_key, + sample_notification_history as create_notification_history +) def test_should_have_decorated_notifications_dao_functions(): @@ -1306,3 +1320,180 @@ def test_get_april_fools(): april_fools = get_april_fools(2016) assert str(april_fools) == '2016-03-31 23:00:00' assert april_fools.tzinfo is None + + +@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_created_and_sending( + 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 + ) + + 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(status='sending') + notification_history(status='created') + notification_history(status='failed') + notification_history(status='delivered') + + total_count = get_total_sent_notifications_in_date_range(start_date, end_date, notification_type) + assert total_count == 2 + + +@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_sms_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 + + +@freeze_time("2016-01-11 12:30:00") +def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict( + notify_db, + notify_db_session, + sample_template +): + notification_history = partial( + create_notification_history, + notify_db, + notify_db_session, + sample_template, + status='delivered' + ) + + notification_history(notification_type='email') + notification_history(notification_type='sms') + + # Create some notifications for the day before + yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) + with freeze_time(yesterday): + notification_history(notification_type='sms') + notification_history(notification_type='sms') + notification_history(notification_type='email') + notification_history(notification_type='email') + notification_history(notification_type='email') + + total_count_dict = get_total_sent_notifications_yesterday() + + assert total_count_dict == { + "start_date": get_midnight_for_day_before(datetime.utcnow()), + "end_date": get_midnight_for_date(datetime.utcnow()), + "email": { + "count": 3 + }, + "sms": { + "count": 2 + } + } From b04c524bc3392652d87d30aea26da45924508e0f Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 12:30:56 +0000 Subject: [PATCH 4/7] Add celery task to run daily at 00:30 that sends notification counts to performance platform --- app/celery/scheduled_tasks.py | 31 +++++++- app/config.py | 5 ++ tests/app/celery/test_scheduled_tasks.py | 95 +++++++++++++++++++----- 3 files changed, 109 insertions(+), 22 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 399cc8f2f..bdcf36d8f 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -1,14 +1,18 @@ -from datetime import datetime +from datetime import date, datetime, timedelta from flask import current_app from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery +from app import performance_platform_client from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than -from app.dao.notifications_dao import (delete_notifications_created_more_than_a_week_ago, - dao_timeout_notifications) +from app.dao.notifications_dao import ( + delete_notifications_created_more_than_a_week_ago, + dao_timeout_notifications, + get_total_sent_notifications_yesterday +) from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago from app.statsd_decorators import statsd from app.celery.tasks import process_job @@ -109,3 +113,24 @@ def timeout_notifications(): if updated: current_app.logger.info( "Timeout period reached for {} notifications, status has been updated.".format(updated)) + + +@notify_celery.task(name='send-daily-performance-platform-stats') +@statsd(namespace="tasks") +def send_daily_performance_stats(): + count_dict = get_total_sent_notifications_yesterday() + start_date = count_dict.get('start_date') + + performance_platform_client.send_performance_stats( + start_date, + 'sms', + count_dict.get('sms').get('count'), + 'day' + ) + + performance_platform_client.send_performance_stats( + start_date, + 'email', + count_dict.get('email').get('count'), + 'day' + ) diff --git a/app/config.py b/app/config.py index cedaf235c..4cf41e855 100644 --- a/app/config.py +++ b/app/config.py @@ -124,6 +124,11 @@ class Config(object): 'schedule': crontab(minute=0, hour='0,1,2'), 'options': {'queue': 'periodic'} }, + 'send-daily-performance-platform-stats': { + 'task': 'send-daily-performance-platform-stats', + 'schedule': crontab(minute=30, hour=0), # 00:30 + 'options': {'queue': 'periodic'} + }, 'timeout-sending-notifications': { 'task': 'timeout-sending-notifications', 'schedule': crontab(minute=0, hour='0,1,2'), diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index f5f1a44c9..da616519c 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1,18 +1,27 @@ from datetime import datetime, timedelta +from functools import partial from flask import current_app from freezegun import freeze_time from app.celery.scheduled_tasks import s3 from app.celery import scheduled_tasks -from app.celery.scheduled_tasks import (delete_verify_codes, - remove_csv_files, - delete_successful_notifications, - delete_failed_notifications, - delete_invitations, - timeout_notifications, - run_scheduled_jobs) +from app.celery.scheduled_tasks import ( + delete_verify_codes, + remove_csv_files, + delete_successful_notifications, + delete_failed_notifications, + delete_invitations, + timeout_notifications, + run_scheduled_jobs, + send_daily_performance_stats +) from app.dao.jobs_dao import dao_get_job_by_id -from tests.app.conftest import sample_notification, sample_job +from app.utils import get_midnight_for_date +from tests.app.conftest import ( + sample_notification as create_sample_notification, + sample_job as create_sample_job, + sample_notification_history as create_notification_history +) from unittest.mock import call @@ -24,6 +33,7 @@ def test_should_have_decorated_tasks_functions(): assert delete_invitations.__wrapped__.__name__ == 'delete_invitations' assert run_scheduled_jobs.__wrapped__.__name__ == 'run_scheduled_jobs' assert remove_csv_files.__wrapped__.__name__ == 'remove_csv_files' + assert send_daily_performance_stats.__wrapped__.__name__ == 'send_daily_performance_stats' def test_should_call_delete_notifications_more_than_week_in_task(notify_api, mocker): @@ -58,7 +68,7 @@ def test_update_status_of_notifications_after_timeout(notify_api, sample_template, mmg_provider): with notify_api.test_request_context(): - not1 = sample_notification( + not1 = create_sample_notification( notify_db, notify_db_session, service=sample_service, @@ -66,7 +76,7 @@ def test_update_status_of_notifications_after_timeout(notify_api, status='sending', created_at=datetime.utcnow() - timedelta( seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - not2 = sample_notification( + not2 = create_sample_notification( notify_db, notify_db_session, service=sample_service, @@ -74,7 +84,7 @@ def test_update_status_of_notifications_after_timeout(notify_api, status='created', created_at=datetime.utcnow() - timedelta( seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - not3 = sample_notification( + not3 = create_sample_notification( notify_db, notify_db_session, service=sample_service, @@ -95,7 +105,7 @@ def test_not_update_status_of_notification_before_timeout(notify_api, sample_template, mmg_provider): with notify_api.test_request_context(): - not1 = sample_notification( + not1 = create_sample_notification( notify_db, notify_db_session, service=sample_service, @@ -111,7 +121,7 @@ def test_should_update_scheduled_jobs_and_put_on_queue(notify_db, notify_db_sess mocked = mocker.patch('app.celery.tasks.process_job.apply_async') one_minute_in_the_past = datetime.utcnow() - timedelta(minutes=1) - job = sample_job(notify_db, notify_db_session, scheduled_for=one_minute_in_the_past, job_status='scheduled') + job = create_sample_job(notify_db, notify_db_session, scheduled_for=one_minute_in_the_past, job_status='scheduled') run_scheduled_jobs() @@ -126,9 +136,24 @@ def test_should_update_all_scheduled_jobs_and_put_on_queue(notify_db, notify_db_ one_minute_in_the_past = datetime.utcnow() - timedelta(minutes=1) ten_minutes_in_the_past = datetime.utcnow() - timedelta(minutes=10) twenty_minutes_in_the_past = datetime.utcnow() - timedelta(minutes=20) - job_1 = sample_job(notify_db, notify_db_session, scheduled_for=one_minute_in_the_past, job_status='scheduled') - job_2 = sample_job(notify_db, notify_db_session, scheduled_for=ten_minutes_in_the_past, job_status='scheduled') - job_3 = sample_job(notify_db, notify_db_session, scheduled_for=twenty_minutes_in_the_past, job_status='scheduled') + job_1 = create_sample_job( + notify_db, + notify_db_session, + scheduled_for=one_minute_in_the_past, + job_status='scheduled' + ) + job_2 = create_sample_job( + notify_db, + notify_db_session, + scheduled_for=ten_minutes_in_the_past, + job_status='scheduled' + ) + job_3 = create_sample_job( + notify_db, + notify_db_session, + scheduled_for=twenty_minutes_in_the_past, + job_status='scheduled' + ) run_scheduled_jobs() @@ -150,10 +175,42 @@ def test_will_remove_csv_files_for_jobs_older_than_seven_days(notify_db, notify_ midnight = datetime(2016, 10, 10, 0, 0, 0, 0) one_millisecond_past_midnight = datetime(2016, 10, 10, 0, 0, 0, 1) - job_1 = sample_job(notify_db, notify_db_session, created_at=one_millisecond_before_midnight) - sample_job(notify_db, notify_db_session, created_at=midnight) - sample_job(notify_db, notify_db_session, created_at=one_millisecond_past_midnight) + job_1 = create_sample_job(notify_db, notify_db_session, created_at=one_millisecond_before_midnight) + create_sample_job(notify_db, notify_db_session, created_at=midnight) + create_sample_job(notify_db, notify_db_session, created_at=one_millisecond_past_midnight) with freeze_time('2016-10-17T00:00:00'): remove_csv_files() s3.remove_job_from_s3.assert_called_once_with(job_1.service_id, job_1.id) + + +@freeze_time("2016-01-11 12:30:00") +def test_send_daily_performance_stats_calls_with_correct_totals(notify_db, notify_db_session, sample_template, mocker): + perf_mock = mocker.patch('app.celery.scheduled_tasks.performance_platform_client.send_performance_stats') + + notification_history = partial( + create_notification_history, + notify_db, + notify_db_session, + sample_template, + status='delivered' + ) + + notification_history(notification_type='email') + notification_history(notification_type='sms') + + # Create some notifications for the day before + yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) + with freeze_time(yesterday): + notification_history(notification_type='sms') + notification_history(notification_type='sms') + notification_history(notification_type='email') + notification_history(notification_type='email') + notification_history(notification_type='email') + + send_daily_performance_stats() + + perf_mock.assert_has_calls([ + call(get_midnight_for_date(yesterday), 'sms', 2, 'day'), + call(get_midnight_for_date(yesterday), 'email', 3, 'day') + ]) From ac1ca608dd516ccf8869419c317910323b83699e Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 13:03:04 +0000 Subject: [PATCH 5/7] Add performance platform url directly in config (not a secret) --- app/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 4cf41e855..34a5e1c3c 100644 --- a/app/config.py +++ b/app/config.py @@ -52,7 +52,7 @@ class Config(object): # Performance platform PERFORMANCE_PLATFORM_ENABLED = os.getenv('PERFORMANCE_PLATFORM_ENABLED') == '1' - PERFORMANCE_PLATFORM_URL = os.getenv('PERFORMANCE_PLATFORM_URL') + PERFORMANCE_PLATFORM_URL = 'https://www.performance.service.gov.uk/data/govuk-notify/notifications' PERFORMANCE_PLATFORM_TOKEN = os.getenv('PERFORMANCE_PLATFORM_TOKEN') # Logging From c87d0a37f382a28a5d77f9427274b5751acb289b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 27 Jan 2017 15:57:25 +0000 Subject: [PATCH 6/7] Refactor the get_midnight functions to return the date in UTC but for the localised time. So in June when we are in BST June 16, 00:00 BST => June 15 23:00 UTC --- app/dao/notifications_dao.py | 9 ++++----- app/service/rest.py | 7 ++++--- app/utils.py | 16 ++++++++++++---- tests/app/celery/test_scheduled_tasks.py | 6 +++--- tests/app/dao/test_notification_dao.py | 5 ++--- tests/app/test_utils.py | 14 +++++++------- 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index cac613243..6fb534061 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -6,7 +6,7 @@ from datetime import ( from flask import current_app from werkzeug.datastructures import MultiDict -from sqlalchemy import (desc, func, or_, and_, asc, extract) +from sqlalchemy import (desc, func, or_, and_, asc) from sqlalchemy.orm import joinedload from app import db, create_uuid @@ -29,9 +29,8 @@ from app.models import ( from app.dao.dao_utils import transactional from app.statsd_decorators import statsd from app.utils import ( - get_midnight_for_date, - get_midnight_for_day_before -) + get_midnight_for_day_before, + get_london_midnight_in_utc) def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -432,7 +431,7 @@ def get_total_sent_notifications_in_date_range(start_date, end_date, notificatio def get_total_sent_notifications_yesterday(): today = datetime.utcnow() start_date = get_midnight_for_day_before(today) - end_date = get_midnight_for_date(today) + end_date = get_london_midnight_in_utc(today) return { "start_date": start_date, diff --git a/app/service/rest.py b/app/service/rest.py index cdca5045b..070c2b983 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -53,7 +53,7 @@ from app.schemas import ( notifications_filter_schema, detailed_service_schema ) -from app.utils import pagination_links +from app.utils import pagination_links, get_london_midnight_in_utc service_blueprint = Blueprint('service', __name__) register_errors(service_blueprint) @@ -281,8 +281,9 @@ def get_detailed_services(start_date, end_date, only_active=False, include_from_ if start_date == datetime.utcnow().date(): stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key) else: - stats = fetch_stats_by_date_range_for_all_services(start_date=start_date, - end_date=end_date, + + stats = fetch_stats_by_date_range_for_all_services(start_date=get_london_midnight_in_utc(start_date), + end_date=get_london_midnight_in_utc(end_date), include_from_test_key=include_from_test_key) for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): diff --git a/app/utils.py b/app/utils.py index 9c5c48e4b..8b8eb86ba 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,5 +1,6 @@ from datetime import datetime, timedelta +import pytz from flask import url_for from app.models import SMS_TYPE, EMAIL_TYPE from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate @@ -30,11 +31,18 @@ def get_template_instance(template, values): }[template['template_type']](template, values) -def get_midnight_for_date(date): - midnight = datetime.combine(date, datetime.min.time()) - return midnight +def get_london_midnight_in_utc(date): + """ + This function converts date to midnight as BST (British Standard Time) to UTC, + the tzinfo is lastly removed from the datetime because the database stores the timestamps without timezone. + :param date: the day to calculate the London midnight in UTC for + :return: the datetime of London midnight in UTC, for example 2016-06-17 = 2016-06-17 23:00:00 + """ + return pytz.timezone('Europe/London').localize(datetime.combine(date, datetime.min.time())).astimezone( + pytz.UTC).replace( + tzinfo=None) def get_midnight_for_day_before(date): day_before = date - timedelta(1) - return get_midnight_for_date(day_before) + return get_london_midnight_in_utc(day_before) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index da616519c..4d903b069 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -16,7 +16,7 @@ from app.celery.scheduled_tasks import ( send_daily_performance_stats ) from app.dao.jobs_dao import dao_get_job_by_id -from app.utils import get_midnight_for_date +from app.utils import get_london_midnight_in_utc from tests.app.conftest import ( sample_notification as create_sample_notification, sample_job as create_sample_job, @@ -211,6 +211,6 @@ def test_send_daily_performance_stats_calls_with_correct_totals(notify_db, notif send_daily_performance_stats() perf_mock.assert_has_calls([ - call(get_midnight_for_date(yesterday), 'sms', 2, 'day'), - call(get_midnight_for_date(yesterday), 'email', 3, 'day') + call(get_london_midnight_in_utc(yesterday), 'sms', 2, 'day'), + call(get_london_midnight_in_utc(yesterday), 'email', 3, 'day') ]) diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 14865ca1c..ba8dfd8e8 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -1,5 +1,4 @@ from datetime import datetime, timedelta, date -import pytz import uuid from functools import partial @@ -47,7 +46,7 @@ from app.dao.notifications_dao import ( from app.dao.services_dao import dao_update_service from app.utils import ( - get_midnight_for_date, + get_london_midnight_in_utc, get_midnight_for_day_before ) @@ -1489,7 +1488,7 @@ def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict( assert total_count_dict == { "start_date": get_midnight_for_day_before(datetime.utcnow()), - "end_date": get_midnight_for_date(datetime.utcnow()), + "end_date": get_london_midnight_in_utc(datetime.utcnow()), "email": { "count": 3 }, diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index fcb071126..8e34978f8 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -2,24 +2,24 @@ from datetime import datetime import pytest from app.utils import ( - get_midnight_for_date, + get_london_midnight_in_utc, get_midnight_for_day_before ) @pytest.mark.parametrize('date, expected_date', [ (datetime(2016, 1, 15, 0, 30), datetime(2016, 1, 15, 0, 0)), - (datetime(2016, 1, 15, 0, 0), datetime(2016, 1, 15, 0, 0)), - (datetime(2016, 1, 15, 11, 59), datetime(2016, 1, 15, 0, 0)), + (datetime(2016, 6, 15, 0, 0), datetime(2016, 6, 14, 23, 0)), + (datetime(2016, 9, 15, 11, 59), datetime(2016, 9, 14, 23, 0)), ]) -def test_get_midnight_for_today_returns_expected_date(date, expected_date): - assert get_midnight_for_date(date) == expected_date +def test_get_london_midnight_in_utc_returns_expected_date(date, expected_date): + assert get_london_midnight_in_utc(date) == expected_date @pytest.mark.parametrize('date, expected_date', [ (datetime(2016, 1, 15, 0, 30), datetime(2016, 1, 14, 0, 0)), - (datetime(2016, 1, 15, 0, 0), datetime(2016, 1, 14, 0, 0)), - (datetime(2016, 1, 15, 11, 59), datetime(2016, 1, 14, 0, 0)), + (datetime(2016, 7, 15, 0, 0), datetime(2016, 7, 13, 23, 0)), + (datetime(2016, 8, 23, 11, 59), datetime(2016, 8, 21, 23, 0)), ]) def test_get_midnight_for_day_before_returns_expected_date(date, expected_date): assert get_midnight_for_day_before(date) == expected_date From cc7bf45766c43a3f10387be084b24f4230f66956 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 27 Jan 2017 16:39:01 +0000 Subject: [PATCH 7/7] Move notification retrieval logic into perf platform client and dont filter by status --- app/celery/scheduled_tasks.py | 7 +- .../performance_platform_client.py | 23 ++++++ app/dao/notifications_dao.py | 26 +------ app/utils.py | 2 +- .../app/clients/test_performance_platform.py | 49 ++++++++++++ tests/app/dao/test_notification_dao.py | 76 +------------------ 6 files changed, 79 insertions(+), 104 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index bdcf36d8f..cfed62ea5 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -1,4 +1,4 @@ -from datetime import date, datetime, timedelta +from datetime import datetime from flask import current_app from sqlalchemy.exc import SQLAlchemyError @@ -10,8 +10,7 @@ from app.dao.invited_user_dao import delete_invitations_created_more_than_two_da from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than from app.dao.notifications_dao import ( delete_notifications_created_more_than_a_week_ago, - dao_timeout_notifications, - get_total_sent_notifications_yesterday + dao_timeout_notifications ) from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago from app.statsd_decorators import statsd @@ -118,7 +117,7 @@ def timeout_notifications(): @notify_celery.task(name='send-daily-performance-platform-stats') @statsd(namespace="tasks") def send_daily_performance_stats(): - count_dict = get_total_sent_notifications_yesterday() + count_dict = performance_platform_client.get_total_sent_notifications_yesterday() start_date = count_dict.get('start_date') performance_platform_client.send_performance_stats( diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py index f9f777a1b..5ad2de7c6 100644 --- a/app/clients/performance_platform/performance_platform_client.py +++ b/app/clients/performance_platform/performance_platform_client.py @@ -1,9 +1,15 @@ import base64 import json +from datetime import datetime from requests import request from flask import current_app +from app.utils import ( + get_midnight_for_day_before, + get_london_midnight_in_utc +) + class PerformancePlatformClient: @@ -26,6 +32,23 @@ class PerformancePlatformClient: self._add_id_for_payload(payload) self._send_stats_to_performance_platform(payload) + def get_total_sent_notifications_yesterday(self): + today = datetime.utcnow() + start_date = get_midnight_for_day_before(today) + end_date = get_london_midnight_in_utc(today) + + from app.dao.notifications_dao import get_total_sent_notifications_in_date_range + return { + "start_date": start_date, + "end_date": end_date, + "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') + } + } + def _send_stats_to_performance_platform(self, payload): headers = { 'Content-Type': "application/json", diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6fb534061..3dff3f390 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -22,15 +22,11 @@ from app.models import ( NOTIFICATION_PENDING, NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, - NOTIFICATION_STATUS_TYPES_COMPLETED, KEY_TYPE_NORMAL, KEY_TYPE_TEST ) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd -from app.utils import ( - get_midnight_for_day_before, - get_london_midnight_in_utc) def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -419,27 +415,9 @@ def get_total_sent_notifications_in_date_range(start_date, end_date, notificatio func.count(NotificationHistory.id).label('count') ).filter( NotificationHistory.key_type != KEY_TYPE_TEST, - NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_COMPLETED), NotificationHistory.created_at >= start_date, NotificationHistory.created_at <= end_date, NotificationHistory.notification_type == notification_type - ).first() + ).scalar() - return 0 if result is None else result.count - - -def get_total_sent_notifications_yesterday(): - today = datetime.utcnow() - start_date = get_midnight_for_day_before(today) - end_date = get_london_midnight_in_utc(today) - - return { - "start_date": start_date, - "end_date": end_date, - "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') - } - } + return result or 0 diff --git a/app/utils.py b/app/utils.py index 8b8eb86ba..e8430b937 100644 --- a/app/utils.py +++ b/app/utils.py @@ -2,7 +2,6 @@ from datetime import datetime, timedelta import pytz from flask import url_for -from app.models import SMS_TYPE, EMAIL_TYPE from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate @@ -26,6 +25,7 @@ def url_with_token(data, url, config): def get_template_instance(template, values): + from app.models import SMS_TYPE, EMAIL_TYPE return { SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: PlainTextEmailTemplate }[template['template_type']](template, values) diff --git a/tests/app/clients/test_performance_platform.py b/tests/app/clients/test_performance_platform.py index b6caad468..f72a9ce33 100644 --- a/tests/app/clients/test_performance_platform.py +++ b/tests/app/clients/test_performance_platform.py @@ -1,7 +1,15 @@ import requests_mock import pytest +from datetime import datetime +from freezegun import freeze_time +from functools import partial from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient +from app.utils import ( + get_london_midnight_in_utc, + get_midnight_for_day_before +) +from tests.app.conftest import sample_notification_history as create_notification_history @pytest.fixture(scope='function') @@ -69,3 +77,44 @@ def test_send_platform_stats_creates_correct_call(notify_api, client): assert request_args['count'] == 142 expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMCswMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk=' 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( + notify_db, + notify_db_session, + client, + sample_template +): + notification_history = partial( + create_notification_history, + notify_db, + notify_db_session, + sample_template, + status='delivered' + ) + + notification_history(notification_type='email') + notification_history(notification_type='sms') + + # Create some notifications for the day before + yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) + with freeze_time(yesterday): + notification_history(notification_type='sms') + notification_history(notification_type='sms') + notification_history(notification_type='email') + notification_history(notification_type='email') + notification_history(notification_type='email') + + total_count_dict = client.get_total_sent_notifications_yesterday() + + assert total_count_dict == { + "start_date": get_midnight_for_day_before(datetime.utcnow()), + "end_date": get_london_midnight_in_utc(datetime.utcnow()), + "email": { + "count": 3 + }, + "sms": { + "count": 2 + } + } diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index ba8dfd8e8..83e8d1132 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -35,7 +35,6 @@ from app.dao.notifications_dao import ( get_notifications_for_job, get_notifications_for_service, get_total_sent_notifications_in_date_range, - get_total_sent_notifications_yesterday, update_notification_status_by_id, update_notification_status_by_reference, dao_delete_notifications_and_history_by_id, @@ -44,12 +43,6 @@ from app.dao.notifications_dao import ( get_april_fools) from app.dao.services_dao import dao_update_service - -from app.utils import ( - get_london_midnight_in_utc, - get_midnight_for_day_before -) - from tests.app.conftest import ( sample_notification, sample_template, @@ -1351,33 +1344,6 @@ def test_get_total_sent_notifications_in_date_range_returns_only_in_date_range( assert total_count == 3 -@pytest.mark.parametrize('notification_type', ['sms', 'email']) -def test_get_total_sent_notifications_in_date_range_excludes_created_and_sending( - 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 - ) - - 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(status='sending') - notification_history(status='created') - notification_history(status='failed') - notification_history(status='delivered') - - total_count = get_total_sent_notifications_in_date_range(start_date, end_date, notification_type) - assert total_count == 2 - - @pytest.mark.parametrize('notification_type', ['sms', 'email']) def test_get_total_sent_notifications_in_date_range_excludes_test_key_notifications( notify_db, @@ -1432,7 +1398,7 @@ def test_get_total_sent_notifications_for_sms_excludes_email_counts( assert total_count == 3 -def test_get_total_sent_notifications_for_sms_excludes_sms_counts( +def test_get_total_sent_notifications_for_email_excludes_sms_counts( notify_db, notify_db_session, sample_template @@ -1456,43 +1422,3 @@ def test_get_total_sent_notifications_for_sms_excludes_sms_counts( total_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email') assert total_count == 2 - - -@freeze_time("2016-01-11 12:30:00") -def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict( - notify_db, - notify_db_session, - sample_template -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - status='delivered' - ) - - notification_history(notification_type='email') - notification_history(notification_type='sms') - - # Create some notifications for the day before - yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) - with freeze_time(yesterday): - notification_history(notification_type='sms') - notification_history(notification_type='sms') - notification_history(notification_type='email') - notification_history(notification_type='email') - notification_history(notification_type='email') - - total_count_dict = get_total_sent_notifications_yesterday() - - assert total_count_dict == { - "start_date": get_midnight_for_day_before(datetime.utcnow()), - "end_date": get_london_midnight_in_utc(datetime.utcnow()), - "email": { - "count": 3 - }, - "sms": { - "count": 2 - } - }