From 5da69dd495f5105fded48c71db1c621a279f4e11 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 21 Jul 2021 16:06:23 +0100 Subject: [PATCH] dont let people get into one-off flow for deleted templates previously we'd skip the template page entirely if someone didnt have manage templates/api keys permission. however, if the template is deleted you'd then go through the flow entering placeholders and stuff before it would then crash when trying to send. instead, just bounce the user to the template page. It has the content and says when the template was deleted. --- app/main/views/send.py | 2 +- app/main/views/templates.py | 3 +-- app/utils/__init__.py | 5 +++-- tests/app/main/views/test_templates.py | 26 ++++++++++++++++++++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 083961977..04356de50 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -874,7 +874,7 @@ def get_send_test_page_title(template_type, entering_recipient, name=None): def get_back_link(service_id, template, step_index, placeholders=None): if step_index == 0: - if should_skip_template_page(template.template_type): + if should_skip_template_page(template._template): return url_for( '.choose_template', service_id=service_id, diff --git a/app/main/views/templates.py b/app/main/views/templates.py index a389af900..79ef96cdb 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -58,8 +58,7 @@ def view_template(service_id, template_id): template_folder = current_service.get_template_folder(template['folder']) user_has_template_permission = current_user.has_template_folder_permission(template_folder) - - if should_skip_template_page(template['template_type']): + if should_skip_template_page(template): return redirect(url_for( '.set_sender', service_id=service_id, template_id=template_id )) diff --git a/app/utils/__init__.py b/app/utils/__init__.py index c80aa9b8f..6d1f3dafb 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -77,11 +77,12 @@ def unicode_truncate(s, length): return encoded.decode('utf-8', 'ignore') -def should_skip_template_page(template_type): +def should_skip_template_page(db_template): return ( current_user.has_permissions('send_messages') and not current_user.has_permissions('manage_templates', 'manage_api_keys') - and template_type != 'letter' + and db_template['template_type'] != 'letter' + and not db_template['archived'] ) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 91697755c..93bac0a6c 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -587,6 +587,32 @@ def test_caseworker_redirected_to_set_sender_for_one_off( ) +@freeze_time('2020-01-01 15:00') +def test_caseworker_sees_template_page_if_template_is_deleted( + client_request, + mock_get_deleted_template, + fake_uuid, + mocker, + active_caseworking_user, +): + + mocker.patch('app.user_api_client.get_user', return_value=active_caseworking_user) + + template_id = fake_uuid + page = client_request.get( + '.view_template', + service_id=SERVICE_ONE_ID, + template_id=template_id, + _test_page_title=False, + ) + + content = str(page) + assert url_for("main.send_one_off", service_id=SERVICE_ONE_ID, template_id=fake_uuid) not in content + assert page.select('p.hint')[0].text.strip() == 'This template was deleted today at 3:00pm.' + + mock_get_deleted_template.assert_called_with(SERVICE_ONE_ID, template_id, None) + + def test_user_with_only_send_and_view_redirected_to_set_sender_for_one_off( client_request, mock_get_service_templates,