From 9fc8636547e663b33b5e6fc247fdeec600883012 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 3 Nov 2021 11:11:38 +0100 Subject: [PATCH 3/4] manager: include roles along with users in role members table (#57955) --- src/authentic2/manager/forms.py | 10 ++++ src/authentic2/manager/role_views.py | 34 +++++++++-- src/authentic2/manager/tables.py | 24 +++++++- .../authentic2/manager/role_members.html | 2 + src/authentic2/manager/views.py | 2 +- tests/test_role_manager.py | 58 ++++++++++++++++++- 6 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index d5f9915d..2f21a92c 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -590,6 +590,16 @@ class UserSearchForm(OUSearchForm, CssClass, PrefixFormMixin, FormWithRequest): return qs +class RoleMembersSearchForm(UserSearchForm): + all_members = forms.BooleanField(initial=False, label=_('View all members'), required=False) + + def __init__(self, *args, **kwargs): + disable_all_members = kwargs.pop('disable_all_members', False) + super().__init__(*args, **kwargs) + if disable_all_members: + self.fields['all_members'].widget.attrs['disabled'] = True + + class UserAddChooseOUForm(OUSearchForm): ou_permission = 'custom_user.add_user' diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py index f0beb15f..5c790061 100644 --- a/src/authentic2/manager/role_views.py +++ b/src/authentic2/manager/role_views.py @@ -171,12 +171,17 @@ edit = RoleEditView.as_view() class RoleMembersView(views.HideOUColumnMixin, RoleViewMixin, views.BaseSubTableView): template_name = 'authentic2/manager/role_members.html' - table_class = tables.RoleMembersTable form_class = forms.ChooseUserForm success_url = '.' - search_form_class = forms.UserSearchForm + search_form_class = forms.RoleMembersSearchForm permissions = ['a2_rbac.view_role'] + @property + def table_class(self): + if self.view_all_members: + return tables.RoleMembersTable + return tables.MixedUserRoleTable + @property def title(self): return self.get_instance_name() @@ -189,11 +194,25 @@ class RoleMembersView(views.HideOUColumnMixin, RoleViewMixin, views.BaseSubTable def can_manage_members(self, value): self._can_manage_members = value + def dispatch(self, request, *args, **kwargs): + self.children = views.filter_view(self.request, self.get_object().children(include_self=False)) + return super().dispatch(request, *args, **kwargs) + + def get_table_data(self): + if self.view_all_members: + return super().get_table_data() + members = views.filter_view(self.request, self.object.members.all()) + members = self.filter_by_search(members) + return list(self.children) + list(members) + def get_table_queryset(self): - children = self.object.children(include_self=False) - via_prefetch = Prefetch('roles', queryset=children, to_attr='via') + via_prefetch = Prefetch('roles', queryset=self.children, to_attr='via') return self.object.all_members().prefetch_related(via_prefetch) + @property + def view_all_members(self): + return self.search_form.is_valid() and self.search_form.cleaned_data.get('all_members') + def form_valid(self, form): user = form.cleaned_data['user'] action = form.cleaned_data['action'] @@ -239,6 +258,13 @@ class RoleMembersView(views.HideOUColumnMixin, RoleViewMixin, views.BaseSubTable kwargs['ou'] = self.object.ou return kwargs + def get_search_form_kwargs(self): + kwargs = super().get_search_form_kwargs() + if not self.children: + kwargs['data']['search-all_members'] = 'on' + kwargs['disable_all_members'] = True + return kwargs + def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) ctx['children'] = list( diff --git a/src/authentic2/manager/tables.py b/src/authentic2/manager/tables.py index 7d271efd..7b655ebc 100644 --- a/src/authentic2/manager/tables.py +++ b/src/authentic2/manager/tables.py @@ -28,6 +28,7 @@ from authentic2_idp_oidc.models import OIDCAuthorization from django_rbac.utils import get_ou_model, get_permission_model, get_role_model User = get_user_model() +Role = get_role_model() class PermissionLinkColumn(tables.LinkColumn): @@ -51,9 +52,9 @@ class VerifiableEmailColumn(tables.Column): class UserLinkColumn(PermissionLinkColumn): def render(self, **kwargs): - user = kwargs['record'] + record = kwargs['record'] value = super().render(**kwargs) - if not user.is_active: + if isinstance(record, User) and not record.is_active: value = html.format_html( '{value} ({disabled})', value=value, disabled=_('disabled') ) @@ -91,6 +92,25 @@ class RoleMembersTable(UserTable): pass +class UserOrRoleColumn(UserLinkColumn): + def render(self, **kwargs): + value = super().render(**kwargs) + if isinstance(kwargs['record'], Role): + value = html.format_html(_('Members of role {value}'), value=value) + return value + + +class MixedUserRoleTable(tables.Table): + name = UserOrRoleColumn( + verbose_name=_('Members'), + text=lambda x: x.get_full_name() if isinstance(x, User) else x.name, + orderable=False, + ) + + class Meta: + attrs = {'class': 'main', 'id': 'user-table'} + + class RoleTable(tables.Table): name = tables.LinkColumn( viewname='a2-manager-role-members', kwargs={'pk': A('pk')}, accessor='name', verbose_name=_('label') diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members.html b/src/authentic2/manager/templates/authentic2/manager/role_members.html index f89e2f11..cfbbd711 100644 --- a/src/authentic2/manager/templates/authentic2/manager/role_members.html +++ b/src/authentic2/manager/templates/authentic2/manager/role_members.html @@ -62,7 +62,9 @@ {% render_table table "authentic2/manager/role_members_table.html" %} {% endwith %} + {% if search_form.cleaned_data.all_members %} {% include "authentic2/manager/export_include.html" with export_view_name="a2-manager-role-members-export" %} + {% endif %} {% if view.can_manage_members %}
diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py index 6d695a4b..bb6eb9df 100644 --- a/src/authentic2/manager/views.py +++ b/src/authentic2/manager/views.py @@ -164,7 +164,7 @@ class SearchFormMixin: return self.search_form_class def get_search_form_kwargs(self): - return {'request': self.request, 'data': self.request.GET} + return {'request': self.request, 'data': self.request.GET.dict()} def get_search_form(self): form_class = self.get_search_form_class() diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py index 8b1a8c2e..aee1336f 100644 --- a/tests/test_role_manager.py +++ b/tests/test_role_manager.py @@ -119,6 +119,8 @@ def test_role_members_via(app, admin): user2.roles.add(role2) response = login(app, admin, '/manage/roles/%s/' % role1.id) + response.forms['search-form']['search-all_members'] = True + response = response.forms['search-form'].submit() rows = list( zip( [text_content(el) for el in response.pyquery('tr td.username')], @@ -375,7 +377,7 @@ def test_role_members_display_inheritance_info(app, superuser, settings, simple_ role = Role.objects.create(name='Role a', ou=get_default_ou()) getattr(simple_role, 'add_%s' % relation)(role) resp = app.get(url) - assert 'Role a' not in resp.text + assert not resp.pyquery('a.role-inheritance-%s:contains("Role a")' % relation) assert '(view all roles)' in resp.text resp = resp.click('(view all roles)', href=relation) @@ -404,3 +406,57 @@ def test_role_members_display_inheritance_info(app, superuser, settings, simple_ 'ou1 - Role 2', 'ou1 - Role 3', ] + + +def test_role_members_user_role_mixed_table(app, superuser, settings, simple_role, simple_user): + simple_user.roles.add(simple_role) + + url = reverse('a2-manager-role-members', kwargs={'pk': simple_role.pk}) + resp = login(app, superuser, url) + + # no children, directly display members details + assert resp.forms['search-form']['search-all_members'].value == 'on' + assert 'disabled' in resp.forms['search-form']['search-all_members'].attrs + assert 'Download list as CSV' in resp.text + + column_names = [text_content(el) for el in resp.pyquery('table#user-table th') if text_content(el)] + assert column_names == [ + 'User', + 'Username', + 'Email Address', + 'First Name', + 'Last Name', + 'Organizational Unit', + 'Direct member', + 'Inherited from', + ] + + rows = [text_content(el) for el in resp.pyquery('tr td.link')] + assert rows == ['Jôhn Dôe'] + + # add child + role = Role.objects.create(name='Role a', ou=get_default_ou()) + user = User.objects.create(username='user1') + user.roles.add(role) + simple_role.add_child(role) + + resp = app.get(url) + assert not resp.forms['search-form']['search-all_members'].value + assert 'disabled' not in resp.forms['search-form']['search-all_members'].attrs + assert 'Download list as CSV' not in resp.text + + column_names = [text_content(el) for el in resp.pyquery('table#user-table th') if text_content(el)] + assert column_names == ['Members'] + + rows = [text_content(el) for el in resp.pyquery('tr td.name')] + assert rows == ['Members of role Role a', 'Jôhn Dôe'] + + resp.forms['search-form']['search-all_members'] = True + resp = resp.forms['search-form'].submit() + + assert resp.forms['search-form']['search-all_members'].value == 'on' + assert 'disabled' not in resp.forms['search-form']['search-all_members'].attrs + assert 'Download list as CSV' in resp.text + + rows = [text_content(el) for el in resp.pyquery('tr td.link')] + assert rows == ['user1', 'Jôhn Dôe'] -- 2.30.2