diff --git a/app/assets/stylesheets/components/checkboxes.scss b/app/assets/stylesheets/components/checkboxes.scss new file mode 100644 index 000000000..40ca600d4 --- /dev/null +++ b/app/assets/stylesheets/components/checkboxes.scss @@ -0,0 +1,44 @@ +.checkboxes-nested { + + margin-bottom: 10px; + + .multiple-choice { + + $circle-diameter: 39px; + $border-thickness: 4px; + $border-indent: ($circle-diameter / 2) - ($border-thickness / 2); + $border-colour: $border-colour; + + float: none; + position: relative; + + &:before { + content: ""; + position: absolute; + bottom: 0; + left: $border-indent; + width: $border-thickness; + height: 100%; + background: $border-colour; + } + + label { + float: none; + } + + [type=checkbox]+label::before { + // To overlap the grey inset line + background: $white; + } + + ul { + // To equalise the spacing between the line and the top/bottom of + // the radio + margin-top: 5px; + margin-bottom: -5px; + padding-left: 12px; + } + + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 1db1fcff1..a12340dca 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -47,6 +47,7 @@ $path: '/static/images/'; @import 'components/api-key'; @import 'components/vendor/previous-next-navigation'; @import 'components/radios'; +@import 'components/checkboxes'; @import 'components/pill'; @import 'components/secondary-button'; @import 'components/show-more'; diff --git a/app/main/forms.py b/app/main/forms.py index bd513ba73..aa5c54ab8 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -255,6 +255,78 @@ def organisation_type(): ) +class FieldWithNoneOption(): + + # This is a special value that is specific to our forms. This is + # more expicit than casting `None` to a string `'None'` which can + # have unexpected edge cases + NONE_OPTION_VALUE = '__NONE__' + + # When receiving Python data, eg when instantiating the form object + # we want to convert that data to our special value, so that it gets + # recognised as being one of the valid choices + def process_data(self, value): + self.data = self.NONE_OPTION_VALUE if value is None else value + + # After validation we want to convert it back to a Python `None` for + # use elsewhere, eg posting to the API + def post_validate(self, form, validation_stopped): + if self.data == self.NONE_OPTION_VALUE and not validation_stopped: + self.data = None + + +class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): + pass + + +class NestedFieldMixin: + def children(self): + # start map with root option as a single child entry + child_map = {None: [option for option in self + if option.data == self.NONE_OPTION_VALUE]} + + # add entries for all other children + for option in self: + if option.data == self.NONE_OPTION_VALUE: + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] is None] + key = self.NONE_OPTION_VALUE + else: + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] == option.data] + key = option.data + + child_map[key] = [option for option in self if option.data in child_ids] + + return child_map + + +class NestedRadioField(RadioFieldWithNoneOption, NestedFieldMixin): + pass + + +class NestedCheckboxesField(SelectMultipleField, NestedFieldMixin): + NONE_OPTION_VALUE = None + + +class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): + pass + + +class RadioFieldWithRequiredMessage(RadioField): + def __init__(self, *args, required_message='Not a valid choice', **kwargs): + self.required_message = required_message + super().__init__(*args, **kwargs) + + def pre_validate(self, form): + try: + return super().pre_validate(form) + except ValueError: + raise ValueError(self.required_message) + + class StripWhitespaceForm(Form): class Meta: def bind_field(self, form, unbound_field, options): @@ -346,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 is not None: + 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('Folders this team member can see') login_authentication = RadioField( 'Sign in using', @@ -365,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() @@ -813,73 +895,6 @@ class ServiceSwitchChannelForm(ServiceOnOffSettingForm): super().__init__(name, *args, **kwargs) -class FieldWithNoneOption(): - - # This is a special value that is specific to our forms. This is - # more expicit than casting `None` to a string `'None'` which can - # have unexpected edge cases - NONE_OPTION_VALUE = '__NONE__' - - # When receiving Python data, eg when instantiating the form object - # we want to convert that data to our special value, so that it gets - # recognised as being one of the valid choices - def process_data(self, value): - self.data = self.NONE_OPTION_VALUE if value is None else value - - # After validation we want to convert it back to a Python `None` for - # use elsewhere, eg posting to the API - def post_validate(self, form, validation_stopped): - if self.data == self.NONE_OPTION_VALUE and not validation_stopped: - self.data = None - - -class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): - pass - - -class NestedRadioField(RadioFieldWithNoneOption): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - - def children(self): - # start map with root option as a single child entry - child_map = {None: [option for option in self - if option.data == self.NONE_OPTION_VALUE]} - - # add entries for all other children - for option in self: - if option.data == self.NONE_OPTION_VALUE: - child_ids = [ - folder['id'] for folder in self.all_template_folders - if folder['parent_id'] is None] - key = self.NONE_OPTION_VALUE - else: - child_ids = [ - folder['id'] for folder in self.all_template_folders - if folder['parent_id'] == option.data] - key = option.data - - child_map[key] = [option for option in self if option.data in child_ids] - - return child_map - - -class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): - pass - - -class RadioFieldWithRequiredMessage(RadioField): - def __init__(self, *args, required_message='Not a valid choice', **kwargs): - self.required_message = required_message - super().__init__(*args, **kwargs) - - def pre_validate(self, form): - try: - return super().pre_validate(form) - except ValueError: - raise ValueError(self.required_message) - - class ServiceSetEmailBranding(StripWhitespaceForm): branding_style = RadioFieldWithNoneOption( diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index feef22c2e..8c954b767 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -84,12 +84,24 @@ 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, + folder_permissions=[ + f['id'] for f in current_service.all_template_folders + if user_id in f.get('users_with_permission', []) + ], + all_template_folders=current_service.all_template_folders + ) if form.validate_on_submit(): user_api_client.set_user_permissions( user_id, service_id, permissions=form.permissions, + folder_permissions=( + form.folder_permissions.data + if current_service.has_permission('edit_folder_permissions') else None + ), ) if service_has_email_auth: user_api_client.update_user_attribute(user_id, auth_type=form.login_authentication.data) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 3bb007bf2..619468f4e 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -161,13 +161,17 @@ class UserApiClient(NotifyAdminAPIClient): resp = self.post('/organisations/{}/users/{}'.format(org_id, user_id), data={}) return User(resp['data'], max_failed_login_count=self.max_failed_login_count) + @cache.delete('service-{service_id}-template-folders') @cache.delete('user-{user_id}') - def set_user_permissions(self, user_id, service_id, permissions): + def set_user_permissions(self, user_id, service_id, permissions, folder_permissions=None): # permissions passed in are the combined admin roles, not db permissions data = { - 'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)] + 'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)], } + if folder_permissions is not None: + data['folder_permissions'] = folder_permissions + endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id) self.post(endpoint, data=data) 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") %} - {% for option in options %} - {% if child_map[option.data] %} - {% call radio(option, disable, option_hints, as_list_item=True) %} - {{ radio_list(child_map[option.data], child_map, disable, option_hints) }} - {% endcall %} - {% else %} - {{ radio(option, disable, option_hints, as_list_item=True) }} - {% endif %} - {% endfor %} - +{% macro radio_list(options, child_map, disable=[], option_hints={}) %} + {{ select_list(options, child_map, disable, option_hints, input="radio") }} {% endmacro %} -{% macro radios_nested( - field, - child_map, - hint=None, - disable=[], - option_hints={}, - hide_legend=False -) %} - {% call radios_wrapper( - field, hint, disable, option_hints, hide_legend - ) %} -
- {{ radio_list(child_map[None], child_map, disable, option_hints) }} -
- {% endcall %} +{% macro radios_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False) %} + {{ select_nested(field, child_map, hint, disable, option_hints, hide_legend, input="radio") }} {% endmacro %} -{% macro radios_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False) %} -
-
- - {% if hide_legend %}{% endif %} - {% if hint %} - - {{ hint }} - - {% endif %} - {% if field.errors %} - - {{ field.errors[0] }} - - {% endif %} - - {{ caller() }} -
-
-{% endmacro %} {% macro radio(option, disable=[], option_hints={}, data_target=None, as_list_item=False) %} - {% if as_list_item %} -
  • - {% else %} -
    - {% endif %} - - - {% if caller %} - {{ caller() }} - {% endif %} - {% if as_list_item %} -
    • - {% else %} - - {% endif %} + {{ select_input(option, disable, option_hints, data_target, as_list_item, input="radio") }} {% endmacro %} diff --git a/app/templates/components/select-input.html b/app/templates/components/select-input.html new file mode 100644 index 000000000..b3cfbb9e4 --- /dev/null +++ b/app/templates/components/select-input.html @@ -0,0 +1,92 @@ +{% macro select(field, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") %} + {% call select_wrapper( + field, hint, disable, option_hints, hide_legend + ) %} + {% for option in field %} + {{ select_input(option, disable, option_hints, input=input) }} + {% endfor %} + {% endcall %} +{% endmacro %} + + +{% macro select_list(options, child_map, disable=[], option_hints={}, input="radio") %} + +{% endmacro %} + + +{% macro select_nested(field, child_map, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") %} + {% call select_wrapper( + field, hint, disable, option_hints, hide_legend + ) %} +
    + {{ select_list(child_map[None], child_map, disable, option_hints, input=input) }} +
    + {% endcall %} +{% endmacro %} + + +{% macro select_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False) %} +
    +
    + + {% if hide_legend %}{% endif %} + {% if hint %} + + {{ hint }} + + {% endif %} + {% if field.errors %} + + {{ field.errors[0] }} + + {% endif %} + + {{ caller() }} +
    +
    +{% endmacro %} + +{% macro select_input(option, disable=[], option_hints={}, data_target=None, as_list_item=False, input="radio") %} + {% if as_list_item %} +
  • + {% else %} +
    + {% endif %} + + + {% if caller %} + {{ caller() }} + {% endif %} + {% if as_list_item %} +
    • + {% else %} + + {% endif %} +{% endmacro %} diff --git a/app/templates/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index fab879e4f..50648da5e 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -1,5 +1,5 @@ -{% from "components/checkbox.html" import checkbox %} -{% from "components/radios.html" import radio, radios, radios_wrapper, conditional_radio_panel %} +{% from "components/checkbox.html" import checkbox, checkboxes_nested %} +{% from "components/radios.html" import radio, radios, conditional_radio_panel %}
    @@ -14,6 +14,10 @@ All team members can see sent messages.

    +{% if current_service.has_permission("edit_folder_permissions") and form.folder_permissions.all_template_folders %} + {{ 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..0ddf73223 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, @@ -442,6 +447,45 @@ def test_edit_user_permissions( fake_uuid, SERVICE_ONE_ID, permissions=permissions_sent_to_api, + folder_permissions=None + ) + + +def test_edit_user_folder_permissions( + client_request, + mocker, + service_one, + mock_get_users_by_service, + mock_get_invites_for_service, + mock_set_user_permissions, + mock_get_template_folders, + fake_uuid, +): + service_one['permissions'] = ['edit_folder_permissions'] + mock_get_template_folders.return_value = [ + {'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-2', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-3', 'name': 'folder_one', 'parent_id': 'folder-id-1', 'users_with_permission': []}, + ] + client_request.post( + 'main.edit_user_permissions', + service_id=SERVICE_ONE_ID, + user_id=fake_uuid, + _data=dict( + folder_permissions=['folder-id-1', 'folder-id-3'] + ), + _expected_status=302, + _expected_redirect=url_for( + 'main.manage_users', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ) + mock_set_user_permissions.assert_called_with( + fake_uuid, + SERVICE_ONE_ID, + permissions=set(), + folder_permissions=['folder-id-1', 'folder-id-3'] ) @@ -474,7 +518,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') @@ -502,7 +547,8 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( 'manage_templates', 'manage_service', 'manage_api_keys', - } + }, + folder_permissions=None ) mock_update_user_attribute.assert_called_with( str(active_user_with_permissions.id), @@ -1015,6 +1061,7 @@ def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_l client_request, active_user_with_permissions, mock_get_users_by_service, + mock_get_template_folders, service_one, mocker ):