diff --git a/app/main/forms.py b/app/main/forms.py index aa5c54ab8..701b4a89f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1285,6 +1285,15 @@ class ReturnedLettersForm(StripWhitespaceForm): class TemplateFolderForm(StripWhitespaceForm): + def __init__(self, all_service_users=None, *args, **kwargs): + super().__init__(*args, **kwargs) + if all_service_users is not None: + self.users_with_permission.all_service_users = all_service_users + self.users_with_permission.choices = [ + (item.id, item.name) for item in all_service_users + ] + + users_with_permission = MultiCheckboxField('Users who can see this folder:') 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 a16564748..97e1b1515 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -430,14 +430,19 @@ def action_blocked(service_id, notification_type, return_to, template_id): @login_required @user_has_permissions('manage_templates') def manage_template_folder(service_id, template_folder_id): - + current_folder = current_service.get_template_folder(template_folder_id) form = TemplateFolderForm( - name=current_service.get_template_folder(template_folder_id)['name'] + name=current_folder['name'], + users_with_permission=current_folder.get('users_with_permission', None), + all_service_users=[user for user in current_service.active_users if user.id != current_user.id] ) - if form.validate_on_submit(): + users_with_permission = form.users_with_permission.data + [current_user.id] template_folder_api_client.update_template_folder( - current_service.id, template_folder_id, name=form.name.data + current_service.id, + template_folder_id, + name=form.name.data, + users_with_permission=users_with_permission ) return redirect( url_for('.choose_template', service_id=service_id, template_folder_id=template_folder_id) @@ -449,7 +454,7 @@ def manage_template_folder(service_id, template_folder_id): template_folder_path=current_service.get_template_folder_path(template_folder_id), current_service_id=current_service.id, template_folder_id=template_folder_id, - template_type="all" + template_type="all", ) diff --git a/app/notify_client/template_folder_api_client.py b/app/notify_client/template_folder_api_client.py index d273b4dc8..f7015c1cb 100644 --- a/app/notify_client/template_folder_api_client.py +++ b/app/notify_client/template_folder_api_client.py @@ -42,10 +42,13 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient): )) @cache.delete('service-{service_id}-template-folders') - def update_template_folder(self, service_id, template_folder_id, name): + def update_template_folder(self, service_id, template_folder_id, name, users_with_permission=None): + data = {"name": name} + if users_with_permission: + data["users_with_permission"] = users_with_permission self.post( '/service/{}/template-folder/{}'.format(service_id, template_folder_id), - {"name": name} + data ) @cache.delete('service-{service_id}-template-folders') diff --git a/app/templates/components/checkbox.html b/app/templates/components/checkbox.html index 0a9e0907b..51594eea6 100644 --- a/app/templates/components/checkbox.html +++ b/app/templates/components/checkbox.html @@ -1,4 +1,4 @@ -{% from "components/select-input.html" import select_nested %} +{% from "components/select-input.html" import select_nested, select %} {% macro checkbox( field, @@ -20,7 +20,12 @@ {% macro checkboxes_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False) %} - {{ select_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False, input="checkbox") }} + {{ select_nested(field, child_map, hint, disable, option_hints, hide_legend, input="checkbox") }} +{% endmacro %} + + +{% macro checkboxes(field, hint=None, disable=[], option_hints={}, hide_legend=False) %} + {{ select(field, hint, disable, option_hints, hide_legend, input="checkbox") }} {% endmacro %} @@ -39,17 +44,3 @@ {% endmacro %} - -{% macro checkbox_group( - legend, - fields -) %} -
- - {{ legend }} - - {% for field in fields %} - {{ checkbox(field) }} - {% endfor %} -
-{% endmacro %} diff --git a/app/templates/views/templates/manage-template-folder.html b/app/templates/views/templates/manage-template-folder.html index 2b14dee9f..43dee3dad 100644 --- a/app/templates/views/templates/manage-template-folder.html +++ b/app/templates/views/templates/manage-template-folder.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/folder-path.html" import folder_path, page_title_folder_path %} {% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox, checkboxes %} {% from "components/page-footer.html" import page_footer %} {% from "components/form.html" import form_wrapper %} @@ -23,6 +24,10 @@ {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %} {{ textbox(form.name) }} + {% if current_service.has_permission("edit_folder_permissions") %} + {{ checkboxes(form.users_with_permission) }} + {% endif %} + {{ page_footer( 'Save', delete_link=url_for( diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 00f22ef34..1b0db751a 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -417,14 +417,18 @@ def test_can_create_email_template_with_parent_folder( def test_get_manage_folder_page( client_request, + active_user_with_permissions, service_one, mock_get_template_folders, + mocker ): folder_id = str(uuid.uuid4()) mock_get_template_folders.return_value = [ - {'id': folder_id, 'name': 'folder_two', 'parent_id': None}, + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id + ]}, ] - + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions]) page = client_request.get( 'main.manage_template_folder', service_id=service_one['id'], @@ -437,10 +441,50 @@ def test_get_manage_folder_page( assert page.select_one('input[name=name]')['value'] == 'folder_two' delete_link = page.find('a', string="Delete this folder") expected_delete_url = "/services/{}/templates/folders/{}/delete".format(service_one['id'], folder_id) - assert expected_delete_url in delete_link["href"] +def test_get_manage_folder_viewing_permissions_for_users( + client_request, + active_user_with_permissions, + service_one, + mock_get_template_folders, + mocker +): + folder_id = str(uuid.uuid4()) + team_member = active_user_view_permissions(str(uuid.uuid4())) + team_member_2 = active_user_view_permissions(str(uuid.uuid4())) + service_one["permissions"] += ["edit_folder_permissions"] + mock_get_template_folders.return_value = [ + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id, team_member_2.id + ]}, + ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member, team_member_2]) + + page = client_request.get( + 'main.manage_template_folder', + service_id=service_one['id'], + template_folder_id=folder_id, + _test_page_title=False, + ) + assert normalize_spaces(page.select_one('title').text) == ( + 'folder_two – Templates – service one – GOV.UK Notify' + ) + form_labels = page.select('legend[class=form-label]') + assert "Users who can see this folder:" in form_labels[0].text + checkboxes = page.select('input[name=users_with_permission]') + + assert len(checkboxes) == 2 + assert checkboxes[0]['value'] == team_member.id + assert "checked" not in checkboxes[0].attrs + + assert checkboxes[1]['value'] == team_member_2.id + assert "checked" in checkboxes[1].attrs + + assert "Test User" in page.findAll('label', {'for': 'users_with_permission-0'})[0].text + + def test_manage_folder_page_404s(client_request, service_one, mock_get_template_folders): client_request.get( 'main.manage_template_folder', @@ -463,18 +507,21 @@ def test_get_manage_folder_page_no_permissions( ) -def test_rename_folder(client_request, service_one, mock_get_template_folders, mocker): +def test_rename_folder(client_request, active_user_with_permissions, service_one, mock_get_template_folders, mocker): mock_update = mocker.patch('app.template_folder_api_client.update_template_folder') folder_id = str(uuid.uuid4()) mock_get_template_folders.return_value = [ - {'id': folder_id, 'name': 'folder_two', 'parent_id': None}, + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id + ]} ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions]) client_request.post( 'main.manage_template_folder', service_id=service_one['id'], template_folder_id=folder_id, - _data={"name": "new beautiful name"}, + _data={"name": "new beautiful name", "users_with_permission": []}, _expected_redirect=url_for("main.choose_template", service_id=service_one['id'], template_folder_id=folder_id, @@ -484,7 +531,40 @@ def test_rename_folder(client_request, service_one, mock_get_template_folders, m mock_update.assert_called_once_with( service_one['id'], folder_id, - name="new beautiful name" + name="new beautiful name", + users_with_permission=[active_user_with_permissions.id] + ) + + +def test_manage_folder_users( + client_request, active_user_with_permissions, service_one, mock_get_template_folders, mocker +): + team_member = active_user_view_permissions(str(uuid.uuid4())) + mock_update = mocker.patch('app.template_folder_api_client.update_template_folder') + folder_id = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id, team_member.id + ]} + ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member]) + + client_request.post( + 'main.manage_template_folder', + service_id=service_one['id'], + template_folder_id=folder_id, + _data={"name": "new beautiful name", "users_with_permission": []}, + _expected_redirect=url_for("main.choose_template", + service_id=service_one['id'], + template_folder_id=folder_id, + _external=True) + ) + + mock_update.assert_called_once_with( + service_one['id'], + folder_id, + name="new beautiful name", + users_with_permission=[active_user_with_permissions.id] ) diff --git a/tests/app/notify_client/test_template_folder_client.py b/tests/app/notify_client/test_template_folder_client.py index bfd615e2f..a679d9e98 100644 --- a/tests/app/notify_client/test_template_folder_client.py +++ b/tests/app/notify_client/test_template_folder_client.py @@ -111,13 +111,13 @@ def test_update_template_folder_calls_correct_api_endpoint(mocker): some_service_id = uuid.uuid4() template_folder_id = uuid.uuid4() expected_url = '/service/{}/template-folder/{}'.format(some_service_id, template_folder_id) - data = {'name': 'foo'} + data = {'name': 'foo', 'users_with_permission': ['some_id']} client = TemplateFolderAPIClient() mock_post = mocker.patch('app.notify_client.template_folder_api_client.TemplateFolderAPIClient.post') - client.update_template_folder(some_service_id, template_folder_id, name='foo') + client.update_template_folder(some_service_id, template_folder_id, name='foo', users_with_permission=['some_id']) mock_post.assert_called_once_with(expected_url, data) mock_redis_delete.assert_called_once_with('service-{}-template-folders'.format(some_service_id))