From 8d9814f073f007e844950c64586463d6c1234703 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 27 Jan 2022 17:38:00 +0100 Subject: [PATCH 1/2] manager: use multiple selection widget to manage role's members (#61188) --- src/authentic2/manager/forms.py | 53 ++++++++++++------- src/authentic2/manager/role_views.py | 20 +++---- .../manager/role_members_table.html | 2 +- tests/test_manager.py | 8 +-- tests/test_role_manager.py | 14 ++--- 5 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index bb8f2275..b7400918 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -30,7 +30,7 @@ from django.urls import reverse from django.utils.text import slugify from django.utils.translation import pgettext, ugettext from django.utils.translation import ugettext_lazy as _ -from django_select2.forms import HeavySelect2Widget +from django_select2.forms import HeavySelect2MultipleWidget from authentic2.a2_rbac.models import OrganizationalUnit, Permission, Role from authentic2.a2_rbac.utils import generate_slug, get_default_ou @@ -859,7 +859,7 @@ class RolesCsvImportForm(LimitQuerysetFormMixin, forms.Form): self.add_error('import_file', error) -class HeavySelect2WidgetNoCache(HeavySelect2Widget): +class HeavySelect2MultipleWidgetNoCache(HeavySelect2MultipleWidget): class Media: js = ('authentic2/manager/js/select2_locale.js',) @@ -867,37 +867,50 @@ class HeavySelect2WidgetNoCache(HeavySelect2Widget): pass +class UserOrRoleField(forms.Field): + widget = HeavySelect2MultipleWidgetNoCache( + data_url='/', + attrs={'data-placeholder': _('Select a user or a role'), 'data-minimum-input-length': 3}, + ) + + class ChooseUserOrRoleForm(FormWithRequest, forms.Form): - user_or_role = forms.CharField(label=_('Add to role')) + user_or_roles = UserOrRoleField(label=_('Add to role')) action = forms.CharField(initial='add', widget=forms.HiddenInput) def __init__(self, *args, **kwargs): self.role = kwargs.pop('role', None) super().__init__(*args, **kwargs) - self.fields['user_or_role'].widget = HeavySelect2WidgetNoCache( - data_url=reverse('user-or-role-select2-json', kwargs={'pk': self.role.pk}), - attrs={'data-placeholder': _('Select a user or a role'), 'data-minimum-input-length': 3}, + self.fields['user_or_roles'].widget.data_url = reverse( + 'user-or-role-select2-json', kwargs={'pk': self.role.pk} ) def clean(self): super().clean() - try: - object_type, pk = self.cleaned_data.get('user_or_role', '').split('-') - pk = int(pk) - except (ValueError, TypeError): - return - - if object_type == 'user': + self.cleaned_data['users'] = set() + self.cleaned_data['roles'] = set() + for user_or_role in self.cleaned_data.get('user_or_roles', []): try: - self.cleaned_data['user'] = self.get_user_queryset(self.request.user, self.role).get(pk=pk) - except User.DoesNotExist: - return - elif object_type == 'role': - try: - self.cleaned_data['role'] = self.get_role_queryset(self.request.user, self.role).get(pk=pk) - except Role.DoesNotExist: + object_type, pk = user_or_role.split('-') + pk = int(pk) + except (ValueError, TypeError): return + if object_type == 'user': + try: + self.cleaned_data['users'].add( + self.get_user_queryset(self.request.user, self.role).get(pk=pk) + ) + except User.DoesNotExist: + return + elif object_type == 'role': + try: + self.cleaned_data['roles'].add( + self.get_role_queryset(self.request.user, self.role).get(pk=pk) + ) + except Role.DoesNotExist: + return + @staticmethod def get_role_queryset(user, role): qs = Role.objects.exclude(pk=role.pk) diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py index 28e37d5d..88219de8 100644 --- a/src/authentic2/manager/role_views.py +++ b/src/authentic2/manager/role_views.py @@ -223,16 +223,16 @@ class RoleMembersView(views.HideOUColumnMixin, RoleViewMixin, views.BaseSubTable action = form.cleaned_data['action'] if not self.can_manage_members: messages.warning(self.request, _('You are not authorized')) - elif 'user' in form.cleaned_data: - if action == 'add': - self.add_user(form.cleaned_data['user']) - elif action == 'remove': - self.remove_user(form.cleaned_data['user']) - elif 'role' in form.cleaned_data: - if action == 'add': - self.add_role(form.cleaned_data['role']) - elif action == 'remove': - self.remove_role(form.cleaned_data['role']) + if action == 'add': + user_action = self.add_user + role_action = self.add_role + elif action == 'remove': + user_action = self.remove_user + role_action = self.remove_role + for user in form.cleaned_data.get('users', []): + user_action(user) + for role in form.cleaned_data.get('roles', []): + role_action(role) return super().form_valid(form) def add_user(self, user): diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members_table.html b/src/authentic2/manager/templates/authentic2/manager/role_members_table.html index 060975b6..ab9928a4 100644 --- a/src/authentic2/manager/templates/authentic2/manager/role_members_table.html +++ b/src/authentic2/manager/templates/authentic2/manager/role_members_table.html @@ -6,5 +6,5 @@ {% endblock %} {% block table.tbody.last.column %} - {% if table.context.view.can_manage_members and row.record.direct %}{% endif %} + {% if table.context.view.can_manage_members and row.record.direct %}{% endif %} {% endblock %} diff --git a/tests/test_manager.py b/tests/test_manager.py index b78e6b08..38725aca 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -937,7 +937,7 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): # user can add members response = app.get('/manage/roles/%s/' % simple_role.pk) form = response.forms['add-member'] - form['user_or_role'].force_value('user-%s' % admin.pk) + form['user_or_roles'].force_value('user-%s' % admin.pk) response = form.submit().follow() assert simple_role in admin.roles.all() @@ -945,7 +945,7 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): q = response.pyquery.remove_namespaces() assert q('table tbody tr td .icon-remove-sign') token = str(response.context['csrf_token']) - params = {'action': 'remove', 'user_or_role': 'user-%s' % admin.pk, 'csrfmiddlewaretoken': token} + params = {'action': 'remove', 'user_or_roles': 'user-%s' % admin.pk, 'csrfmiddlewaretoken': token} app.post('/manage/roles/%s/' % simple_role.pk, params=params) assert simple_role not in admin.roles.all() @@ -981,7 +981,7 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): # user can add role as a member through role members form response = app.get('/manage/roles/%s/' % simple_role.pk) form = response.forms['add-member'] - form['user_or_role'].force_value('role-%s' % role.pk) + form['user_or_roles'].force_value('role-%s' % role.pk) response = form.submit().follow() assert role in simple_role.children() @@ -989,7 +989,7 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): q = response.pyquery.remove_namespaces() assert q('table tbody tr td .icon-remove-sign') token = str(response.context['csrf_token']) - params = {'action': 'remove', 'user_or_role': 'role-%s' % role.pk, 'csrfmiddlewaretoken': token} + params = {'action': 'remove', 'user_or_roles': 'role-%s' % role.pk, 'csrfmiddlewaretoken': token} app.post('/manage/roles/%s/' % simple_role.pk, params=params) assert role not in simple_role.children() diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py index 0765ec38..915d166a 100644 --- a/tests/test_role_manager.py +++ b/tests/test_role_manager.py @@ -563,7 +563,7 @@ def test_role_members_user_role_add_remove(app, superuser, settings, simple_role select2_json = request_select2(app, resp, term='Jôhn') assert len(select2_json['results']) == 1 form = resp.forms['add-member'] - form['user_or_role'].force_value(select2_json['results'][0]['id']) + form['user_or_roles'].force_value(select2_json['results'][0]['id']) resp = form.submit().follow() assert 'Jôhn Dôe' in resp.text @@ -573,12 +573,12 @@ def test_role_members_user_role_add_remove(app, superuser, settings, simple_role data_pks = [row.attrib['data-pk'] for row in resp.pyquery('table tbody tr')] assert data_pks == ['user-%s' % simple_user.pk] data_pk_args = [row.attrib['data-pk-arg'] for row in resp.pyquery('table tbody tr td a.js-remove-object')] - assert data_pk_args == ['user_or_role'] + assert data_pk_args == ['user_or_roles'] select2_json = request_select2(app, resp, term='role_ou1') assert len(select2_json['results']) == 1 form = resp.forms['add-member'] - form['user_or_role'].force_value(select2_json['results'][0]['id']) + form['user_or_roles'].force_value(select2_json['results'][0]['id']) resp = form.submit().follow() assert 'role_ou1' in resp.text @@ -588,22 +588,22 @@ def test_role_members_user_role_add_remove(app, superuser, settings, simple_role data_pks = [row.attrib['data-pk'] for row in resp.pyquery('table tbody tr')] assert data_pks == ['role-%s' % role_ou1.pk, 'user-%s' % simple_user.pk] data_pk_args = [row.attrib['data-pk-arg'] for row in resp.pyquery('table tbody tr td a.js-remove-object')] - assert data_pk_args == ['user_or_role', 'user_or_role'] + assert data_pk_args == ['user_or_roles', 'user_or_roles'] # simulate click on Jôhn Dôe delete icon token = str(resp.context['csrf_token']) - params = {'action': 'remove', 'user_or_role': 'user-%s' % simple_user.pk, 'csrfmiddlewaretoken': token} + params = {'action': 'remove', 'user_or_roles': 'user-%s' % simple_user.pk, 'csrfmiddlewaretoken': token} resp = app.post('/manage/roles/%s/' % simple_role.pk, params=params).follow() assert 'Jôhn Dôe' not in resp.text # simulate click on role_ou1 delete icon token = str(resp.context['csrf_token']) - params = {'action': 'remove', 'user_or_role': 'role-%s' % role_ou1.pk, 'csrfmiddlewaretoken': token} + params = {'action': 'remove', 'user_or_roles': 'role-%s' % role_ou1.pk, 'csrfmiddlewaretoken': token} resp = app.post('/manage/roles/%s/' % simple_role.pk, params=params).follow() assert 'role_ou1' not in resp.text # invalid choices are ignored for invalid_choice in ('', 'wrong-wrong', 'user-', 'user-xxx', 'role', 'user-99999'): form = resp.forms['add-member'] - form['user_or_role'].force_value(invalid_choice) + form['user_or_roles'].force_value(invalid_choice) resp = form.submit().maybe_follow() -- 2.34.1