From 019c6a4e3a271a3f4358ca69706677cc45228a66 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 3 Aug 2018 14:35:36 +0100 Subject: [PATCH] Revert "Purge notifications for configured days of retention" --- app/dao/notifications_dao.py | 22 +-- app/dao/service_data_retention_dao.py | 8 - app/dao/services_dao.py | 5 +- app/service/rest.py | 34 +--- .../notification_dao/test_notification_dao.py | 100 ++++++++++ ...t_notification_dao_delete_notifications.py | 187 ------------------ .../dao/test_service_data_retention_dao.py | 19 +- tests/app/dao/test_services_dao.py | 33 +--- tests/app/service/test_rest.py | 54 ++--- .../test_service_data_retention_rest.py | 9 - tests/app/service/test_statistics_rest.py | 14 +- 11 files changed, 148 insertions(+), 337 deletions(-) delete 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 cbbcc73cc..dce08f132 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -7,6 +7,7 @@ from datetime import ( ) from flask import current_app + from notifications_utils.recipients import ( validate_and_format_email_address, InvalidEmailError, @@ -41,8 +42,7 @@ from app.models import ( NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_SENT, SMS_TYPE, - EMAIL_TYPE, - ServiceDataRetention + EMAIL_TYPE ) from app.dao.dao_utils import transactional @@ -310,26 +310,10 @@ 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) - services_with_data_retention = [x.service_id for x in flexible_data_retention] - - deleted += db.session.query(Notification).filter( + 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/app/dao/service_data_retention_dao.py b/app/dao/service_data_retention_dao.py index 2e2859302..1e27c52a1 100644 --- a/app/dao/service_data_retention_dao.py +++ b/app/dao/service_data_retention_dao.py @@ -20,14 +20,6 @@ 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/dao/services_dao.py b/app/dao/services_dao.py index b637bd4cd..b9c66a3a0 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -246,8 +246,9 @@ def delete_service_and_all_associated_db_objects(service): @statsd(namespace="dao") -def dao_fetch_stats_for_service(service_id, limit_days): - start_date = midnight_n_days_ago(limit_days) +def dao_fetch_stats_for_service(service_id): + # We always want between seven and eight days + start_date = midnight_n_days_ago(7) 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 4133f2d31..8eb157b98 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -29,7 +29,6 @@ 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, @@ -166,9 +165,7 @@ 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', - request.args.get('limit_days', 7))) + return jsonify(data=get_service_statistics(service_id, request.args.get('today_only') == 'True')) @service_blueprint.route('', methods=['POST']) @@ -327,20 +324,15 @@ 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') - 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') + limit_days = 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) @@ -427,17 +419,15 @@ 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, limit_days=7) + service.statistics = get_service_statistics(service_id, today_only) return detailed_service_schema.dump(service).data -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_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_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True): @@ -756,12 +746,6 @@ 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/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 3a0b4e324..3e36c697c 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -54,9 +54,11 @@ 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, @@ -605,6 +607,104 @@ 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 deleted file mode 100644 index 2235db882..000000000 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ /dev/null @@ -1,187 +0,0 @@ -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 diff --git a/tests/app/dao/test_service_data_retention_dao.py b/tests/app/dao/test_service_data_retention_dao.py index 19b77ca63..cbf3661d2 100644 --- a/tests/app/dao/test_service_data_retention_dao.py +++ b/tests/app/dao/test_service_data_retention_dao.py @@ -8,11 +8,10 @@ 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_notification_type + fetch_service_data_retention_by_id ) from app.models import ServiceDataRetention -from tests.app.db import create_service, create_service_data_retention +from tests.app.db import create_service def test_fetch_service_data_retention(sample_service): @@ -132,17 +131,3 @@ 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/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f7cca600b..2720db90c 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, 7) + stats = dao_fetch_stats_for_service(service_two.id) 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, 7) + stats = dao_fetch_stats_for_service(service_id) 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, 7) + stats = dao_fetch_stats_for_service(sample_template.service_id) 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, 7) + stats = dao_fetch_stats_for_service(sample_template.service_id) assert len(stats) == 1 assert stats[0].notification_type == 'sms' assert stats[0].status == 'created' @@ -634,30 +634,7 @@ 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, 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) + stats = dao_fetch_stats_for_service(sample_template.service_id) assert len(stats) == rows_returned diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a2c82d9f8..ac4fb027a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -42,8 +42,7 @@ from tests.app.db import ( create_letter_contact, create_inbound_number, create_service_sms_sender, - create_service_with_defined_sms_sender, - create_service_data_retention + create_service_with_defined_sms_sender ) from tests.app.db import create_user @@ -1226,29 +1225,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(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) +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') - notification_1 = create_notification(template=template_1) - notification_2 = create_notification(template=template_1) - notification_3 = create_notification(template=template_1) + create_sample_notification(notify_db, notify_db_session, service=service_2) - auth_header = create_authorization_header() + 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) - response = client.get( - path='/service/{}/notifications'.format(service_1.id), - headers=[auth_header]) + auth_header = create_authorization_header() - 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 + 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 def test_get_all_notifications_for_service_formatted_for_csv(client, sample_template): @@ -1269,19 +1268,6 @@ 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( diff --git a/tests/app/service/test_service_data_retention_rest.py b/tests/app/service/test_service_data_retention_rest.py index 7797f62cb..da58d597e 100644 --- a/tests/app/service/test_service_data_retention_rest.py +++ b/tests/app/service/test_service_data_retention_rest.py @@ -35,15 +35,6 @@ 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 68f949234..4fe36a62a 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, limit_days, stats', [ - (False, 7, {'requested': 2, 'delivered': 1, 'failed': 0}), - (True, 7, {'requested': 1, 'delivered': 0, 'failed': 0}) +@pytest.mark.parametrize('today_only, stats', [ + (False, {'requested': 2, 'delivered': 1, 'failed': 0}), + (True, {'requested': 1, 'delivered': 0, 'failed': 0}) ], ids=['seven_days', 'today']) -def test_get_service_notification_statistics(admin_request, sample_template, today_only, limit_days, stats): +def test_get_service_notification_statistics(admin_request, sample_template, today_only, stats): with freeze_time('2000-01-01T12:00:00'): create_notification(sample_template, status='delivered') with freeze_time('2000-01-02T12:00:00'): @@ -141,8 +141,7 @@ 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, - limit_days=limit_days + today_only=today_only ) assert set(resp['data'].keys()) == {SMS_TYPE, EMAIL_TYPE, LETTER_TYPE} @@ -152,8 +151,7 @@ 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(), - limit_days=7 + service_id=uuid.uuid4() ) assert resp['data'] == {