From 7cbf5de240068fc127b6b2a4a5c19d3eada254f3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Nov 2018 15:33:09 +0000 Subject: [PATCH 01/10] add new template folder The add new templates page now has option to add template folders. Tweaked wording of other options and h1 to clarify options since it's not all about templates any more. Added api client and stuff for it --- app/__init__.py | 2 + app/main/forms.py | 11 ++--- app/main/views/templates.py | 15 +++++++ app/models/service.py | 5 +++ app/navigation.py | 4 ++ app/notify_client/service_api_client.py | 1 - .../template_folder_api_client.py | 28 +++++++++++++ app/templates/views/templates/add.html | 4 +- tests/app/main/views/test_letters.py | 4 +- tests/app/main/views/test_template_folders.py | 28 +++++++++++++ .../test_template_folder_client.py | 42 +++++++++++++++++++ 11 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 app/notify_client/template_folder_api_client.py create mode 100644 tests/app/main/views/test_template_folders.py create mode 100644 tests/app/notify_client/test_template_folder_client.py diff --git a/app/__init__.py b/app/__init__.py index f39d4eaae..cac27c73d 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -67,6 +67,7 @@ from app.notify_client.inbound_number_client import inbound_number_client from app.notify_client.billing_api_client import billing_api_client from app.notify_client.complaint_api_client import complaint_api_client from app.notify_client.platform_stats_api_client import platform_stats_api_client +from app.notify_client.template_folder_api_client import template_folder_api_client from app.commands import setup_commands from app.utils import get_cdn_domain, gmt_timezones, id_safe @@ -128,6 +129,7 @@ def create_app(application): billing_api_client.init_app(application) complaint_api_client.init_app(application) platform_stats_api_client.init_app(application) + template_folder_api_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' diff --git a/app/main/forms.py b/app/main/forms.py index 643fdcd31..eda0bcca9 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -850,21 +850,22 @@ class DateFilterForm(StripWhitespaceForm): class ChooseTemplateType(StripWhitespaceForm): template_type = RadioField( - 'What kind of template do you want to add?', + '', validators=[ DataRequired() ] ) - def __init__(self, include_letters=False, include_copy=False, *args, **kwargs): + def __init__(self, include_letters=False, include_copy=False, include_folder=False, *args, **kwargs): super().__init__(*args, **kwargs) self.template_type.choices = filter(None, [ - ('email', 'Email'), - ('sms', 'Text message'), - ('letter', 'Letter') if include_letters else 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') if include_folder else None, ]) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 849cb2e5e..e178d1d3c 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -183,6 +183,7 @@ def add_template_by_type(service_id): service_api_client.count_service_templates(service_id) > 0, len(user_api_client.get_service_ids_for_user(current_user)) > 1, )), + include_folder=current_service.has_permission('edit_folders') ) if form.validate_on_submit(): @@ -288,6 +289,20 @@ def action_blocked(service_id, notification_type, return_to, template_id): ) +@main.route("/services//templates/add-folder", methods=['GET', 'POST']) +def add_folder(service_id): + if not current_service.has_permission('edit_folders'): + abort(403) + + form = NewFolderForm() + + if form.validate_on_submit(): + folder_api_client.create_template_folder(form.name.data, current_service.id, parent_id=None) + return redirect( + url_for('.view_templates', service_id=service_id) + ) + + @main.route("/services//templates/add-", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_templates') diff --git a/app/models/service.py b/app/models/service.py index c663962d3..293d7d6e8 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -8,6 +8,7 @@ from app.notify_client.job_api_client import job_api_client from app.notify_client.organisations_api_client import organisations_client from app.notify_client.service_api_client import service_api_client from app.notify_client.user_api_client import user_api_client +from app.notify_client.template_folder_client import template_folder_client from app.utils import get_default_sms_sender @@ -252,3 +253,7 @@ class Service(): @property def has_inbound_number(self): return bool(self.inbound_number) + + @cached_property + def template_folders(self): + return template_folder_client.get_template_folders(self.id) diff --git a/app/navigation.py b/app/navigation.py index cf893c5ca..4bbc81fe0 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -109,6 +109,7 @@ class HeaderNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', + 'add_folder', 'agreement', 'api_callbacks', 'api_documentation', @@ -299,6 +300,7 @@ class MainNavigation(Navigation): 'action_blocked', 'add_service_template', 'add_template_by_type', + 'add_folder', 'check_messages', 'check_notification', 'choose_template', @@ -552,6 +554,7 @@ class CaseworkNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', + 'add_folder', 'agreement', 'api_callbacks', 'api_documentation', @@ -787,6 +790,7 @@ class OrgNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', + 'add_folder', 'agreement', 'api_callbacks', 'api_documentation', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 539fbbb73..78d03a563 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,4 +1,3 @@ -from __future__ import unicode_literals from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache diff --git a/app/notify_client/template_folder_api_client.py b/app/notify_client/template_folder_api_client.py new file mode 100644 index 000000000..680838a55 --- /dev/null +++ b/app/notify_client/template_folder_api_client.py @@ -0,0 +1,28 @@ +from app.notify_client import NotifyAdminAPIClient, cache + + +class TemplateFolderAPIClient(NotifyAdminAPIClient): + # Fudge assert in the super __init__ so + # we can set those variables later. + def __init__(self): + super().__init__('a' * 73, 'b') + + @cache.delete('service-{service_id}-template-folders') + def create_template_folder( + self, + service_id, + name, + parent_id=None + ): + data = { + 'name': name, + 'parent_id': parent_id + } + return self.post('/service/{}/template-folder'.format(service_id), data)['data']['id'] + + @cache.set('service-{service_id}-template-folders') + def get_template_folders(self, service_id): + return self.get('/service/{}/template-folder'.format(service_id))['data'] + + +template_folder_api_client = TemplateFolderAPIClient() diff --git a/app/templates/views/templates/add.html b/app/templates/views/templates/add.html index c46f73da4..923acfb63 100644 --- a/app/templates/views/templates/add.html +++ b/app/templates/views/templates/add.html @@ -5,12 +5,12 @@ {% extends "withnav_template.html" %} {% block service_page_title %} - Add new template + Add new template or folder {% endblock %} {% block maincolumn_content %} -

Add new template

+

What do you want to add?

{% call form_wrapper() %} {{ radios(form.template_type) }} diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index 09597c427..8c861eea2 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -56,11 +56,11 @@ def test_letters_lets_in_without_permission( @pytest.mark.parametrize('permissions, choices', [ ( ['email', 'sms', 'letter'], - ['Email', 'Text message', 'Letter', 'Copy of an existing template'] + ['Email template', 'Text message template', 'Letter template', 'Copy of an existing template'] ), ( ['email', 'sms'], - ['Email', 'Text message', 'Copy of an existing template'] + ['Email template', 'Text message template', 'Copy of an existing template'] ), ]) def test_given_option_to_add_letters_if_allowed( diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py new file mode 100644 index 000000000..b7ac1bee8 --- /dev/null +++ b/tests/app/main/views/test_template_folders.py @@ -0,0 +1,28 @@ + + +def test_add_folder( + client_request, + service_one, + mocker, + mock_get_service_templates, + mock_get_organisations_and_services_for_user, +): + service_one['permissions'] += ['edit_folders'] + 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'], + _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' + ] diff --git a/tests/app/notify_client/test_template_folder_client.py b/tests/app/notify_client/test_template_folder_client.py new file mode 100644 index 000000000..e2e0cad27 --- /dev/null +++ b/tests/app/notify_client/test_template_folder_client.py @@ -0,0 +1,42 @@ +import pytest +import uuid + +from app.notify_client.template_folder_api_client import TemplateFolderAPIClient + + +@pytest.mark.parametrize('parent_id', [uuid.uuid4(), None]) +def test_create_template_folder_calls_correct_api_endpoint(mocker, api_user_active, parent_id): + mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') + + some_service_id = uuid.uuid4() + expected_url = '/service/{}/template-folder'.format(some_service_id) + data = {'name': 'foo', 'parent_id': parent_id} + + client = TemplateFolderAPIClient() + + mock_post = mocker.patch('app.notify_client.template_folder_api_client.TemplateFolderAPIClient.post') + + client.create_template_folder(some_service_id, name='foo', parent_id=parent_id) + + mock_post.assert_called_once_with(expected_url, data) + mock_redis_delete.assert_called_once_with('service-{}-template-folders'.format(some_service_id)) + + +def test_get_template_folders_calls_correct_api_endpoint(mocker, api_user_active): + mock_redis_get = mocker.patch('app.notify_client.RedisClient.get', return_value=None) + mock_api_get = mocker.patch('app.notify_client.NotifyAdminAPIClient.get', return_value={'data': {'a': 'b'}}) + mock_redis_set = mocker.patch('app.notify_client.RedisClient.set') + + some_service_id = uuid.uuid4() + expected_url = '/service/{}/template-folder'.format(some_service_id) + redis_key = 'service-{}-template-folders'.format(some_service_id) + + client = TemplateFolderAPIClient() + + ret = client.get_template_folders(some_service_id) + + assert ret == {'a': 'b'} + + mock_redis_get.assert_called_once_with(redis_key) + mock_api_get.assert_called_once_with(expected_url) + mock_redis_set.assert_called_once_with(redis_key, '{"a": "b"}', ex=604800) From 1ecf360e7f6e02cbbd7c06e48fc9099dd61d8d8f Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Nov 2018 16:02:43 +0000 Subject: [PATCH 02/10] add template folder form --- app/main/forms.py | 4 +++ app/main/views/templates.py | 15 +++++--- app/models/service.py | 4 +-- app/navigation.py | 8 ++--- .../views/service-settings/name.html | 1 + .../views/templates/add-template-folder.html | 20 +++++++++++ tests/app/main/views/test_template_folders.py | 35 ++++++++++++++++++- 7 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 app/templates/views/templates/add-template-folder.html diff --git a/app/main/forms.py b/app/main/forms.py index eda0bcca9..3dc585d33 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1100,3 +1100,7 @@ class ReturnedLettersForm(StripWhitespaceForm): DataRequired(message="Can’t be empty"), ] ) + + +class TemplateFolderForm(StripWhitespaceForm): + name = StringField('Folder name', validators=[DataRequired(message='Can’t be empty')]) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index e178d1d3c..39f0e9dbe 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -14,6 +14,7 @@ from app import ( service_api_client, template_statistics_client, user_api_client, + template_folder_api_client ) from app.main import main from app.main.forms import ( @@ -23,6 +24,7 @@ from app.main.forms import ( SearchTemplatesForm, SetTemplateSenderForm, SMSTemplateForm, + TemplateFolderForm ) from app.main.views.send import get_example_csv_rows, get_sender_details from app.models.service import Service @@ -290,18 +292,23 @@ def action_blocked(service_id, notification_type, return_to, template_id): @main.route("/services//templates/add-folder", methods=['GET', 'POST']) -def add_folder(service_id): +def add_template_folder(service_id): if not current_service.has_permission('edit_folders'): abort(403) - form = NewFolderForm() + form = TemplateFolderForm() if form.validate_on_submit(): - folder_api_client.create_template_folder(form.name.data, current_service.id, parent_id=None) + template_folder_api_client.create_template_folder(current_service.id, name=form.name.data, parent_id=None) return redirect( - url_for('.view_templates', service_id=service_id) + url_for('.choose_template', service_id=service_id) ) + return render_template( + 'views/templates/add-template-folder.html', + form=form + ) + @main.route("/services//templates/add-", methods=['GET', 'POST']) @login_required diff --git a/app/models/service.py b/app/models/service.py index 293d7d6e8..9b5c534e4 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -8,7 +8,7 @@ from app.notify_client.job_api_client import job_api_client from app.notify_client.organisations_api_client import organisations_client from app.notify_client.service_api_client import service_api_client from app.notify_client.user_api_client import user_api_client -from app.notify_client.template_folder_client import template_folder_client +from app.notify_client.template_folder_api_client import template_folder_api_client from app.utils import get_default_sms_sender @@ -256,4 +256,4 @@ class Service(): @cached_property def template_folders(self): - return template_folder_client.get_template_folders(self.id) + return template_folder_api_client.get_template_folders(self.id) diff --git a/app/navigation.py b/app/navigation.py index 4bbc81fe0..af45c74ba 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -109,7 +109,7 @@ class HeaderNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', - 'add_folder', + 'add_template_folder', 'agreement', 'api_callbacks', 'api_documentation', @@ -300,7 +300,7 @@ class MainNavigation(Navigation): 'action_blocked', 'add_service_template', 'add_template_by_type', - 'add_folder', + 'add_template_folder', 'check_messages', 'check_notification', 'choose_template', @@ -554,7 +554,7 @@ class CaseworkNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', - 'add_folder', + 'add_template_folder', 'agreement', 'api_callbacks', 'api_documentation', @@ -790,7 +790,7 @@ class OrgNavigation(Navigation): 'add_service', 'add_service_template', 'add_template_by_type', - 'add_folder', + 'add_template_folder', 'agreement', 'api_callbacks', 'api_documentation', diff --git a/app/templates/views/service-settings/name.html b/app/templates/views/service-settings/name.html index 33694395d..182af6fc8 100644 --- a/app/templates/views/service-settings/name.html +++ b/app/templates/views/service-settings/name.html @@ -1,3 +1,4 @@ + {% extends "withnav_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-footer.html" import page_footer %} diff --git a/app/templates/views/templates/add-template-folder.html b/app/templates/views/templates/add-template-folder.html new file mode 100644 index 000000000..81a816918 --- /dev/null +++ b/app/templates/views/templates/add-template-folder.html @@ -0,0 +1,20 @@ + +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% block service_page_title %} + Add a folder +{% endblock %} + +{% block maincolumn_content %} + +

Add a folder

+ + {% call form_wrapper() %} + {{ textbox(form.name) }} + {{ page_footer('Save') }} + {% endcall %} + +{% endblock %} diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index b7ac1bee8..50bc396b7 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -1,6 +1,7 @@ +from flask import url_for -def test_add_folder( +def test_add_page_shows_option_for_folder( client_request, service_one, mocker, @@ -26,3 +27,35 @@ def test_add_folder( 'Copy of an existing template', 'Folder' ] + + +def test_get_add_template_folder_page(client_request, service_one): + service_one['permissions'] += ['edit_folders'] + + page = client_request.get('main.add_template_folder', service_id=service_one['id']) + + assert page.find('input', attrs={'name': 'name'}) is not None + + +def test_add_template_folder_page_rejects_if_service_doesnt_have_permission(client_request, service_one): + client_request.get('main.add_template_folder', service_id=service_one['id'], _expected_status=403) + client_request.post('main.add_template_folder', service_id=service_one['id'], _expected_status=403) + + +def test_post_add_template_folder_page(client_request, service_one, mocker): + mock_create = mocker.patch('app.template_folder_api_client.create_template_folder') + + service_one['permissions'] += ['edit_folders'] + + client_request.post( + 'main.add_template_folder', + service_id=service_one['id'], + _data={'name': 'foo'}, + _expected_redirect=url_for( + 'main.choose_template', + service_id=service_one['id'], + _external=True, + ) + ) + + mock_create.assert_called_once_with(service_one['id'], name='foo', parent_id=None) From 078595da9dcfad97098a6c2c29c91b07b6d40fdb Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Nov 2018 17:11:58 +0000 Subject: [PATCH 03/10] show list of template folders on the choose templates page Not linking anywhere yet. Also not sorted, and not filtered by parent_id. --- app/main/views/templates.py | 5 +++-- app/models/service.py | 4 +++- app/notify_client/template_folder_api_client.py | 2 +- app/templates/views/templates/choose.html | 10 ++++++++++ tests/app/main/views/test_accept_invite.py | 1 + tests/app/main/views/test_send.py | 2 -- tests/app/main/views/test_templates.py | 5 +++++ tests/app/notify_client/test_template_folder_client.py | 8 ++++++-- tests/app/test_navigation.py | 3 +++ tests/conftest.py | 8 ++++++++ 10 files changed, 40 insertions(+), 8 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 39f0e9dbe..67973ad13 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -12,9 +12,9 @@ from notifications_utils.recipients import first_column_headings from app import ( current_service, service_api_client, + template_folder_api_client, template_statistics_client, user_api_client, - template_folder_api_client ) from app.main import main from app.main.forms import ( @@ -24,7 +24,7 @@ from app.main.forms import ( SearchTemplatesForm, SetTemplateSenderForm, SMSTemplateForm, - TemplateFolderForm + TemplateFolderForm, ) from app.main.views.send import get_example_csv_rows, get_sender_details from app.models.service import Service @@ -117,6 +117,7 @@ def choose_template(service_id, template_type='all'): return render_template( 'views/templates/choose.html', + template_folders=current_service.template_folders, templates=current_service.templates_by_type(template_type), show_search_box=(len(current_service.templates_by_type(template_type)) > 7), show_template_nav=( diff --git a/app/models/service.py b/app/models/service.py index 9b5c534e4..18c53aae9 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -7,8 +7,10 @@ from app.notify_client.inbound_number_client import inbound_number_client from app.notify_client.job_api_client import job_api_client from app.notify_client.organisations_api_client import organisations_client from app.notify_client.service_api_client import service_api_client +from app.notify_client.template_folder_api_client import ( + template_folder_api_client, +) from app.notify_client.user_api_client import user_api_client -from app.notify_client.template_folder_api_client import template_folder_api_client from app.utils import get_default_sms_sender diff --git a/app/notify_client/template_folder_api_client.py b/app/notify_client/template_folder_api_client.py index 680838a55..c04a7bada 100644 --- a/app/notify_client/template_folder_api_client.py +++ b/app/notify_client/template_folder_api_client.py @@ -22,7 +22,7 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient): @cache.set('service-{service_id}-template-folders') def get_template_folders(self, service_id): - return self.get('/service/{}/template-folder'.format(service_id))['data'] + return self.get('/service/{}/template-folder'.format(service_id))['template_folders'] template_folder_api_client = TemplateFolderAPIClient() diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index e69ac226a..85b586666 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -59,6 +59,16 @@ {{ live_search(target_selector='#template-list .column-whole', show=show_search_box, form=search_form) }}