mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-07-16 03:10:16 -04:00
Don’t ask for data retention unless channel is known
If the user is looking at the notifications page for all message types (which is what we show ‘caseworkers’) then it doesn’t make sense to ask the API for the data retention period for that message type (because it will be `None`). Doing so causes the API to return a `404`, which then causes the admin app to return `404`. Passing through `None` as the value of limit days will just cause the API to return everything in the `notifications` table, which is fine for us.
This commit is contained in:
@@ -216,10 +216,12 @@ 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 = None
|
||||
|
||||
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 message_type is not None:
|
||||
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(
|
||||
|
||||
@@ -21,10 +21,28 @@ from tests.conftest import (
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"user,extra_args,expected_update_endpoint,page_title", [
|
||||
(active_user_view_permissions, {'message_type': 'email'}, '/email.json', 'Emails'),
|
||||
(active_user_view_permissions, {'message_type': 'sms'}, '/sms.json', 'Text messages'),
|
||||
(active_caseworking_user, {}, '.json', 'Sent messages'),
|
||||
"user,extra_args,expected_update_endpoint,expected_limit_days,page_title", [
|
||||
(
|
||||
active_user_view_permissions,
|
||||
{'message_type': 'email'},
|
||||
'/email.json',
|
||||
7,
|
||||
'Emails',
|
||||
),
|
||||
(
|
||||
active_user_view_permissions,
|
||||
{'message_type': 'sms'},
|
||||
'/sms.json',
|
||||
7,
|
||||
'Text messages',
|
||||
),
|
||||
(
|
||||
active_caseworking_user,
|
||||
{},
|
||||
'.json',
|
||||
None,
|
||||
'Sent messages',
|
||||
),
|
||||
]
|
||||
)
|
||||
@pytest.mark.parametrize(
|
||||
@@ -76,6 +94,7 @@ def test_can_show_notifications(
|
||||
user,
|
||||
extra_args,
|
||||
expected_update_endpoint,
|
||||
expected_limit_days,
|
||||
page_title,
|
||||
status_argument,
|
||||
expected_api_call,
|
||||
@@ -131,7 +150,7 @@ def test_can_show_notifications(
|
||||
assert 'to' not in query_dict
|
||||
|
||||
mock_get_notifications.assert_called_with(
|
||||
limit_days=7,
|
||||
limit_days=expected_limit_days,
|
||||
page=expected_page_argument,
|
||||
service_id=service_one['id'],
|
||||
status=expected_api_call,
|
||||
@@ -149,6 +168,20 @@ def test_can_show_notifications(
|
||||
assert json_content.keys() == {'counts', 'notifications', 'service_data_retention_days'}
|
||||
|
||||
|
||||
def test_can_show_notifications_if_data_retention_not_available(
|
||||
client_request,
|
||||
mock_get_notifications,
|
||||
mock_get_service_statistics,
|
||||
mock_has_no_jobs,
|
||||
):
|
||||
page = client_request.get(
|
||||
'main.view_notifications',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
status='sending,delivered,failed',
|
||||
)
|
||||
assert page.h1.text.strip() == 'Messages'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('user, query_parameters, expected_download_link', [
|
||||
(
|
||||
active_user_with_permissions,
|
||||
|
||||
Reference in New Issue
Block a user