From 7413423243a1ae909d2dd70b19637f32125e65be Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 25 Feb 2019 16:23:28 +0000 Subject: [PATCH] Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`. --- app/main/forms.py | 12 +++++++++++- app/main/views/manage_users.py | 6 +++++- app/templates/components/checkbox.html | 7 +++++++ app/templates/components/radios.html | 15 +++++---------- app/templates/views/manage-users/permissions.html | 8 ++++++-- .../views/service-settings/contact_link.html | 5 +++-- tests/app/main/views/test_manage_users.py | 8 +++++++- 7 files changed, 44 insertions(+), 17 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 9d18b5a06..d3e410bd2 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -418,6 +418,15 @@ PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), { class PermissionsForm(PermissionsAbstract): + def __init__(self, all_template_folders=None, *args, **kwargs): + super().__init__(*args, **kwargs) + if all_template_folders: + self.folder_permissions.all_template_folders = all_template_folders + self.folder_permissions.choices = [ + (item['id'], item['name']) for item in ([{'name': 'Templates', 'id': None}] + all_template_folders) + ] + + folder_permissions = NestedCheckboxesField('Folder permissions') login_authentication = RadioField( 'Sign in using', @@ -437,8 +446,9 @@ class PermissionsForm(PermissionsAbstract): return (getattr(self, permission) for permission, _ in permissions) @classmethod - def from_user(cls, user, service_id): + def from_user(cls, user, service_id, **kwargs): return cls( + **kwargs, **{ role: user.has_permission_for_service(service_id, role) for role in roles.keys() diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index feef22c2e..56ebdcb32 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -84,7 +84,11 @@ def edit_user_permissions(service_id, user_id): if user.mobile_number: mobile_number = redact_mobile_number(user.mobile_number, " ") - form = PermissionsForm.from_user(user, service_id) + form = PermissionsForm.from_user( + user, + service_id, + all_template_folders=current_service.all_template_folders + ) if form.validate_on_submit(): user_api_client.set_user_permissions( diff --git a/app/templates/components/checkbox.html b/app/templates/components/checkbox.html index 0c6fcc843..0a9e0907b 100644 --- a/app/templates/components/checkbox.html +++ b/app/templates/components/checkbox.html @@ -1,3 +1,5 @@ +{% from "components/select-input.html" import select_nested %} + {% macro checkbox( field, hint=False, @@ -17,6 +19,11 @@ {% endmacro %} +{% 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") }} +{% endmacro %} + + {% macro checkbox_input(id, name, data=None, value="y") %} @@ -14,6 +14,10 @@ All team members can see sent messages.

+{% if current_service.has_permission("edit_folder_permissions") %} + {{ checkboxes_nested(form.folder_permissions, form.folder_permissions.children()) }} +{% endif %} + {% if service_has_email_auth %} {% if not mobile_number %} {{ radios( diff --git a/app/templates/views/service-settings/contact_link.html b/app/templates/views/service-settings/contact_link.html index d4b5bb385..7e6fb3171 100644 --- a/app/templates/views/service-settings/contact_link.html +++ b/app/templates/views/service-settings/contact_link.html @@ -1,7 +1,8 @@ {% extends "withnav_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-footer.html" import page_footer %} -{% from "components/radios.html" import radio, radios_wrapper %} +{% from "components/radios.html" import radio %} +{% from "components/select-input.html" import select_wrapper %} {% from "components/form.html" import form_wrapper %} {% block service_page_title %} @@ -21,7 +22,7 @@

{% call form_wrapper() %} - {% call radios_wrapper(form.contact_details_type, hide_legend=true) %} + {% call select_wrapper(form.contact_details_type, hide_legend=true) %} {% for option in form.contact_details_type %} {% set data_target = option.data.replace('_', '-') ~ "-type" %} diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 46c48f60f..122f58e9b 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -228,6 +228,7 @@ def test_service_with_no_email_auth_hides_auth_type_options( auth_options_hidden, service_one, mock_get_users_by_service, + mock_get_template_folders ): if service_has_email_auth: service_one['permissions'].append('email_auth') @@ -249,6 +250,7 @@ def test_service_with_no_email_auth_hides_auth_type_options( def test_service_without_caseworking_doesnt_show_admin_vs_caseworker( client_request, mock_get_users_by_service, + mock_get_template_folders, endpoint, service_has_caseworking, extra_args, @@ -304,6 +306,7 @@ def test_manage_users_page_shows_member_auth_type_if_service_has_email_auth_acti def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( client_request, mock_get_users_by_service, + mock_get_template_folders, user, sms_option_disabled, expected_label, @@ -353,6 +356,7 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( def test_should_show_page_for_one_user( client_request, mock_get_users_by_service, + mock_get_template_folders, endpoint, extra_args, expected_checkboxes, @@ -419,6 +423,7 @@ def test_edit_user_permissions( mock_get_users_by_service, mock_get_invites_for_service, mock_set_user_permissions, + mock_get_template_folders, fake_uuid, submitted_permissions, permissions_sent_to_api, @@ -474,7 +479,8 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( mock_set_user_permissions, mock_update_user_attribute, service_one, - auth_type + auth_type, + mock_get_template_folders ): service_one['permissions'].append('email_auth')