From db5378bac2ef53afafe9f0d3234c581eca354ed6 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 29 Jan 2019 15:43:59 +0000 Subject: [PATCH] Edit template postage from a separate view --- app/main/forms.py | 11 +++ app/main/views/templates.py | 36 +++++++-- app/navigation.py | 4 + app/notify_client/service_api_client.py | 9 +++ .../partials/templates/guidance-postage.html | 16 ++++ .../templates/edit-template-postage.html | 29 +++++++ tests/app/main/views/test_templates.py | 75 +++++++++---------- .../notify_client/test_service_api_client.py | 5 ++ tests/conftest.py | 2 +- 9 files changed, 140 insertions(+), 47 deletions(-) create mode 100644 app/templates/partials/templates/guidance-postage.html create mode 100644 app/templates/views/templates/edit-template-postage.html diff --git a/app/main/forms.py b/app/main/forms.py index e79c4d538..7ebf99f8b 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -471,6 +471,17 @@ class LetterTemplateForm(EmailTemplateForm): ) +class LetterTemplatePostageForm(StripWhitespaceForm): + postage = RadioField( + 'Choose the postage for this letter template', + choices=[ + ('first', 'First class'), + ('second', 'Second class'), + ], + validators=[DataRequired()] + ) + + class ForgotPasswordForm(StripWhitespaceForm): email_address = email_address(gov_user=False) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 261f578ff..195d291e6 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -21,6 +21,7 @@ from app.main.forms import ( ChooseTemplateType, EmailTemplateForm, LetterTemplateForm, + LetterTemplatePostageForm, SearchTemplatesForm, SetTemplateSenderForm, SMSTemplateForm, @@ -268,8 +269,7 @@ def add_template_by_type(service_id, template_folder_id=None): form = ChooseTemplateType( include_letters=current_service.has_permission('letter'), include_copy=( - current_service.all_templates or - len(user_api_client.get_service_ids_for_user(current_user)) > 1 + current_service.all_templates or len(user_api_client.get_service_ids_for_user(current_user)) > 1 ), include_folder=current_service.has_permission('edit_folders') ) @@ -558,9 +558,9 @@ def add_service_template(service_id, template_type, template_folder_id=None): ) except HTTPError as e: if ( - e.status_code == 400 and - 'content' in e.message and - any(['character count greater than' in x for x in e.message['content']]) + e.status_code == 400 + and 'content' in e.message + and any(['character count greater than' in x for x in e.message['content']]) ): form.template_content.errors.extend(e.message['content']) else: @@ -625,8 +625,7 @@ def edit_service_template(service_id, template_id): 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 = ( - first_column_headings[new_template.template_type] + - list(new_template.placeholders) + first_column_headings[new_template.template_type] + list(new_template.placeholders) ) return render_template( 'views/templates/breaking-change.html', @@ -833,6 +832,29 @@ def set_template_sender(service_id, template_id): ) +@main.route('/services//templates//edit-postage', methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_templates') +def edit_template_postage(service_id, template_id): + template = service_api_client.get_service_template(service_id, template_id)['data'] + if template["template_type"] != "letter" or not current_service.has_permission("choose_postage"): + abort(404) + form = LetterTemplatePostageForm(**template) + if form.validate_on_submit(): + postage = form.postage.data + service_api_client.update_service_template_postage(service_id, template_id, postage) + + return redirect(url_for('.view_template', service_id=service_id, template_id=template_id)) + + return render_template( + 'views/templates/edit-template-postage.html', + form=form, + service_id=service_id, + template_id=template_id, + template_postage=template["postage"] + ) + + def get_template_sender_form_dict(service_id, template): context = { 'email': { diff --git a/app/navigation.py b/app/navigation.py index 22861f562..0df32b119 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -153,6 +153,7 @@ class HeaderNavigation(Navigation): 'edit_organisation_name', 'edit_provider', 'edit_service_template', + 'edit_template_postage', 'edit_user_org_permissions', 'edit_user_permissions', 'email_not_received', @@ -309,6 +310,7 @@ class MainNavigation(Navigation): 'copy_template', 'delete_service_template', 'edit_service_template', + 'edit_template_postage', 'manage_template_folder', 'send_messages', 'send_one_off', @@ -598,6 +600,7 @@ class CaseworkNavigation(Navigation): 'edit_organisation_name', 'edit_provider', 'edit_service_template', + 'edit_template_postage', 'edit_user_org_permissions', 'edit_user_permissions', 'email_branding', @@ -832,6 +835,7 @@ class OrgNavigation(Navigation): 'edit_data_retention', 'edit_provider', 'edit_service_template', + 'edit_template_postage', 'edit_user_permissions', 'email_branding', 'email_not_received', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 9f54b0189..9c1f25603 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -206,6 +206,15 @@ class ServiceAPIClient(NotifyAdminAPIClient): data ) + @cache.delete('service-{service_id}-templates') + @cache.delete('template-{template_id}-version-None') + @cache.delete('template-{template_id}-versions') + def update_service_template_postage(self, service_id, template_id, postage): + return self.post( + "/service/{0}/template/{1}".format(service_id, template_id), + _attach_current_user({'postage': postage}) + ) + @cache.set('template-{template_id}-version-{version}') def get_service_template(self, service_id, template_id, version=None): """ diff --git a/app/templates/partials/templates/guidance-postage.html b/app/templates/partials/templates/guidance-postage.html new file mode 100644 index 000000000..51557f113 --- /dev/null +++ b/app/templates/partials/templates/guidance-postage.html @@ -0,0 +1,16 @@ +

+ Delivery times +

+

+ Letters sent before 5:30pm are dispatched the next working day (Monday to Friday). +

+

+ First class letters are delivered one day after they're dispatched. Second class letters are delivered 2 days after they're dispatched. +

+

+ Royal Mail delivers from Monday to Saturday, excluding bank holidays. +

+ See a list of postage prices. +

+ Back to template +

diff --git a/app/templates/views/templates/edit-template-postage.html b/app/templates/views/templates/edit-template-postage.html new file mode 100644 index 000000000..3dd69457c --- /dev/null +++ b/app/templates/views/templates/edit-template-postage.html @@ -0,0 +1,29 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import sticky_page_footer %} +{% from "components/radios.html" import radios %} +{% from "components/form.html" import form_wrapper %} + +{% block service_page_title %} + Edit postage +{% endblock %} + +{% block maincolumn_content %} + +

