diff --git a/app/main/forms.py b/app/main/forms.py index 0ae859fea..79a935ae5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -447,6 +447,16 @@ class EmailTemplateForm(BaseTemplateForm): class LetterTemplateForm(EmailTemplateForm): + postage = RadioField( + 'Choose postage', + choices=[ + ('first', 'First class'), + ('second', 'Second class'), + ('None', "Service default"), + ], + validators=[DataRequired()], + default='None' + ) subject = TextAreaField( u'Main heading', diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 39583386b..92f1bedbf 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -297,6 +297,14 @@ def service_switch_can_edit_folders(service_id): return redirect(url_for('.service_settings', service_id=service_id)) +@main.route("/services//service-settings/can-choose-postage") +@login_required +@user_is_platform_admin +def service_switch_can_choose_postage(service_id): + current_service.switch_permission('choose_postage') + return redirect(url_for('.service_settings', service_id=service_id)) + + @main.route("/services//service-settings/archive", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_service') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index ec3c39a56..191cac718 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -76,6 +76,7 @@ def view_template(service_id, template_id): show_recipient=True, page_count=get_page_count_for_letter(template), ), + template_postage=template["postage"], default_letter_contact_block_id=default_letter_contact_block_id, ) @@ -570,21 +571,29 @@ def edit_service_template(service_id, template_id): template = service_api_client.get_service_template(service_id, template_id)['data'] template['template_content'] = template['content'] form = form_objects[template['template_type']](**template) - if form.validate_on_submit(): if form.process_type.data != template['process_type']: abort_403_if_not_admin_user() subject = form.subject.data if hasattr(form, 'subject') else None - new_template = get_template({ + + new_template_data = { 'name': form.name.data, 'content': form.template_content.data, 'subject': subject, 'template_type': template['template_type'], 'id': template['id'], 'process_type': form.process_type.data, - 'reply_to_text': template['reply_to_text'] - }, current_service) + 'reply_to_text': template['reply_to_text'], + } + if current_service.has_permission("choose_postage") and template["template_type"] == "letter": + postage = {"postage": form.postage.data} + else: + postage = {} + + new_template_data.update(postage) + + new_template = get_template(new_template_data, current_service) template_change = get_template(template, current_service).compare_to(new_template) if template_change.placeholders_added and not request.form.get('confirm'): example_column_headings = ( @@ -611,7 +620,8 @@ def edit_service_template(service_id, template_id): form.template_content.data, service_id, subject, - form.process_type.data + form.process_type.data, + postage=postage.get("postage") ) except HTTPError as e: if e.status_code == 400: @@ -642,6 +652,7 @@ def edit_service_template(service_id, template_id): return render_template( 'views/edit-{}-template.html'.format(template['template_type']), form=form, + can_choose_postage=current_service.has_permission("choose_postage"), template_id=template_id, template_type=template['template_type'], heading_action='Edit', diff --git a/app/navigation.py b/app/navigation.py index 2fb37551d..8cf8c514b 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -241,6 +241,7 @@ class HeaderNavigation(Navigation): 'service_set_sms_prefix', 'service_settings', 'service_sms_senders', + 'service_switch_can_choose_postage', 'service_switch_can_edit_folders', 'service_switch_can_send_email', 'service_switch_can_send_precompiled_letter', @@ -475,6 +476,7 @@ class MainNavigation(Navigation): 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_sms_sender', + 'service_switch_can_choose_postage', 'service_switch_can_edit_folders', 'service_switch_can_send_email', 'service_switch_can_send_precompiled_letter', @@ -702,6 +704,7 @@ class CaseworkNavigation(Navigation): 'service_set_sms_prefix', 'service_settings', 'service_sms_senders', + 'service_switch_can_choose_postage', 'service_switch_can_edit_folders', 'service_switch_can_send_email', 'service_switch_can_send_precompiled_letter', @@ -937,6 +940,7 @@ class OrgNavigation(Navigation): 'service_set_sms_prefix', 'service_settings', 'service_sms_senders', + 'service_switch_can_choose_postage', 'service_switch_can_edit_folders', 'service_switch_can_send_email', 'service_switch_can_send_precompiled_letter', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 7af14425f..78f19896c 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -153,7 +153,9 @@ class ServiceAPIClient(NotifyAdminAPIClient): @cache.delete('service-{service_id}-templates') @cache.delete('template-{id_}-version-None') @cache.delete('template-{id_}-versions') - def update_service_template(self, id_, name, type_, content, service_id, subject=None, process_type=None): + def update_service_template( + self, id_, name, type_, content, service_id, subject=None, process_type=None, postage=None + ): """ Update a service template. """ @@ -172,6 +174,14 @@ class ServiceAPIClient(NotifyAdminAPIClient): data.update({ 'process_type': process_type }) + if postage in ["first", "second"]: + data.update({ + 'postage': postage + }) + elif postage == 'None': + data.update({ + 'postage': None + }) data = _attach_current_user(data) endpoint = "/service/{0}/template/{1}".format(service_id, id_) return self.post(endpoint, data) diff --git a/app/templates/views/edit-letter-template.html b/app/templates/views/edit-letter-template.html index 4d0c62163..353ae4721 100644 --- a/app/templates/views/edit-letter-template.html +++ b/app/templates/views/edit-letter-template.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-footer.html" import page_footer %} +{% from "components/radios.html" import radios %} {% from "components/form.html" import form_wrapper %} {% block service_page_title %} @@ -17,6 +18,9 @@
{{ textbox(form.name, width='1-1', hint='Your recipients won’t see this', rows=10) }} + {% if can_choose_postage %} + {{ radios(form.postage, hint='Go to Settings to change default postage for your service') }} + {% endif %} {{ textbox(form.subject, width='1-1', highlight_tags=True, rows=2) }} {{ textbox(form.template_content, highlight_tags=True, width='1-1', rows=8) }} {{ page_footer( diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 5470d37f2..576ab201f 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -380,6 +380,11 @@ {{ 'Stop editing folders' if 'edit_folders' in current_service.permissions else 'Allow to edit folders' }} +
  • + + {{ 'Stop choosing postage per template' if 'choose_postage' in current_service.permissions else 'Allow to choose postage per template' }} + +
  • {% if current_service.active %}
  • diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index 333d35c1c..61b75a257 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -21,6 +21,13 @@ Send
  • +
    + {% if current_service.has_permission("choose_postage") and template_postage %} +

    Postage

    : {{ template_postage }} class + {% elif (current_service.has_permission("choose_postage") and not template_postage) or not current_service.has_permission("choose_postage") %} +

    Postage

    : {{ current_service.postage }} class + {% endif %} +
    {% endif %} {% else %} {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) %} diff --git a/tests/__init__.py b/tests/__init__.py index c65fb0996..458873f55 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -214,6 +214,7 @@ def template_json(service_id, reply_to=None, reply_to_text=None, is_precompiled_letter=False, + postage=None ): template = { 'id': id_, @@ -230,6 +231,7 @@ def template_json(service_id, 'reply_to_text': reply_to_text, 'is_precompiled_letter': is_precompiled_letter, 'folder': None, + 'postage': postage } if content is None: template['content'] = "template content" diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index 03317ce35..7403082e9 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -48,6 +48,15 @@ def get_service_settings_page( ({'permissions': ['edit_folders']}, '.service_switch_can_edit_folders', {}, 'Stop editing folders'), ({'permissions': []}, '.service_switch_can_edit_folders', {}, 'Allow to edit folders'), + + ( + {'permissions': ['choose_postage']}, + '.service_switch_can_choose_postage', + {}, + 'Stop choosing postage per template' + ), + ({'permissions': []}, '.service_switch_can_choose_postage', {}, 'Allow to choose postage per template'), + ({'permissions': ['sms']}, '.service_set_inbound_number', {'set_inbound_sms': True}, 'Allow inbound sms'), ({'active': True}, '.archive_service', {}, 'Archive service'), diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 09dee0ec0..d05edb241 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,3 +1,4 @@ +import re from datetime import datetime from unittest.mock import ANY, Mock @@ -401,6 +402,127 @@ def test_user_with_only_send_and_view_sees_letter_page( assert page.select_one('h1').text.strip() == 'Two week reminder' +@pytest.mark.parametrize("permissions,template_postage,expected_result", [ + (["choose_postage", "letter"], "first", "first"), + (["choose_postage", "letter"], None, "second"), + (["letter"], "first", "second"), +]) +def test_view_letter_template_displays_postage_dynamically_based_on_service_permissions_and_template_postage( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + single_letter_contact_block, + mock_has_jobs, + active_user_with_permissions, + mocker, + fake_uuid, + permissions, + template_postage, + expected_result +): + mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1) + service_one['permissions'] = permissions + client_request.login(active_user_with_permissions) + mock_get_service_letter_template(mocker, postage=template_postage) + page = client_request.get( + 'main.view_template', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + ) + + assert "Postage: {} class".format(expected_result) in page.text + + +def test_view_non_letter_template_does_not_display_postage( + logged_in_client, + mock_get_service_template, + mock_get_template_folders, + service_one, + fake_uuid, +): + + response = logged_in_client.get(url_for( + '.view_template', + service_id=service_one['id'], + template_id=fake_uuid)) + + assert response.status_code == 200 + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert "Postage" not in page.text + + +@pytest.mark.parametrize("permissions, expected_result", [ + (["choose_postage", "letter"], True), + (["letter"], False), +]) +def test_edit_letter_templates_postage_choice_visibility_and_default( + client_request, + service_one, + active_user_with_permissions, + mocker, + fake_uuid, + permissions, + expected_result +): + mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1) + service_one['permissions'] = permissions + client_request.login(active_user_with_permissions) + mock_get_service_letter_template(mocker) + page = client_request.get( + 'main.edit_service_template', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + ) + assert bool(page.find(string=re.compile("Choose postage"))) is expected_result + + if expected_result: + assert page.select('input[checked]')[0].attrs["value"] == 'None' + + +@pytest.mark.parametrize("permissions, expected_result", [ + (["choose_postage", "letter"], "first"), + (["letter"], None), +]) +def test_edit_letter_templates_postage_permissions( + logged_in_client, + service_one, + mocker, + fake_uuid, + mock_update_service_template, + permissions, + expected_result +): + service_one['permissions'] = permissions + mock_get_service_letter_template(mocker) + template_id = fake_uuid + + logged_in_client.post( + url_for( + 'main.edit_service_template', + service_id=SERVICE_ONE_ID, + template_id=template_id + ), + data={ + 'name': 'Two week reminder', + 'template_content': "Some content", + 'subject': 'Subject', + 'postage': 'first' + } + ) + mock_update_service_template.assert_called_with( + template_id, + 'Two week reminder', + 'letter', + "Some content", + SERVICE_ONE_ID, + 'Subject', + 'normal', + postage=expected_result + ) + + @pytest.mark.parametrize('permissions, links_to_be_shown, permissions_warning_to_be_shown', [ ( ['view_activity'], @@ -915,7 +1037,7 @@ def test_should_redirect_when_saving_a_template( assert response.location == url_for( '.view_template', service_id=service['id'], template_id=template_id, _external=True) mock_update_service_template.assert_called_with( - template_id, name, 'sms', content, service['id'], None, 'normal') + template_id, name, 'sms', content, service['id'], None, 'normal', postage=None) def test_should_edit_content_when_process_type_is_priority_not_platform_admin( @@ -951,7 +1073,8 @@ def test_should_edit_content_when_process_type_is_priority_not_platform_admin( "new template content with & entity", service['id'], None, - 'priority' + 'priority', + postage=None ) @@ -1037,9 +1160,10 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p mock_update_service_template.called == 0 -@pytest.mark.parametrize('template_mock, expected_paragraphs', [ +@pytest.mark.parametrize('template_mock, template_type, expected_paragraphs', [ ( mock_get_service_email_template, + "email", [ 'You removed ((date))', 'You added ((name))', @@ -1048,6 +1172,7 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p ), ( mock_get_service_letter_template, + "letter", [ 'You removed ((date))', 'You added ((name))', @@ -1067,6 +1192,7 @@ def test_should_show_interstitial_when_making_breaking_change( fake_uuid, mocker, template_mock, + template_type, expected_paragraphs, ): template_mock( @@ -1076,17 +1202,22 @@ def test_should_show_interstitial_when_making_breaking_change( ) service_id = fake_uuid template_id = fake_uuid + data = { + 'id': template_id, + 'name': "new name", + 'template_content': "hello lets talk about ((thing))", + 'template_type': template_type, + 'subject': 'reminder \'" & ((name))', + 'service': service_id, + 'process_type': 'normal' + } + + if template_type == "letter": + data["postage"] = 'None' + response = logged_in_client.post( url_for('.edit_service_template', service_id=service_id, template_id=template_id), - data={ - 'id': template_id, - 'name': "new name", - 'template_content': "hello lets talk about ((thing))", - 'template_type': 'email', - 'subject': 'reminder \'" & ((name))', - 'service': service_id, - 'process_type': 'normal' - } + data=data ) assert response.status_code == 200 @@ -1232,7 +1363,7 @@ def test_should_redirect_when_saving_a_template_email( template_id=template_id, _external=True) mock_update_service_template.assert_called_with( - template_id, name, 'email', content, service_id, subject, 'normal') + template_id, name, 'email', content, service_id, subject, 'normal', postage=None) def test_should_show_delete_template_page_with_time_block( diff --git a/tests/conftest.py b/tests/conftest.py index 282cd22f2..db5389bb0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -880,8 +880,8 @@ def mock_get_service_email_template_without_placeholders(mocker): @pytest.fixture(scope='function') -def mock_get_service_letter_template(mocker, content=None, subject=None): - def _get(service_id, template_id, version=None): +def mock_get_service_letter_template(mocker, content=None, subject=None, postage=None): + def _get(service_id, template_id, version=None, postage=postage): template = template_json( service_id, template_id, @@ -889,6 +889,7 @@ def mock_get_service_letter_template(mocker, content=None, subject=None): "letter", content or "Template content with & entity", subject or "Subject", + postage=postage, ) return {'data': template} @@ -910,8 +911,8 @@ def mock_create_service_template(mocker, fake_uuid): @pytest.fixture(scope='function') def mock_update_service_template(mocker): - def _update(id_, name, type_, content, service, subject=None, process_type=None): - template = template_json(service, id_, name, type_, content, subject, process_type) + def _update(id_, name, type_, content, service, subject=None, process_type=None, postage=None): + template = template_json(service, id_, name, type_, content, subject, process_type, postage) return {'data': template} return mocker.patch( @@ -939,7 +940,7 @@ def mock_create_service_template_content_too_big(mocker): @pytest.fixture(scope='function') def mock_update_service_template_400_content_too_big(mocker): - def _update(id_, name, type_, content, service, subject=None, process_type=None): + def _update(id_, name, type_, content, service, subject=None, process_type=None, postage=None): json_mock = Mock(return_value={ 'message': {'content': ["Content has a character count greater than the limit of 459"]}, 'result': 'error'