From 9674005c71de71e4e219e86036207a2c60e9ecc4 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 18 Jul 2018 13:39:15 +0100 Subject: [PATCH 1/3] Updated the function to delete notifications over a week old to look at ServiceDataRetention to only delete notifications specified number of days for the notification type. --- app/dao/notifications_dao.py | 22 ++- .../notification_dao/test_notification_dao.py | 100 ---------- ...t_notification_dao_delete_notifications.py | 187 ++++++++++++++++++ 3 files changed, 206 insertions(+), 103 deletions(-) create mode 100644 tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index dce08f132..cbbcc73cc 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -7,7 +7,6 @@ from datetime import ( ) from flask import current_app - from notifications_utils.recipients import ( validate_and_format_email_address, InvalidEmailError, @@ -42,7 +41,8 @@ from app.models import ( NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_SENT, SMS_TYPE, - EMAIL_TYPE + EMAIL_TYPE, + ServiceDataRetention ) from app.dao.dao_utils import transactional @@ -310,10 +310,26 @@ def _filter_query(query, filter_dict=None): @statsd(namespace="dao") @transactional def delete_notifications_created_more_than_a_week_ago_by_type(notification_type): + flexible_data_retention = ServiceDataRetention.query.filter( + ServiceDataRetention.notification_type == notification_type + ).all() + + deleted = 0 + for f in flexible_data_retention: + days_of_retention = convert_utc_to_bst(datetime.utcnow()).date() - timedelta(days=f.days_of_retention) + deleted += db.session.query(Notification).filter( + func.date(Notification.created_at) < days_of_retention, + Notification.notification_type == f.notification_type, + Notification.service_id == f.service_id + ).delete(synchronize_session='fetch') + seven_days_ago = convert_utc_to_bst(datetime.utcnow()).date() - timedelta(days=7) - deleted = db.session.query(Notification).filter( + services_with_data_retention = [x.service_id for x in flexible_data_retention] + + deleted += db.session.query(Notification).filter( func.date(Notification.created_at) < seven_days_ago, Notification.notification_type == notification_type, + Notification.service_id.notin_(services_with_data_retention) ).delete(synchronize_session='fetch') return deleted diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 3e36c697c..3a0b4e324 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -54,11 +54,9 @@ from app.models import ( from tests.app.conftest import ( sample_notification, sample_template as create_sample_template, - sample_email_template, sample_service, sample_job, sample_notification_history as create_notification_history, - sample_letter_template ) from tests.app.db import ( create_job, @@ -607,104 +605,6 @@ def test_update_notification_sets_status(sample_notification): assert notification_from_db.status == 'failed' -@pytest.mark.parametrize('month, delete_run_time', - [(4, '2016-04-10 23:40'), (1, '2016-01-11 00:40')]) -@pytest.mark.parametrize( - 'notification_type, expected_sms_count, expected_email_count, expected_letter_count', - [('sms', 7, 10, 10), - ('email', 10, 7, 10), - ('letter', 10, 10, 7)] -) -def test_should_delete_notifications_by_type_after_seven_days( - sample_service, - month, - delete_run_time, - notification_type, - expected_sms_count, - expected_email_count, - expected_letter_count -): - assert len(Notification.query.all()) == 0 - - email_template = create_template(service=sample_service, template_type='email') - sms_template = create_template(service=sample_service) - letter_template = create_template(service=sample_service, template_type='letter') - - # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type - for i in range(1, 11): - past_date = '2016-0{0}-{1:02d} {1:02d}:00:00.000000'.format(month, i) - with freeze_time(past_date): - create_notification(template=email_template, created_at=datetime.utcnow(), status="permanent-failure") - create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered") - create_notification(template=letter_template, created_at=datetime.utcnow(), status="temporary-failure") - - all_notifications = Notification.query.all() - assert len(all_notifications) == 30 - - # Records from before 3rd should be deleted - with freeze_time(delete_run_time): - delete_notifications_created_more_than_a_week_ago_by_type(notification_type) - remaining_sms_notifications = Notification.query.filter_by(notification_type='sms').all() - remaining_letter_notifications = Notification.query.filter_by(notification_type='letter').all() - remaining_email_notifications = Notification.query.filter_by(notification_type='email').all() - - assert len(remaining_sms_notifications) == expected_sms_count - assert len(remaining_email_notifications) == expected_email_count - assert len(remaining_letter_notifications) == expected_letter_count - - if notification_type == 'sms': - notifications_to_check = remaining_sms_notifications - if notification_type == 'email': - notifications_to_check = remaining_email_notifications - if notification_type == 'letter': - notifications_to_check = remaining_letter_notifications - - for notification in notifications_to_check: - assert notification.created_at.date() >= date(2016, month, 3) - - -@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) -@freeze_time("2016-01-10 12:00:00.000000") -def test_should_not_delete_notification_history(notify_db, notify_db_session, sample_service, notification_type): - with freeze_time('2016-01-01 12:00'): - email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) - sms_template = create_sample_template(notify_db, notify_db_session, service=sample_service) - letter_template = sample_letter_template(sample_service) - - sample_notification( - notify_db, - notify_db_session, - created_at=datetime.utcnow(), - status="failed", - service=sample_service, - template=email_template - ) - sample_notification( - notify_db, - notify_db_session, - created_at=datetime.utcnow(), - status="failed", - service=sample_service, - template=sms_template - ) - sample_notification( - notify_db, - notify_db_session, - created_at=datetime.utcnow(), - status="failed", - service=sample_service, - template=letter_template - ) - - assert Notification.query.count() == 3 - assert NotificationHistory.query.count() == 3 - - delete_notifications_created_more_than_a_week_ago_by_type(notification_type) - - assert Notification.query.count() == 2 - assert NotificationHistory.query.count() == 3 - - @freeze_time("2016-01-10") def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template): assert len(Notification.query.all()) == 0 diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py new file mode 100644 index 000000000..2235db882 --- /dev/null +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -0,0 +1,187 @@ +from datetime import ( + datetime, + date, + timedelta +) + +import pytest +from freezegun import freeze_time + +from app.dao.notifications_dao import delete_notifications_created_more_than_a_week_ago_by_type +from app.models import Notification, NotificationHistory +from tests.app.db import ( + create_template, + create_notification, + create_service_data_retention, + create_service +) + + +@pytest.mark.parametrize('month, delete_run_time', + [(4, '2016-04-10 23:40'), (1, '2016-01-11 00:40')]) +@pytest.mark.parametrize( + 'notification_type, expected_sms_count, expected_email_count, expected_letter_count', + [('sms', 7, 10, 10), + ('email', 10, 7, 10), + ('letter', 10, 10, 7)] +) +def test_should_delete_notifications_by_type_after_seven_days( + sample_service, + month, + delete_run_time, + notification_type, + expected_sms_count, + expected_email_count, + expected_letter_count +): + assert len(Notification.query.all()) == 0 + + email_template, letter_template, sms_template = _create_templates(sample_service) + + # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type + for i in range(1, 11): + past_date = '2016-0{0}-{1:02d} {1:02d}:00:00.000000'.format(month, i) + with freeze_time(past_date): + create_notification(template=email_template, created_at=datetime.utcnow(), status="permanent-failure") + create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered") + create_notification(template=letter_template, created_at=datetime.utcnow(), status="temporary-failure") + + all_notifications = Notification.query.all() + assert len(all_notifications) == 30 + + # Records from before 3rd should be deleted + with freeze_time(delete_run_time): + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) + remaining_sms_notifications = Notification.query.filter_by(notification_type='sms').all() + remaining_letter_notifications = Notification.query.filter_by(notification_type='letter').all() + remaining_email_notifications = Notification.query.filter_by(notification_type='email').all() + + assert len(remaining_sms_notifications) == expected_sms_count + assert len(remaining_email_notifications) == expected_email_count + assert len(remaining_letter_notifications) == expected_letter_count + + if notification_type == 'sms': + notifications_to_check = remaining_sms_notifications + if notification_type == 'email': + notifications_to_check = remaining_email_notifications + if notification_type == 'letter': + notifications_to_check = remaining_letter_notifications + + for notification in notifications_to_check: + assert notification.created_at.date() >= date(2016, month, 3) + + +@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +@freeze_time("2016-01-10 12:00:00.000000") +def test_should_not_delete_notification_history(sample_service, notification_type): + with freeze_time('2016-01-01 12:00'): + email_template, letter_template, sms_template = _create_templates(sample_service) + + create_notification(template=email_template, status='permanent-failure') + create_notification(template=sms_template, status='permanent-failure') + create_notification(template=letter_template, status='permanent-failure') + + assert Notification.query.count() == 3 + assert NotificationHistory.query.count() == 3 + + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) + + assert Notification.query.count() == 2 + assert NotificationHistory.query.count() == 3 + + +@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +def test_delete_notifications_for_days_of_retention(sample_service, notification_type): + service_with_default_data_retention = create_service(service_name='default data retention') + email_template, letter_template, sms_template = _create_templates(sample_service) + default_email_template, default_letter_template, default_sms_template = _create_templates( + service_with_default_data_retention) + create_notification(template=email_template, status='delivered') + create_notification(template=sms_template, status='permanent-failure') + create_notification(template=letter_template, status='temporary-failure') + create_notification(template=email_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=4)) + create_notification(template=sms_template, status='permanent-failure', + created_at=datetime.utcnow() - timedelta(days=4)) + create_notification(template=letter_template, status='temporary-failure', + created_at=datetime.utcnow() - timedelta(days=4)) + create_notification(template=default_email_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=8)) + create_notification(template=default_sms_template, status='permanent-failure', + created_at=datetime.utcnow() - timedelta(days=8)) + create_notification(template=default_letter_template, status='temporary-failure', + created_at=datetime.utcnow() - timedelta(days=8)) + create_service_data_retention(service_id=sample_service.id, notification_type=notification_type) + + assert len(Notification.query.all()) == 9 + + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) + assert len(Notification.query.all()) == 7 + assert len(Notification.query.filter_by(notification_type=notification_type).all()) == 1 + + +@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +def test_delete_notifications_keep_data_for_days_of_retention_is_longer(sample_service, notification_type): + create_service_data_retention(service_id=sample_service.id, notification_type=notification_type, + days_of_retention=15) + email_template, letter_template, sms_template = _create_templates(sample_service) + + service_with_default_data_retention = create_service(service_name='default data retention') + default_email_template, default_letter_template, default_sms_template = _create_templates( + service_with_default_data_retention) + + create_notification(template=email_template, status='delivered') + create_notification(template=sms_template, status='permanent-failure') + create_notification(template=letter_template, status='temporary-failure') + + create_notification(template=email_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=14)) + create_notification(template=sms_template, status='permanent-failure', + created_at=datetime.utcnow() - timedelta(days=14)) + create_notification(template=letter_template, status='temporary-failure', + created_at=datetime.utcnow() - timedelta(days=14)) + + create_notification(template=default_email_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=8)) + create_notification(template=default_sms_template, status='permanent-failure', + created_at=datetime.utcnow() - timedelta(days=8)) + create_notification(template=default_letter_template, status='temporary-failure', + created_at=datetime.utcnow() - timedelta(days=8)) + + assert len(Notification.query.all()) == 9 + + delete_notifications_created_more_than_a_week_ago_by_type(notification_type) + assert len(Notification.query.filter_by().all()) == 8 + assert len(Notification.query.filter_by(notification_type=notification_type).all()) == 2 + + +def test_delete_notifications_delete_notification_type_for_default_time_if_no_days_of_retention_for_type( + sample_service +): + create_service_data_retention(service_id=sample_service.id, notification_type='sms', + days_of_retention=15) + email_template, letter_template, sms_template = _create_templates(sample_service) + + create_notification(template=email_template, status='delivered') + create_notification(template=sms_template, status='permanent-failure') + create_notification(template=letter_template, status='temporary-failure') + + create_notification(template=email_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=14)) + create_notification(template=sms_template, status='permanent-failure', + created_at=datetime.utcnow() - timedelta(days=14)) + create_notification(template=letter_template, status='temporary-failure', + created_at=datetime.utcnow() - timedelta(days=14)) + + assert len(Notification.query.all()) == 6 + + delete_notifications_created_more_than_a_week_ago_by_type('email') + assert len(Notification.query.filter_by().all()) == 5 + assert len(Notification.query.filter_by(notification_type='email').all()) == 1 + + +def _create_templates(sample_service): + email_template = create_template(service=sample_service, template_type='email') + sms_template = create_template(service=sample_service) + letter_template = create_template(service=sample_service, template_type='letter') + return email_template, letter_template, sms_template From dae29a1b611df34508695994fdd77760afc92f71 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 19 Jul 2018 14:28:15 +0100 Subject: [PATCH 2/3] Update the query for the notifications activity page to return the data for the days of retention if set. --- app/dao/service_data_retention_dao.py | 8 +++ app/service/rest.py | 10 +++- .../dao/test_service_data_retention_dao.py | 19 ++++++- tests/app/service/test_rest.py | 54 ++++++++++++------- 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/app/dao/service_data_retention_dao.py b/app/dao/service_data_retention_dao.py index 1e27c52a1..2e2859302 100644 --- a/app/dao/service_data_retention_dao.py +++ b/app/dao/service_data_retention_dao.py @@ -20,6 +20,14 @@ def fetch_service_data_retention(service_id): return data_retention_list +def fetch_service_data_retention_by_notification_type(service_id, notification_type): + data_retention_list = ServiceDataRetention.query.filter_by( + service_id=service_id, + notification_type=notification_type + ).first() + return data_retention_list + + @transactional def insert_service_data_retention(service_id, notification_type, days_of_retention): new_data_retention = ServiceDataRetention(service_id=service_id, diff --git a/app/service/rest.py b/app/service/rest.py index 8eb157b98..4e3a638d8 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -29,6 +29,7 @@ from app.dao.service_data_retention_dao import ( fetch_service_data_retention_by_id, insert_service_data_retention, update_service_data_retention, + fetch_service_data_retention_by_notification_type ) from app.dao.service_sms_sender_dao import ( archive_sms_sender, @@ -324,15 +325,20 @@ def get_service_history(service_id): @service_blueprint.route('//notifications', methods=['GET']) def get_all_notifications_for_service(service_id): data = notifications_filter_schema.load(request.args).data + notification_type = data.get('template_type')[0] if data.get('template_type') else None if data.get('to'): - notification_type = data.get('template_type')[0] if data.get('template_type') else None return search_for_notification_by_to_field(service_id=service_id, search_term=data['to'], statuses=data.get('status'), notification_type=notification_type) page = data['page'] if 'page' in data else 1 page_size = data['page_size'] if 'page_size' in data else current_app.config.get('PAGE_SIZE') - limit_days = data.get('limit_days') + days_of_retention = None + if notification_type: + days_of_retention = fetch_service_data_retention_by_notification_type( + service_id=service_id, + notification_type=notification_type).days_of_retention + limit_days = days_of_retention if days_of_retention else data.get('limit_days') include_jobs = data.get('include_jobs', True) include_from_test_key = data.get('include_from_test_key', False) include_one_off = data.get('include_one_off', True) diff --git a/tests/app/dao/test_service_data_retention_dao.py b/tests/app/dao/test_service_data_retention_dao.py index cbf3661d2..19b77ca63 100644 --- a/tests/app/dao/test_service_data_retention_dao.py +++ b/tests/app/dao/test_service_data_retention_dao.py @@ -8,10 +8,11 @@ from app.dao.service_data_retention_dao import ( fetch_service_data_retention, insert_service_data_retention, update_service_data_retention, - fetch_service_data_retention_by_id + fetch_service_data_retention_by_id, + fetch_service_data_retention_by_notification_type ) from app.models import ServiceDataRetention -from tests.app.db import create_service +from tests.app.db import create_service, create_service_data_retention def test_fetch_service_data_retention(sample_service): @@ -131,3 +132,17 @@ def test_update_service_data_retention_does_not_update_row_if_data_retention_is_ service_id=uuid.uuid4(), days_of_retention=5) assert updated_count == 0 + + +@pytest.mark.parametrize('notification_type, alternate', + [('sms', 'email'), + ('email', 'sms'), ('letter', 'email')]) +def test_fetch_service_data_retention_by_notification_type(sample_service, notification_type, alternate): + data_retention = create_service_data_retention(service_id=sample_service.id, notification_type=notification_type) + create_service_data_retention(service_id=sample_service.id, notification_type=alternate) + result = fetch_service_data_retention_by_notification_type(sample_service.id, notification_type) + assert result == data_retention + + +def test_fetch_service_data_retention_by_notification_type_returns_none_when_no_rows(sample_service): + assert not fetch_service_data_retention_by_notification_type(sample_service.id, 'email') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ac4fb027a..a2c82d9f8 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -42,7 +42,8 @@ from tests.app.db import ( create_letter_contact, create_inbound_number, create_service_sms_sender, - create_service_with_defined_sms_sender + create_service_with_defined_sms_sender, + create_service_data_retention ) from tests.app.db import create_user @@ -1225,29 +1226,29 @@ def test_get_service_and_api_key_history(notify_api, notify_db, notify_db_sessio assert json_resp['data']['api_key_history'][0]['id'] == str(api_key.id) -def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notify_db_session): - with notify_api.test_request_context(), notify_api.test_client() as client: - service_1 = create_service(service_name="1", email_from='1') - service_2 = create_service(service_name="2", email_from='2') +def test_get_all_notifications_for_service_in_order(client, notify_db_session): + service_1 = create_service(service_name="1", email_from='1') + service_2 = create_service(service_name="2", email_from='2') + template_1 = create_template(service=service_1) + template_2 = create_template(service=service_2) + create_notification(template=template_2) - create_sample_notification(notify_db, notify_db_session, service=service_2) + notification_1 = create_notification(template=template_1) + notification_2 = create_notification(template=template_1) + notification_3 = create_notification(template=template_1) - notification_1 = create_sample_notification(notify_db, notify_db_session, service=service_1) - notification_2 = create_sample_notification(notify_db, notify_db_session, service=service_1) - notification_3 = create_sample_notification(notify_db, notify_db_session, service=service_1) + auth_header = create_authorization_header() - auth_header = create_authorization_header() + response = client.get( + path='/service/{}/notifications'.format(service_1.id), + headers=[auth_header]) - response = client.get( - path='/service/{}/notifications'.format(service_1.id), - headers=[auth_header]) - - resp = json.loads(response.get_data(as_text=True)) - assert len(resp['notifications']) == 3 - assert resp['notifications'][0]['to'] == notification_3.to - assert resp['notifications'][1]['to'] == notification_2.to - assert resp['notifications'][2]['to'] == notification_1.to - assert response.status_code == 200 + resp = json.loads(response.get_data(as_text=True)) + assert len(resp['notifications']) == 3 + assert resp['notifications'][0]['id'] == str(notification_3.id) + assert resp['notifications'][1]['id'] == str(notification_2.id) + assert resp['notifications'][2]['id'] == str(notification_1.id) + assert response.status_code == 200 def test_get_all_notifications_for_service_formatted_for_csv(client, sample_template): @@ -1268,6 +1269,19 @@ def test_get_all_notifications_for_service_formatted_for_csv(client, sample_temp assert resp['notifications'][0]['status'] == 'Sending' +def test_get_all_notifications_for_service_limits_days_for_number_of_days_of_retention(client, sample_template): + create_service_data_retention(service_id=sample_template.service_id, notification_type='sms', days_of_retention=9) + create_notification(template=sample_template) + create_notification(template=sample_template, created_at=datetime.utcnow() + timedelta(days=8)) + + response = client.get( + path='/service/{}/notifications?template_type=sms&limit_days=7'.format(sample_template.service_id), + headers=[create_authorization_header()]) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['notifications']) == 2 + + def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_session): service_1 = create_service(service_name="1", email_from='1') response = client.get( From 0675f09afb19d26d46fd3d9f6869793a61124b88 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 19 Jul 2018 17:06:46 +0100 Subject: [PATCH 3/3] Need to be able to query the notification statistics for the right number of days. If the request is for the big numbers on the activity page, then we need to use the number right number of days. Added an end point to get the data retention for the service and notification type, which is needed on the activity page to say how long the report is available for. --- app/dao/services_dao.py | 5 ++- app/service/rest.py | 24 ++++++++++---- tests/app/dao/test_services_dao.py | 33 ++++++++++++++++--- .../test_service_data_retention_rest.py | 9 +++++ tests/app/service/test_statistics_rest.py | 14 ++++---- 5 files changed, 64 insertions(+), 21 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index b9c66a3a0..b637bd4cd 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -246,9 +246,8 @@ def delete_service_and_all_associated_db_objects(service): @statsd(namespace="dao") -def dao_fetch_stats_for_service(service_id): - # We always want between seven and eight days - start_date = midnight_n_days_ago(7) +def dao_fetch_stats_for_service(service_id, limit_days): + start_date = midnight_n_days_ago(limit_days) return _stats_for_service_query(service_id).filter( Notification.created_at >= start_date ).all() diff --git a/app/service/rest.py b/app/service/rest.py index 4e3a638d8..4133f2d31 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -166,7 +166,9 @@ def get_service_by_id(service_id): @service_blueprint.route('//statistics') def get_service_notification_statistics(service_id): - return jsonify(data=get_service_statistics(service_id, request.args.get('today_only') == 'True')) + return jsonify(data=get_service_statistics(service_id, + request.args.get('today_only') == 'True', + request.args.get('limit_days', 7))) @service_blueprint.route('', methods=['POST']) @@ -425,15 +427,17 @@ def get_monthly_notification_stats(service_id): def get_detailed_service(service_id, today_only=False): service = dao_fetch_service_by_id(service_id) - service.statistics = get_service_statistics(service_id, today_only) + service.statistics = get_service_statistics(service_id, today_only, limit_days=7) return detailed_service_schema.dump(service).data -def get_service_statistics(service_id, today_only): - # today_only flag is used by the send page to work out if the service will exceed their daily usage by sending a job - stats_fn = dao_fetch_todays_stats_for_service if today_only else dao_fetch_stats_for_service - stats = stats_fn(service_id) - return statistics.format_statistics(stats) +def get_service_statistics(service_id, today_only, limit_days): + if today_only: + stats = dao_fetch_todays_stats_for_service(service_id) + return statistics.format_statistics(stats) + if limit_days: + stats = dao_fetch_stats_for_service(service_id=service_id, limit_days=int(limit_days)) + return statistics.format_statistics(stats) def get_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True): @@ -752,6 +756,12 @@ def is_service_name_unique(): return jsonify(result=result), 200 +@service_blueprint.route('//data-retention/notification-type/', methods=['GET']) +def get_data_retention_for_service_notification_type(service_id, notification_type): + data_retention = fetch_service_data_retention_by_notification_type(service_id, notification_type) + return jsonify(data_retention.serialize()), 200 + + @service_blueprint.route('//data-retention', methods=['GET']) def get_data_retention_for_service(service_id): data_retention_list = fetch_service_data_retention(service_id) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 2720db90c..f7cca600b 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -534,7 +534,7 @@ def test_fetch_stats_filters_on_service(sample_notification): message_limit=1000) dao_create_service(service_two, sample_notification.service.created_by) - stats = dao_fetch_stats_for_service(service_two.id) + stats = dao_fetch_stats_for_service(service_two.id, 7) assert len(stats) == 0 @@ -546,7 +546,7 @@ def test_fetch_stats_ignores_historical_notification_data(sample_notification): assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 1 - stats = dao_fetch_stats_for_service(service_id) + stats = dao_fetch_stats_for_service(service_id, 7) assert len(stats) == 0 @@ -557,7 +557,7 @@ def test_fetch_stats_counts_correctly(notify_db, notify_db_session, sample_templ create_notification(notify_db, notify_db_session, template=sample_email_template, status='technical-failure') create_notification(notify_db, notify_db_session, template=sample_template, status='created') - stats = dao_fetch_stats_for_service(sample_template.service_id) + stats = dao_fetch_stats_for_service(sample_template.service_id, 7) stats = sorted(stats, key=lambda x: (x.notification_type, x.status)) assert len(stats) == 3 @@ -591,7 +591,7 @@ def test_fetch_stats_counts_should_ignore_team_key( create_notification( notify_db, notify_db_session) - stats = dao_fetch_stats_for_service(sample_template.service_id) + stats = dao_fetch_stats_for_service(sample_template.service_id, 7) assert len(stats) == 1 assert stats[0].notification_type == 'sms' assert stats[0].status == 'created' @@ -634,7 +634,30 @@ def test_fetch_stats_should_not_gather_notifications_older_than_7_days(sample_te create_notification_db(sample_template,) with freeze_time('Monday 16th July 2018 12:00'): - stats = dao_fetch_stats_for_service(sample_template.service_id) + stats = dao_fetch_stats_for_service(sample_template.service_id, 7) + + assert len(stats) == rows_returned + + +@pytest.mark.parametrize('created_at, rows_returned', [ + ('Saturday 7th July 2018 12:00', 0), + ('Saturday 7th July 2018 22:59', 0), + ('Sunday 8th July 2018 12:00', 1), + ('Sunday 8th July 2018 22:59', 1), + ('Sunday 8th July 2018 23:00', 1), + ('Monday 9th July 2018 09:00', 1), + ('Monday 9th July 2018 15:00', 1), + ('Monday 16th July 2018 12:00', 1), +]) +def test_fetch_stats_should_not_gather_notifications_older_than_the_days_given( + sample_template, created_at, rows_returned +): + # It's monday today. Things made last monday should still show + with freeze_time(created_at): + create_notification_db(sample_template,) + + with freeze_time('Monday 16th July 2018 12:00'): + stats = dao_fetch_stats_for_service(sample_template.service_id, 8) assert len(stats) == rows_returned diff --git a/tests/app/service/test_service_data_retention_rest.py b/tests/app/service/test_service_data_retention_rest.py index da58d597e..7797f62cb 100644 --- a/tests/app/service/test_service_data_retention_rest.py +++ b/tests/app/service/test_service_data_retention_rest.py @@ -35,6 +35,15 @@ def test_get_service_data_retention_returns_empty_list(client, sample_service): assert len(json.loads(response.get_data(as_text=True))) == 0 +def test_get_data_retention_for_service_notification_type(client, sample_service): + data_retention = create_service_data_retention(service_id=sample_service.id) + response = client.get('/service/{}/data-retention/notification-type/{}'.format(sample_service.id, 'sms'), + headers=[('Content-Type', 'application/json'), create_authorization_header()], + ) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == data_retention.serialize() + + def test_get_service_data_retention_by_id(client, sample_service): sms_data_retention = create_service_data_retention(service_id=sample_service.id) create_service_data_retention(service_id=sample_service.id, notification_type='email', diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 4fe36a62a..68f949234 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -129,11 +129,11 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample assert resp_json[2]["is_precompiled_letter"] is False -@pytest.mark.parametrize('today_only, stats', [ - (False, {'requested': 2, 'delivered': 1, 'failed': 0}), - (True, {'requested': 1, 'delivered': 0, 'failed': 0}) +@pytest.mark.parametrize('today_only, limit_days, stats', [ + (False, 7, {'requested': 2, 'delivered': 1, 'failed': 0}), + (True, 7, {'requested': 1, 'delivered': 0, 'failed': 0}) ], ids=['seven_days', 'today']) -def test_get_service_notification_statistics(admin_request, sample_template, today_only, stats): +def test_get_service_notification_statistics(admin_request, sample_template, today_only, limit_days, stats): with freeze_time('2000-01-01T12:00:00'): create_notification(sample_template, status='delivered') with freeze_time('2000-01-02T12:00:00'): @@ -141,7 +141,8 @@ def test_get_service_notification_statistics(admin_request, sample_template, tod resp = admin_request.get( 'service.get_service_notification_statistics', service_id=sample_template.service_id, - today_only=today_only + today_only=today_only, + limit_days=limit_days ) assert set(resp['data'].keys()) == {SMS_TYPE, EMAIL_TYPE, LETTER_TYPE} @@ -151,7 +152,8 @@ def test_get_service_notification_statistics(admin_request, sample_template, tod def test_get_service_notification_statistics_with_unknown_service(admin_request): resp = admin_request.get( 'service.get_service_notification_statistics', - service_id=uuid.uuid4() + service_id=uuid.uuid4(), + limit_days=7 ) assert resp['data'] == {