+ Edit postage +

+ {% call form_wrapper() %} +
+
+ {{ radios(form.postage) }} + {{ sticky_page_footer( + 'Save' + ) }} +
+ +
+ {% endcall %} + +{% endblock %} diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index cbb1930ab..13619541a 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,4 +1,3 @@ -import re from datetime import datetime from unittest.mock import ANY, Mock @@ -486,73 +485,71 @@ def test_view_non_letter_template_does_not_display_postage( 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( +def test_edit_letter_template_postage_page_displays_correctly( 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 + service_one['permissions'] = ["choose_postage", "letter"] client_request.login(active_user_with_permissions) mock_get_service_letter_template(mocker) page = client_request.get( - 'main.edit_service_template', + 'main.edit_template_postage', 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' + assert page.select_one('h1').text.strip() == 'Edit postage' + assert page.select('input[checked]')[0].attrs["value"] == 'second' -@pytest.mark.parametrize("permissions, expected_result", [ - (["choose_postage", "letter"], "first"), - (["letter"], None), -]) -def test_edit_letter_templates_postage_permissions( +def test_edit_letter_template_postage_page_404s_if_template_is_not_a_letter( + client_request, + service_one, + mock_get_service_template, + active_user_with_permissions, + mocker, + fake_uuid, +): + service_one['permissions'] = ["choose_postage", "letter"] + client_request.login(active_user_with_permissions) + page = client_request.get( + 'main.edit_template_postage', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _expected_status=404 + ) + + assert page.select_one('h1').text.strip() != 'Edit postage' + + +def test_edit_letter_templates_postage_updates_postage( logged_in_client, service_one, mocker, - fake_uuid, - mock_update_service_template, - permissions, - expected_result + fake_uuid ): - service_one['permissions'] = permissions + mock_update_template_postage = mocker.patch( + 'app.main.views.templates.service_api_client.update_service_template_postage' + ) + service_one['permissions'] = ["choose_postage", "letter"] mock_get_service_letter_template(mocker) template_id = fake_uuid logged_in_client.post( url_for( - 'main.edit_service_template', + 'main.edit_template_postage', service_id=SERVICE_ONE_ID, template_id=template_id ), - data={ - 'name': 'Two week reminder', - 'template_content': "Some content", - 'subject': 'Subject', - 'postage': 'first' - } + data={'postage': 'first'} ) - mock_update_service_template.assert_called_with( - template_id, - 'Two week reminder', - 'letter', - "Some content", + mock_update_template_postage.assert_called_with( SERVICE_ONE_ID, - 'Subject', - 'normal', - postage=expected_result + template_id, + "first" ) diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 41dffba7d..b5dd793a2 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -401,6 +401,11 @@ def test_deletes_service_cache( 'template-{}-version-None'.format(FAKE_TEMPLATE_ID), 'template-{}-versions'.format(FAKE_TEMPLATE_ID), ]), + ('update_service_template_postage', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'first'], [ + 'service-{}-templates'.format(SERVICE_ONE_ID), + 'template-{}-version-None'.format(FAKE_TEMPLATE_ID), + 'template-{}-versions'.format(FAKE_TEMPLATE_ID), + ]), ('delete_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [ 'service-{}-templates'.format(SERVICE_ONE_ID), 'template-{}-version-None'.format(FAKE_TEMPLATE_ID), diff --git a/tests/conftest.py b/tests/conftest.py index 14de9ce8b..e2f898515 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -885,7 +885,7 @@ def mock_get_service_email_template_without_placeholders(mocker): @pytest.fixture(scope='function') -def mock_get_service_letter_template(mocker, content=None, subject=None, postage=None): +def mock_get_service_letter_template(mocker, content=None, subject=None, postage='second'): def _get(service_id, template_id, version=None, postage=postage): template = template_json( service_id,