From 194756bc2e9c990887ac587c4f3ec3bf8efc27a9 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Mon, 25 Feb 2019 14:55:26 +0000 Subject: [PATCH 1/6] Make radio form components reusable for nested checkboxes For the template folders permission editing we need a nested checkboxes form that is similar to "move folder" input, except it's using checkboxes instead of radio buttons. This moves most of the macros into a shared "select-input" components file, which are wrapped by the existing radios.html by setting the required input type. --- app/main/forms.py | 13 ++- app/templates/components/radios.html | 108 +++------------------ app/templates/components/select-input.html | 92 ++++++++++++++++++ 3 files changed, 113 insertions(+), 100 deletions(-) create mode 100644 app/templates/components/select-input.html diff --git a/app/main/forms.py b/app/main/forms.py index bd513ba73..eee4082e1 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -837,10 +837,7 @@ class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): pass -class NestedRadioField(RadioFieldWithNoneOption): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - +class NestedFieldMixin: def children(self): # start map with root option as a single child entry child_map = {None: [option for option in self @@ -864,6 +861,14 @@ class NestedRadioField(RadioFieldWithNoneOption): return child_map +class NestedRadioField(RadioFieldWithNoneOption, NestedFieldMixin): + pass + + +class NestedCheckboxesField(SelectMultipleField, NestedFieldMixin): + pass + + class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): pass diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 351158acb..b7b51f6cd 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -1,111 +1,27 @@ -{% macro radios( - field, - hint=None, - disable=[], - option_hints={}, - hide_legend=False -) %} - {% call radios_wrapper( - field, hint, disable, option_hints, hide_legend - ) %} - {% for option in field %} - {{ radio(option, disable, option_hints) }} - {% endfor %} - {% endcall %} +{% from "components/select-input.html" import select, select_list, select_nested, select_wrapper %} + +{% macro radios(field, hint=None, disable=[], option_hints={}, hide_legend=False) %} + {{ select(field, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") }} {% endmacro %} -{% macro radio_list( - options, - child_map, - disable=[], - option_hints={} -) %} - +{% 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=None, disable=[], option_hints={}, hide_legend=False, 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() }} -
-
+ {{ select_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False, input="radio") }} {% 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=None, as_list_item=False, 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 %} From 340fd021bbd93b59d0b2d0e62ad8cfe9aaa296ac Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 25 Feb 2019 16:17:48 +0000 Subject: [PATCH 2/6] Move field definitions before form definitions --- app/main/forms.py | 144 +++++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index eee4082e1..9d18b5a06 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): @@ -813,78 +885,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 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): - pass - - -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( From 7413423243a1ae909d2dd70b19637f32125e65be Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 25 Feb 2019 16:23:28 +0000 Subject: [PATCH 3/6] 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') From 3ba4a22b7c5517d5ac7dd3b3e5399431c2bb232e Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Wed, 27 Feb 2019 15:44:52 +0000 Subject: [PATCH 4/6] Add checkboxes-nested CSS rules --- .../stylesheets/components/checkboxes.scss | 44 +++++++++++++++++++ app/assets/stylesheets/main.scss | 1 + 2 files changed, 45 insertions(+) create mode 100644 app/assets/stylesheets/components/checkboxes.scss 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'; From 6fa975e867e2ebfe546c87329409adf06c3fae17 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Wed, 27 Feb 2019 15:45:18 +0000 Subject: [PATCH 5/6] Send updated user folder permissions to the API Integrates the folder permissions form with the updated API endpoint to store changes in the user folders. Since user folder permissions are returned in the full list of template folders for the service we need to invalidate the cache key for it each time we update user permissions. --- app/main/forms.py | 2 +- app/main/views/manage_users.py | 8 ++++ app/notify_client/user_api_client.py | 8 +++- .../views/manage-users/permissions.html | 2 +- tests/app/main/views/test_manage_users.py | 43 ++++++++++++++++++- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index d3e410bd2..ba9f2dbbd 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -420,7 +420,7 @@ PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), { class PermissionsForm(PermissionsAbstract): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) - if all_template_folders: + 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) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 56ebdcb32..8c954b767 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -87,6 +87,10 @@ def edit_user_permissions(service_id, user_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 ) @@ -94,6 +98,10 @@ def edit_user_permissions(service_id, user_id): 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/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index 8b2e54f2c..50648da5e 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -14,7 +14,7 @@ All team members can see sent messages.

    -{% if current_service.has_permission("edit_folder_permissions") %} +{% 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 %} diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 122f58e9b..0ddf73223 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -447,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'] ) @@ -508,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), @@ -1021,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 ): From a2389fe2ca2516f33e19ce23b80d216590e6350a Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Tue, 5 Mar 2019 11:45:50 +0000 Subject: [PATCH 6/6] Make folder permissions form label more descriptive --- app/main/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/forms.py b/app/main/forms.py index ba9f2dbbd..aa5c54ab8 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -426,7 +426,7 @@ class PermissionsForm(PermissionsAbstract): (item['id'], item['name']) for item in ([{'name': 'Templates', 'id': None}] + all_template_folders) ] - folder_permissions = NestedCheckboxesField('Folder permissions') + folder_permissions = NestedCheckboxesField('Folders this team member can see') login_authentication = RadioField( 'Sign in using',