From 7d7b790dc1a9864cb35c48f563d16c72902b64ef Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 11 Feb 2019 16:09:08 +0000 Subject: [PATCH] Remove add template as separate page This has moved to the choose template page. --- app/main/forms.py | 22 ------ app/main/views/templates.py | 20 ----- app/navigation.py | 4 - .../views/templates/action_blocked.html | 4 +- tests/app/main/views/test_letters.py | 17 ++-- tests/app/main/views/test_template_folders.py | 78 ------------------- tests/app/main/views/test_templates.py | 38 ++------- 7 files changed, 17 insertions(+), 166 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 1a1eea733..399705c24 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -938,28 +938,6 @@ class DateFilterForm(StripWhitespaceForm): include_from_test_key = BooleanField("Include test keys", default="checked", false_values={"N"}) -class ChooseTemplateType(StripWhitespaceForm): - - template_type = RadioField( - '', - validators=[ - DataRequired() - ] - ) - - def __init__(self, include_letters=False, include_copy=False, *args, **kwargs): - - super().__init__(*args, **kwargs) - - self.template_type.choices = filter(None, [ - ('email', 'Email template'), - ('sms', 'Text message template'), - ('letter', 'Letter template') if include_letters else None, - ('copy-existing', 'Copy of an existing template') if include_copy else None, - ('folder', 'Folder'), - ]) - - class SearchByNameForm(StripWhitespaceForm): search = SearchField('Search by name') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 2dd94f9b6..62a79f182 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -18,7 +18,6 @@ from app import ( ) from app.main import main from app.main.forms import ( - ChooseTemplateType, EmailTemplateForm, LetterTemplateForm, LetterTemplatePostageForm, @@ -252,25 +251,6 @@ def view_template_version_preview(service_id, template_id, version, filetype): return TemplatePreview.from_database_object(db_template, filetype) -@main.route("/services//templates/add", methods=['GET', 'POST']) -@main.route("/services//templates/folders//add", methods=['GET', 'POST']) -@login_required -@user_has_permissions('manage_templates') -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 - ), - ) - - if form.validate_on_submit(): - return _add_template_by_type(form.template_type.data, template_folder_id) - - return render_template('views/templates/add.html', form=form) - - def _add_template_by_type(template_type, template_folder_id): if template_type == 'copy-existing': diff --git a/app/navigation.py b/app/navigation.py index 6ba3117e1..616a76326 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -110,7 +110,6 @@ class HeaderNavigation(Navigation): 'add_data_retention', 'add_service', 'add_service_template', - 'add_template_by_type', 'add_template_folder', 'agreement', 'api_callbacks', @@ -299,7 +298,6 @@ class MainNavigation(Navigation): 'templates': { 'action_blocked', 'add_service_template', - 'add_template_by_type', 'add_template_folder', 'check_messages', 'check_notification', @@ -553,7 +551,6 @@ class CaseworkNavigation(Navigation): 'add_organisation', 'add_service', 'add_service_template', - 'add_template_by_type', 'add_template_folder', 'agreement', 'api_callbacks', @@ -789,7 +786,6 @@ class OrgNavigation(Navigation): 'add_organisation', 'add_service', 'add_service_template', - 'add_template_by_type', 'add_template_folder', 'agreement', 'api_callbacks', diff --git a/app/templates/views/templates/action_blocked.html b/app/templates/views/templates/action_blocked.html index 91190bef6..a9b52d72d 100644 --- a/app/templates/views/templates/action_blocked.html +++ b/app/templates/views/templates/action_blocked.html @@ -15,14 +15,14 @@ Sending {{ notification_type }} has been disabled for your service.

- If you need to send {{ notification_type }} + If you need to send {{ notification_type }} get in touch with the GOV.UK Notify team.

{% set back_link_dict = { 'add_new_template': { - 'url' : '.add_template_by_type', 'text': 'Back to add new template' + 'url' : '.choose_template', 'text': 'Back to templates' }, 'templates': { 'url' : '.choose_template', 'text' : 'Back to templates' diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index b2ad74a28..3711b9623 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -1,7 +1,6 @@ from functools import partial import pytest -from bs4 import BeautifulSoup from flask import url_for letters_urls = [ @@ -56,31 +55,29 @@ def test_letters_lets_in_without_permission( @pytest.mark.parametrize('permissions, choices', [ ( ['email', 'sms', 'letter'], - ['Email template', 'Text message template', 'Letter template', 'Copy of an existing template', 'Folder'] + ['Email template', 'Text message template', 'Letter template', 'Copy of an existing template'] ), ( ['email', 'sms'], - ['Email template', 'Text message template', 'Copy of an existing template', 'Folder'] + ['Email template', 'Text message template', 'Copy of an existing template'] ), ]) def test_given_option_to_add_letters_if_allowed( - logged_in_client, + client_request, service_one, mocker, mock_get_service_templates, + mock_get_template_folders, mock_get_organisations_and_services_for_user, permissions, choices, ): service_one['permissions'] = permissions - mocker.patch('app.service_api_client.get_service', return_value={"data": service_one}) - response = logged_in_client.get(url_for('main.add_template_by_type', service_id=service_one['id'])) + page = client_request.get('main.choose_template', service_id=service_one['id']) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - radios = page.select('input[type=radio]') - labels = page.select('label') + radios = page.select('#add_new_template_form input[type=radio]') + labels = page.select('#add_new_template_form label') assert len(radios) == len(choices) assert len(labels) == len(choices) diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 06239c0cd..b1a90c8e2 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -37,36 +37,6 @@ def _template(template_type, name, parent=None, template_id=None): } -@pytest.mark.parametrize('parent_folder_id', [None, PARENT_FOLDER_ID]) -def test_add_page_shows_option_for_folder( - client_request, - service_one, - parent_folder_id, - mocker, - mock_get_service_templates, - mock_get_organisations_and_services_for_user, -): - mocker.patch('app.service_api_client.get_service', return_value={"data": service_one}) - - page = client_request.get( - 'main.add_template_by_type', - service_id=service_one['id'], - template_folder_id=parent_folder_id, - _test_page_title=False - ) - - radios = page.select('input[type=radio]') - labels = page.select('label') - - assert [x['value'] for x in radios] == ['email', 'sms', 'copy-existing', 'folder'] - assert [x.text.strip() for x in labels] == [ - 'Email template', - 'Text message template', - 'Copy of an existing template', - 'Folder' - ] - - @pytest.mark.parametrize('parent_folder_id', [None, PARENT_FOLDER_ID]) def test_get_add_template_folder_page(client_request, service_one, parent_folder_id): @@ -437,54 +407,6 @@ def test_should_show_templates_folder_page( mock_get_service_templates.assert_called_once_with(SERVICE_ONE_ID) -@pytest.mark.parametrize("template_type", ["email", "sms"]) -def test_add_template_by_type_should_redirect_to_add_service_template( - client_request, - service_one, - template_type, - mock_get_service_templates, - mock_get_organisations_and_services_for_user, -): - client_request.post( - 'main.add_template_by_type', - service_id=SERVICE_ONE_ID, - template_folder_id=PARENT_FOLDER_ID, - _data={'template_type': template_type}, - _expected_redirect=url_for('main.add_service_template', - service_id=SERVICE_ONE_ID, - template_type=template_type, - template_folder_id=PARENT_FOLDER_ID, - _external=True), - ) - - -def test_add_template_by_type_should_redirect_to_view_template_for_letter( - client_request, - service_one, - mock_get_service_templates, - mock_get_organisations_and_services_for_user, - mock_create_service_template -): - service_one['permissions'] += ['letter'] - client_request.post( - 'main.add_template_by_type', - service_id=SERVICE_ONE_ID, - template_folder_id=PARENT_FOLDER_ID, - _data={'template_type': 'letter'}, - _expected_redirect=url_for('main.view_template', - service_id=SERVICE_ONE_ID, - template_id='Untitled', - _external=True), - ) - mock_create_service_template.assert_called_once_with('Untitled', - 'letter', - 'Body', - SERVICE_ONE_ID, - 'Main heading', - 'normal', - PARENT_FOLDER_ID) - - def test_can_create_email_template_with_parent_folder( client_request, mock_create_service_template diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index a33a6e635..499dd1c0e 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -743,28 +743,20 @@ def test_dont_show_preview_letter_templates_for_bad_filetype( assert mock_get_service_template.called is False -@pytest.mark.parametrize('endpoint, data', ( - ('main.add_template_by_type', { - 'template_type': 'copy-existing' - }), - ('main.choose_template', { - 'operation': 'add-new-template', - 'add_template_by_template_type': 'copy-existing' - }), -)) def test_choosing_to_copy_redirects( client_request, service_one, mock_get_service_templates, mock_get_template_folders, mock_get_organisations_and_services_for_user, - endpoint, - data, ): client_request.post( - endpoint, + 'main.choose_template', service_id=SERVICE_ONE_ID, - _data=data, + _data={ + 'operation': 'add-new-template', + 'add_template_by_template_type': 'copy-existing' + }, _expected_status=302, _expected_redirect=url_for( 'main.choose_template_to_copy', @@ -1158,20 +1150,6 @@ def test_cant_copy_template_from_non_member_service( @pytest.mark.parametrize('endpoint, data, expected_error', ( - ( - 'main.add_template_by_type', - { - 'template_type': 'email', - }, - "Sending emails has been disabled for your service." - ), - ( - 'main.add_template_by_type', - { - 'template_type': 'sms', - }, - "Sending text messages has been disabled for your service." - ), ( 'main.choose_template', { @@ -1207,10 +1185,10 @@ def test_should_not_allow_creation_of_template_through_form_without_correct_perm _follow_redirects=True, ) assert normalize_spaces(page.select('main p')[0].text) == expected_error - assert page.select(".page-footer-back-link")[0].text == "Back to add new template" + assert page.select(".page-footer-back-link")[0].text == "Back to templates" assert page.select(".page-footer-back-link")[0]['href'] == url_for( - '.add_template_by_type', - service_id=service_one['id'], + '.choose_template', + service_id=SERVICE_ONE_ID, template_id='0', )