From 0675f09afb19d26d46fd3d9f6869793a61124b88 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 19 Jul 2018 17:06:46 +0100 Subject: [PATCH] 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'] == {