diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 10c8a91e4..9d0b22ab0 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -216,6 +216,11 @@ def get_notifications(service_id, message_type, status_override=None): abort(404) filter_args = parse_filter_args(request.args) filter_args['status'] = set_status_filters(filter_args) + + service_data_retention_days = service_api_client.get_service_data_retention_by_notification_type( + service_id, message_type + ).get('days_of_retention', current_app.config['ACTIVITY_STATS_LIMIT_DAYS']) + if request.path.endswith('csv') and current_user.has_permissions('view_activity'): return Response( generate_notifications_csv( @@ -224,7 +229,7 @@ def get_notifications(service_id, message_type, status_override=None): page_size=5000, template_type=[message_type], status=filter_args.get('status'), - limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'] + limit_days=service_data_retention_days ), mimetype='text/csv', headers={ @@ -235,7 +240,7 @@ def get_notifications(service_id, message_type, status_override=None): page=page, template_type=[message_type] if message_type else [], status=filter_args.get('status'), - limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'], + limit_days=service_data_retention_days, to=request.form.get('to', ''), ) url_args = { @@ -262,13 +267,18 @@ def get_notifications(service_id, message_type, status_override=None): download_link = None return { + 'service_data_retention_days': service_data_retention_days, 'counts': render_template( 'views/activity/counts.html', status=request.args.get('status'), status_filters=get_status_filters( current_service, message_type, - service_api_client.get_service_statistics(service_id, today_only=False) + service_api_client.get_service_statistics( + service_id, + today_only=False, + limit_days=service_data_retention_days + ) ) ), 'notifications': render_template( @@ -277,6 +287,7 @@ def get_notifications(service_id, message_type, status_override=None): notifications['notifications'] )), page=page, + limit_days=service_data_retention_days, prev_page=prev_page, next_page=next_page, status=request.args.get('status'), diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index b2d2f79b5..73ac7b6f3 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -41,8 +41,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): """ return self.get('/service/{0}'.format(service_id)) - def get_service_statistics(self, service_id, today_only): - return self.get('/service/{0}/statistics'.format(service_id), params={'today_only': today_only})['data'] + def get_service_statistics(self, service_id, today_only, limit_days=None): + return self.get('/service/{0}/statistics'.format(service_id), params={ + 'today_only': today_only, 'limit_days': limit_days + })['data'] def get_services(self, params_dict=None): """ @@ -498,5 +500,8 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_service_data_retention(self, service_id): return self.get("/service/{}/data-retention".format(service_id)) + def get_service_data_retention_by_notification_type(self, service_id, notification_type): + return self.get("/service/{}/data-retention/notification-type/{}".format(service_id, notification_type)) + def get_service_data_retention_by_id(self, service_id, data_retention_id): return self.get("service/{}/data-retention/{}".format(service_id, data_retention_id)) diff --git a/app/templates/views/activity/notifications.html b/app/templates/views/activity/notifications.html index a4bc2a3c8..d2b8e3465 100644 --- a/app/templates/views/activity/notifications.html +++ b/app/templates/views/activity/notifications.html @@ -11,7 +11,7 @@ notifications, caption="Recent activity", caption_visible=False, - empty_message='No messages found  (messages are kept for 7 days)'|safe, + empty_message='No messages found  (messages are kept for {} days)'.format(limit_days)|safe, field_headings=['Recipient', 'Status'], field_headings_visible=False ) %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 6f361d708..41c89b5d6 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -58,7 +58,7 @@

Download this report   - Data available for 7 days + Data available for {{ partials.service_data_retention_days }} days

