From a60d26990021a7a2cfa541a4cea76ded5d71071a Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 18 Dec 2018 15:43:36 +0000 Subject: [PATCH 01/10] Show postage on template page if service can choose postage and postage set on the template. --- app/main/views/templates.py | 1 + app/templates/views/templates/_template.html | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 151f301de..8c2fda7ab 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, ) 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) %} From 687e9e5866e43585b4978448ba7de9dfe05b59c2 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 18 Dec 2018 18:22:03 +0000 Subject: [PATCH 02/10] Change postage while editing template --- app/main/forms.py | 10 +++++++++ app/main/views/templates.py | 22 +++++++++++++++---- app/notify_client/service_api_client.py | 6 ++++- app/templates/views/edit-letter-template.html | 4 ++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index ccb11dae5..b8f95bab4 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'), + ('service_default', "Service default"), + ], + validators=[DataRequired()], + default='service_default' + ) subject = TextAreaField( u'Main heading', diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 8c2fda7ab..998e8971e 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -571,15 +571,27 @@ def edit_service_template(service_id, template_id): 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" + ) and form.postage.data in ["first", "second"]: + 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 = ( @@ -606,7 +618,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: @@ -637,6 +650,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/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 7af14425f..75485e745 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -153,7 +153,7 @@ 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 +172,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): data.update({ 'process_type': process_type }) + if postage: + data.update({ + 'postage': postage + }) 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( From 695f1150b5843c6eec5eb2e194958c5eebd4e8c1 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 19 Dec 2018 10:58:35 +0000 Subject: [PATCH 03/10] service_default postage resets template postage to None --- app/main/views/templates.py | 7 ++----- app/notify_client/service_api_client.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 998e8971e..767b6220f 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -581,11 +581,8 @@ def edit_service_template(service_id, template_id): 'process_type': form.process_type.data, 'reply_to_text': template['reply_to_text'], } - if ( - current_service.has_permission("choose_postage") and template["template_type"] == "letter" - ) and form.postage.data in ["first", "second"]: - postage = {"postage": form.postage.data } - + if current_service.has_permission("choose_postage") and template["template_type"] == "letter": + postage = {"postage": form.postage.data} else: postage = {} diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 75485e745..e74420413 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, postage=None): + def update_service_template( + self, id_, name, type_, content, service_id, subject=None, process_type=None, postage=None + ): """ Update a service template. """ @@ -172,10 +174,14 @@ class ServiceAPIClient(NotifyAdminAPIClient): data.update({ 'process_type': process_type }) - if postage: + if postage in ["first", "second"]: data.update({ 'postage': postage }) + elif postage == "service_default": + data.update({ + 'postage': None + }) data = _attach_current_user(data) endpoint = "/service/{0}/template/{1}".format(service_id, id_) return self.post(endpoint, data) From e1191326f443081f314e392a2e3120ca4906bab1 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 19 Dec 2018 17:06:09 +0000 Subject: [PATCH 04/10] Fix tests after enabling editing of postage on letter templates --- tests/__init__.py | 2 ++ tests/app/main/views/test_templates.py | 35 ++++++++++++++++---------- tests/conftest.py | 9 ++++--- 3 files changed, 29 insertions(+), 17 deletions(-) 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/test_templates.py b/tests/app/main/views/test_templates.py index 09dee0ec0..6b438b568 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -915,7 +915,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 +951,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 +1038,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 +1050,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 +1070,7 @@ def test_should_show_interstitial_when_making_breaking_change( fake_uuid, mocker, template_mock, + template_type, expected_paragraphs, ): template_mock( @@ -1076,17 +1080,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.update({"postage": "service_default"}) + 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 +1241,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..931a7bc69 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -881,7 +881,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): - def _get(service_id, template_id, version=None): + def _get(service_id, template_id, version=None, postage=None): 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' From 85b8b343e21c6bf81a75429d5836aea73f8a4652 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 20 Dec 2018 18:09:00 +0000 Subject: [PATCH 05/10] Service deafault radio checked by default, existing tests pass. --- app/main/forms.py | 4 ++-- app/notify_client/service_api_client.py | 2 +- tests/app/main/views/test_templates.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index b8f95bab4..7a45459ff 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -452,10 +452,10 @@ class LetterTemplateForm(EmailTemplateForm): choices=[ ('first', 'First class'), ('second', 'Second class'), - ('service_default', "Service default"), + ('None', "Service default"), ], validators=[DataRequired()], - default='service_default' + default='None' ) subject = TextAreaField( diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e74420413..78f19896c 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -178,7 +178,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): data.update({ 'postage': postage }) - elif postage == "service_default": + elif postage == 'None': data.update({ 'postage': None }) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 6b438b568..a7a715566 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1091,7 +1091,7 @@ def test_should_show_interstitial_when_making_breaking_change( } if template_type == "letter": - data.update({"postage": "service_default"}) + data["postage"] = 'None' response = logged_in_client.post( url_for('.edit_service_template', service_id=service_id, template_id=template_id), From 5144db7baaa40b52671cc12bec8f31d0eaac6de0 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 21 Dec 2018 12:51:05 +0000 Subject: [PATCH 06/10] Test postage display on view template page --- tests/app/main/views/test_templates.py | 67 ++++++++++++++++++++++++++ tests/conftest.py | 4 +- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index a7a715566..8528540ae 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -12,6 +12,7 @@ from app.main.views.templates import ( get_last_use_message, ) from tests import ( + service_json, single_notification_json, template_json, validate_route_permission, @@ -401,6 +402,72 @@ def test_user_with_only_send_and_view_sees_letter_page( assert page.select_one('h1').text.strip() == 'Two week reminder' +def test_view_letter_template_displays_postage( + client_request, + mock_get_service_templates, + mock_get_template_folders, + mock_get_service_letter_template, + single_letter_contact_block, + mock_has_jobs, + active_user_with_permissions, + mocker, + fake_uuid, +): + mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1) + client_request.login(active_user_with_permissions) + import pdb; pdb.set_trace() + page = client_request.get( + 'main.view_template', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + ) + + assert "Postage: second class" in page.text + + +def test_view_letter_template_displays_postage_from_template_if_service_has_choose_postage_permission( + 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, +): + mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1) + service_one['permissions'] = ["choose_postage", "letter"] + client_request.login(active_user_with_permissions) + mock_get_service_letter_template(mocker, postage="first") + page = client_request.get( + 'main.view_template', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + ) + + assert "Postage: first class" 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, links_to_be_shown, permissions_warning_to_be_shown', [ ( ['view_activity'], diff --git a/tests/conftest.py b/tests/conftest.py index 931a7bc69..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, postage=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, From cbead5d6652118887d0bfbc0febdf16042f325bd Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 21 Dec 2018 14:29:35 +0000 Subject: [PATCH 07/10] Refactor tests for displaying template postage --- tests/app/main/views/test_templates.py | 39 ++++++++------------------ 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 8528540ae..2beaa4d0e 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -402,30 +402,12 @@ def test_user_with_only_send_and_view_sees_letter_page( assert page.select_one('h1').text.strip() == 'Two week reminder' -def test_view_letter_template_displays_postage( - client_request, - mock_get_service_templates, - mock_get_template_folders, - mock_get_service_letter_template, - single_letter_contact_block, - mock_has_jobs, - active_user_with_permissions, - mocker, - fake_uuid, -): - mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1) - client_request.login(active_user_with_permissions) - import pdb; pdb.set_trace() - page = client_request.get( - 'main.view_template', - service_id=SERVICE_ONE_ID, - template_id=fake_uuid, - ) - - assert "Postage: second class" in page.text - - -def test_view_letter_template_displays_postage_from_template_if_service_has_choose_postage_permission( +@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, @@ -435,18 +417,21 @@ def test_view_letter_template_displays_postage_from_template_if_service_has_choo 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'] = ["choose_postage", "letter"] + service_one['permissions'] = permissions client_request.login(active_user_with_permissions) - mock_get_service_letter_template(mocker, postage="first") + 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: first class" in page.text + assert "Postage: {} class".format(expected_result) in page.text def test_view_non_letter_template_does_not_display_postage( From bc1e0b71670725993ada0d81b43d7b025f7977a5 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 21 Dec 2018 14:50:07 +0000 Subject: [PATCH 08/10] Test choose postage section display when editing letter template --- tests/app/main/views/test_templates.py | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 2beaa4d0e..f63f598a7 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -2,6 +2,7 @@ from datetime import datetime from unittest.mock import ANY, Mock import pytest +import re from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time @@ -453,6 +454,34 @@ 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( + 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, links_to_be_shown, permissions_warning_to_be_shown', [ ( ['view_activity'], From 50935e79caed86ec389386f15dbb9e96905492fe Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 21 Dec 2018 15:51:24 +0000 Subject: [PATCH 09/10] Test mock_update_service_template called with right args --- app/main/views/templates.py | 1 - tests/app/main/views/test_templates.py | 45 ++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 767b6220f..90239e8c3 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -565,7 +565,6 @@ 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() diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index f63f598a7..d05edb241 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,8 +1,8 @@ +import re from datetime import datetime from unittest.mock import ANY, Mock import pytest -import re from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time @@ -13,7 +13,6 @@ from app.main.views.templates import ( get_last_use_message, ) from tests import ( - service_json, single_notification_json, template_json, validate_route_permission, @@ -482,6 +481,48 @@ def test_edit_letter_templates_postage_choice_visibility_and_default( 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'], From 97058d3c5b248124c1f0a48839da53cbabedd478 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 27 Dec 2018 17:29:21 +0000 Subject: [PATCH 10/10] Add service setting switch to choose postage per template --- app/main/views/service_settings.py | 8 ++++++++ app/navigation.py | 4 ++++ app/templates/views/service-settings.html | 5 +++++ .../service_settings/test_service_setting_permissions.py | 9 +++++++++ 4 files changed, 26 insertions(+) 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/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/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/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'),