{% endif %} diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 74d2e0c6b..2dfc2a198 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -70,6 +70,7 @@ def test_can_show_notifications( service_one, mock_get_notifications, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mock_has_no_jobs, user, extra_args, @@ -144,7 +145,7 @@ def test_can_show_notifications( **extra_args )) json_content = json.loads(json_response.get_data(as_text=True)) - assert json_content.keys() == {'counts', 'notifications'} + assert json_content.keys() == {'counts', 'notifications', 'service_data_retention_days'} @pytest.mark.parametrize('user, query_parameters, expected_download_link', [ @@ -194,6 +195,7 @@ def test_link_to_download_notifications( fake_uuid, mock_get_notifications, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mock_has_no_jobs, user, query_parameters, @@ -229,6 +231,7 @@ def test_letters_with_status_virus_scan_failed_shows_a_failure_description( logged_in_client, service_one, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, ): mock_get_notifications( mocker, @@ -258,6 +261,7 @@ def test_should_not_show_preview_link_for_precompiled_letters_in_virus_states( logged_in_client, service_one, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, letter_status, ): mock_get_notifications( @@ -280,6 +284,7 @@ def test_should_not_show_preview_link_for_precompiled_letters_in_virus_states( def test_shows_message_when_no_notifications( client_request, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mock_get_notifications_with_no_notifications, ): @@ -341,6 +346,7 @@ def test_search_recipient_form( logged_in_client, mock_get_notifications, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, initial_query_arguments, form_post_data, expected_search_box_label, @@ -382,6 +388,7 @@ def test_should_show_notifications_for_a_service_with_next_previous( active_user_with_permissions, mock_get_notifications_with_previous_next, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mocker, ): response = logged_in_client.get(url_for( @@ -462,6 +469,7 @@ def test_html_contains_notification_id( active_user_with_permissions, mock_get_notifications, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mocker, ): response = logged_in_client.get(url_for( @@ -482,6 +490,7 @@ def test_redacts_templates_that_should_be_redacted( mocker, active_user_with_permissions, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, ): mock_get_notifications( mocker, @@ -514,6 +523,7 @@ def test_big_numbers_and_search_dont_show_for_letters( mock_get_notifications, active_user_with_permissions, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, message_type, tablist_visible, search_bar_visible @@ -543,6 +553,7 @@ def test_sending_status_hint_does_not_include_status_for_letters( service_one, active_user_with_permissions, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, message_type, hint_status_visible, mocker @@ -570,6 +581,7 @@ def test_should_expected_hint_for_letters( service_one, active_user_with_permissions, mock_get_service_statistics, + mock_get_service_data_retention_by_notification_type, mocker, fake_uuid, is_precompiled_letter, diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 7bc4afd4f..92dac3626 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -35,15 +35,21 @@ def test_client_gets_service(mocker): mock_get.assert_called_once_with('/service/foo') -@pytest.mark.parametrize('today_only', [True, False]) -def test_client_gets_service_statistics(mocker, today_only): +@pytest.mark.parametrize('today_only, limit_days', [ + (True, None), + (False, None), + (False, 30), +]) +def test_client_gets_service_statistics(mocker, today_only, limit_days): client = ServiceAPIClient() mock_get = mocker.patch.object(client, 'get', return_value={'data': {'a': 'b'}}) - ret = client.get_service_statistics('foo', today_only) + ret = client.get_service_statistics('foo', today_only, limit_days) assert ret == {'a': 'b'} - mock_get.assert_called_once_with('/service/foo/statistics', params={'today_only': today_only}) + mock_get.assert_called_once_with('/service/foo/statistics', params={ + 'today_only': today_only, 'limit_days': limit_days + }) def test_client_only_updates_allowed_attributes(mocker): diff --git a/tests/conftest.py b/tests/conftest.py index dee53e4ee..8008b5d64 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -523,7 +523,7 @@ def mock_get_international_service(mocker, api_user_active): @pytest.fixture(scope='function') def mock_get_service_statistics(mocker, api_user_active): - def _get(service_id, today_only): + def _get(service_id, today_only, limit_days=None): return { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, @@ -533,6 +533,14 @@ def mock_get_service_statistics(mocker, api_user_active): return mocker.patch('app.service_api_client.get_service_statistics', side_effect=_get) +@pytest.fixture(scope='function') +def mock_get_service_data_retention_by_notification_type(mocker, api_user_active): + def _get(service_id, notification_type): + return {} + + return mocker.patch('app.service_api_client.get_service_data_retention_by_notification_type', side_effect=_get) + + @pytest.fixture(scope='function') def mock_get_detailed_services(mocker, fake_uuid): service_one = service_json(