From a16131de069a7bdb5734530269d8d5714d8537fa Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Mon, 26 Jul 2021 16:03:12 +0200
Subject: [PATCH 3/3] manager: controle role inheritance using table (#53481)

---
 src/authentic2/manager/forms.py               |   6 +-
 src/authentic2/manager/role_views.py          | 203 +++++++++---------
 src/authentic2/manager/tables.py              |  21 ++
 .../authentic2/manager/role_members.html      |  20 +-
 .../authentic2/manager/role_remove_child.html |  23 --
 .../manager/role_remove_parent.html           |  23 --
 .../authentic2/manager/roles_inheritance.html |  25 +++
 .../authentic2/manager/user_ou_roles.html     |   2 +-
 src/authentic2/manager/urls.py                |  10 -
 tests/test_manager.py                         | 189 +++++++++-------
 10 files changed, 262 insertions(+), 260 deletions(-)
 delete mode 100644 src/authentic2/manager/templates/authentic2/manager/role_remove_child.html
 delete mode 100644 src/authentic2/manager/templates/authentic2/manager/role_remove_parent.html
 create mode 100644 src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html

diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py
index dbef3854..c44b4c3a 100644
--- a/src/authentic2/manager/forms.py
+++ b/src/authentic2/manager/forms.py
@@ -133,12 +133,12 @@ class UsersForm(LimitQuerysetFormMixin, CssClass, forms.Form):
 
 
 class RolesForm(LimitQuerysetFormMixin, CssClass, forms.Form):
-
     roles = fields.ChooseRolesField(label=_('Add some roles'))
 
 
-class RoleParentsForm(LimitQuerysetFormMixin, CssClass, forms.Form):
-    roles = fields.ChooseManageableMemberRolesField(label=_('Add some roles'))
+class RoleParentForm(LimitQuerysetFormMixin, CssClass, forms.Form):
+    role = fields.ChooseManageableMemberRoleField(label=_('Add some roles'))
+    action = forms.CharField(initial='add', widget=forms.HiddenInput)
 
 
 class ChooseUserRoleForm(LimitQuerysetFormMixin, CssClass, forms.Form):
diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py
index 5eecb17f..d82cdb5e 100644
--- a/src/authentic2/manager/role_views.py
+++ b/src/authentic2/manager/role_views.py
@@ -21,7 +21,8 @@ from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.db import transaction
-from django.db.models import Count, F
+from django.db.models import BooleanField, Count, ExpressionWrapper, F, Prefetch
+from django.db.models.functions import Cast
 from django.db.models.query import Prefetch, Q
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
@@ -34,7 +35,7 @@ from authentic2 import data_transfer, hooks
 from authentic2.apps.journal.views import JournalViewWithContext
 from authentic2.forms.profile import modelform_factory
 from authentic2.utils.misc import redirect
-from django_rbac.utils import get_ou_model, get_permission_model, get_role_model
+from django_rbac.utils import get_ou_model, get_permission_model, get_role_model, get_role_parenting_model
 
 from . import app_settings, forms, resources, tables, views
 from .journal_views import BaseJournalView
@@ -365,135 +366,129 @@ class RoleMembersExportView(views.ExportMixin, RoleMembersView):
 members_export = RoleMembersExportView.as_view()
 
 
-class RoleAddChildView(
-    views.AjaxFormViewMixin,
-    views.TitleMixin,
-    views.PermissionMixin,
-    views.FormNeedsRequest,
-    SingleObjectMixin,
-    FormView,
-):
+class RoleAddChildView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTableView):
     title = _('Add child role')
-    model = get_role_model()
-    form_class = forms.RolesForm
-    success_url = '..'
-    template_name = 'authentic2/manager/form.html'
+    form_class = forms.ChooseRoleForm
+    table_class = tables.InheritanceRolesTable
+    search_form_class = forms.RoleSearchForm
+    template_name = 'authentic2/manager/roles_inheritance.html'
     permissions = ['a2_rbac.manage_members_role']
+    success_url = '.'
+    slug_field = 'uuid'
 
-    def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        return super().dispatch(request, *args, **kwargs)
+    def get_table_queryset(self):
+        qs = super().get_table_queryset()
+        qs = qs.exclude(pk=self.object.pk)
+        children = self.object.children(annotate=True, include_self=False)
+        children = children.annotate(is_direct=Cast('direct', output_field=BooleanField()))
+        qs = qs.annotate(
+            checked=ExpressionWrapper(Q(pk__in=children.filter(is_direct=True)), output_field=BooleanField())
+        )
+        qs = qs.annotate(
+            indeterminate=ExpressionWrapper(
+                Q(pk__in=children.filter(is_direct=False)), output_field=BooleanField()
+            )
+        )
+        RoleParenting = get_role_parenting_model()
+        rp_qs = RoleParenting.objects.filter(parent__in=children).annotate(name=F('parent__name'))
+        qs = qs.prefetch_related(Prefetch('parent_relation', queryset=rp_qs, to_attr='via'))
+        return qs
 
     def form_valid(self, form):
-        parent = self.get_object()
-        for role in form.cleaned_data['roles']:
-            parent.add_child(role)
+        role = form.cleaned_data['role']
+        action = form.cleaned_data['action']
+        if action == 'add':
+            self.object.add_child(role)
+            hooks.call_hooks(
+                'event', name='manager-add-child-role', user=self.request.user, parent=self.object, child=role
+            )
+            self.request.journal.record('manager.role.inheritance.addition', parent=self.object, child=role)
+        elif action == 'remove':
+            self.object.remove_child(role)
             hooks.call_hooks(
-                'event', name='manager-add-child-role', user=self.request.user, parent=parent, child=role
+                'event',
+                name='manager-remove-child-role',
+                user=self.request.user,
+                parent=self.object,
+                child=role,
             )
-            self.request.journal.record('manager.role.inheritance.addition', parent=parent, child=role)
+            self.request.journal.record('manager.role.inheritance.removal', parent=self.object, child=role)
         return super().form_valid(form)
 
+    def get_search_form_kwargs(self):
+        kwargs = super().get_search_form_kwargs()
+        kwargs['queryset'] = self.request.user.filter_by_perm(
+            'a2_rbac.view_role', get_role_model().objects.all()
+        )
+        return kwargs
+
 
 add_child = RoleAddChildView.as_view()
 
 
-class RoleAddParentView(
-    views.AjaxFormViewMixin, views.TitleMixin, views.FormNeedsRequest, SingleObjectMixin, FormView
-):
+class RoleAddParentView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTableView):
     title = _('Add parent role')
-    model = get_role_model()
-    form_class = forms.RoleParentsForm
-    success_url = '..'
-    template_name = 'authentic2/manager/form.html'
+    form_class = forms.RoleParentForm
+    table_class = tables.InheritanceRolesTable
+    search_form_class = forms.RoleSearchForm
+    template_name = 'authentic2/manager/roles_inheritance.html'
+    success_url = '.'
+    slug_field = 'uuid'
 
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if self.object.is_internal():
+        if self.get_object().is_internal():
             raise PermissionDenied
         return super().dispatch(request, *args, **kwargs)
 
+    def get_table_queryset(self):
+        qs = super().get_table_queryset()
+        qs = self.request.user.filter_by_perm('a2_rbac.manage_members_role', qs)
+        qs = qs.exclude(pk=self.object.pk)
+        parents = self.object.parents(annotate=True, include_self=False)
+        parents = parents.annotate(is_direct=Cast('direct', output_field=BooleanField()))
+        qs = qs.annotate(
+            checked=ExpressionWrapper(Q(pk__in=parents.filter(is_direct=True)), output_field=BooleanField())
+        )
+        qs = qs.annotate(
+            indeterminate=ExpressionWrapper(
+                Q(pk__in=parents.filter(is_direct=False)), output_field=BooleanField()
+            )
+        )
+        RoleParenting = get_role_parenting_model()
+        rp_qs = RoleParenting.objects.filter(child__in=parents).annotate(name=F('child__name'))
+        qs = qs.prefetch_related(Prefetch('child_relation', queryset=rp_qs, to_attr='via'))
+        return qs
+
     def form_valid(self, form):
-        child = self.get_object()
-        for role in form.cleaned_data['roles']:
-            child.add_parent(role)
+        role = form.cleaned_data['role']
+        action = form.cleaned_data['action']
+        if action == 'add':
+            self.object.add_parent(role)
             hooks.call_hooks(
-                'event', name='manager-add-child-role', user=self.request.user, parent=role, child=child
+                'event', name='manager-add-child-role', user=self.request.user, parent=role, child=self.object
+            )
+            self.request.journal.record('manager.role.inheritance.addition', parent=role, child=self.object)
+        elif action == 'remove':
+            self.object.remove_parent(role)
+            hooks.call_hooks(
+                'event',
+                name='manager-remove-child-role',
+                user=self.request.user,
+                parent=role,
+                child=self.object,
             )
-            self.request.journal.record('manager.role.inheritance.addition', parent=role, child=child)
+            self.request.journal.record('manager.role.inheritance.removal', parent=role, child=self.object)
         return super().form_valid(form)
 
-
-add_parent = RoleAddParentView.as_view()
-
-
-class RoleRemoveChildView(views.AjaxFormViewMixin, SingleObjectMixin, views.PermissionMixin, TemplateView):
-    title = _('Remove child role')
-    model = get_role_model()
-    success_url = '../..'
-    template_name = 'authentic2/manager/role_remove_child.html'
-    permissions = ['a2_rbac.manage_members_role']
-
-    def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        self.child = self.get_queryset().get(pk=kwargs['child_pk'])
-        return super().dispatch(request, *args, **kwargs)
-
-    def get_context_data(self, **kwargs):
-        ctx = super().get_context_data(**kwargs)
-        ctx['child'] = self.child
-        return ctx
-
-    def post(self, request, *args, **kwargs):
-        self.object.remove_child(self.child)
-        hooks.call_hooks(
-            'event',
-            name='manager-remove-child-role',
-            user=self.request.user,
-            parent=self.object,
-            child=self.child,
-        )
-        self.request.journal.record('manager.role.inheritance.removal', parent=self.object, child=self.child)
-        return redirect(self.request, self.success_url)
-
-
-remove_child = RoleRemoveChildView.as_view()
-
-
-class RoleRemoveParentView(views.AjaxFormViewMixin, SingleObjectMixin, TemplateView):
-    title = _('Remove parent role')
-    model = get_role_model()
-    success_url = '../..'
-    template_name = 'authentic2/manager/role_remove_parent.html'
-
-    def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if self.object.is_internal():
-            raise PermissionDenied
-        self.parent = self.get_queryset().get(pk=kwargs['parent_pk'])
-        return super().dispatch(request, *args, **kwargs)
-
-    def get_context_data(self, **kwargs):
-        ctx = super().get_context_data(**kwargs)
-        ctx['parent'] = self.parent
-        return ctx
-
-    def post(self, request, *args, **kwargs):
-        if not self.request.user.has_perm('a2_rbac.manage_members_role', self.parent):
-            raise PermissionDenied
-        self.object.remove_parent(self.parent)
-        hooks.call_hooks(
-            'event',
-            name='manager-remove-child-role',
-            user=self.request.user,
-            parent=self.parent,
-            child=self.object,
+    def get_search_form_kwargs(self):
+        kwargs = super().get_search_form_kwargs()
+        kwargs['queryset'] = self.request.user.filter_by_perm(
+            'a2_rbac.manage_members_role', get_role_model().objects.all()
         )
-        self.request.journal.record('manager.role.inheritance.removal', parent=self.parent, child=self.object)
-        return redirect(self.request, self.success_url)
+        return kwargs
 
 
-remove_parent = RoleRemoveParentView.as_view()
+add_parent = RoleAddParentView.as_view()
 
 
 class RoleAddAdminRoleView(
diff --git a/src/authentic2/manager/tables.py b/src/authentic2/manager/tables.py
index 930d751d..9321152d 100644
--- a/src/authentic2/manager/tables.py
+++ b/src/authentic2/manager/tables.py
@@ -240,3 +240,24 @@ class UserAuthorizationsTable(tables.Table):
         attrs = {'class': 'main plaintable', 'id': 'user-authorizations-table'}
         fields = ('client', 'created', 'expired')
         empty_text = _('This user has not granted profile data access to any service yet.')
+
+
+class InheritanceRolesTable(tables.Table):
+    name = tables.LinkColumn(
+        viewname='a2-manager-role-members', kwargs={'pk': A('pk')}, accessor='name', verbose_name=_('label')
+    )
+    via = tables.TemplateColumn(
+        '''{% for rel in record.via %}{{ rel.name }}{% if not forloop.last %}, {% endif %}{% endfor %}''',
+        verbose_name=_('Inherited from'),
+        orderable=False,
+    )
+    member = tables.TemplateColumn(
+        '<input class="role-member{% if record.indeterminate %} indeterminate{% endif %}" name="role-{{ record.pk }}" type="checkbox" {% if record.checked %}checked{% endif %}/>',
+        verbose_name='',
+    )
+
+    class Meta:
+        model = get_role_model()
+        attrs = {'class': 'main plaintable', 'id': 'inheritance-role-table'}
+        fields = ('name', 'ou')
+        empty_text = _('None')
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members.html b/src/authentic2/manager/templates/authentic2/manager/role_members.html
index b6594bf4..f31412f4 100644
--- a/src/authentic2/manager/templates/authentic2/manager/role_members.html
+++ b/src/authentic2/manager/templates/authentic2/manager/role_members.html
@@ -109,14 +109,10 @@
    {% trans "Child roles:" %}
    {% for child in children %}
      <a href="{% url "a2-manager-role-members" pk=child.pk %}">{{ child }}</a>
-     {% if child.direct %}
-       <a rel="popup" href="{% url "a2-manager-role-remove-child" pk=object.pk child_pk=child.pk %}" class="role-remove icon-minus-sign"></a>
-     {% else %}
-       <a title="{% trans "Indirect child role" %}" class="disabled role-remove icon-minus-sign"></a>
-     {% endif %}
+     {% if not forloop.last %} − {% endif %}
    {% endfor %}
   {% if view.can_manage_members %}
-    <a rel="popup" href="{% url "a2-manager-role-add-child" pk=object.pk %}" class="role-add icon-add-sign"></a>
+    <a href="{% url "a2-manager-role-add-child" pk=object.pk %}" class="role-add icon-add-sign"></a>
   {% else %}
     <a title="{% trans "Permission denied" %}" class="disabled role-add icon-add-sign"></a>
   {% endif %}
@@ -127,18 +123,10 @@
      <a class="role" href="{% url "a2-manager-role-members" pk=parent.pk %}">
        {% if parent.ou and has_multiple_ou %}{{ parent.ou }} - {% endif %}{{ parent }}
      </a>
-     {% if parent.direct %}
-       {% if not object.is_internal %}
-         <a rel="popup" href="{% url "a2-manager-role-remove-parent" pk=object.pk parent_pk=parent.pk %}" class="role-remove icon-minus-sign"></a>
-       {% else %}
-         <a title="{% trans "This role is technical, you cannot modify its permissions." %}" class="disabled role-add icon-minus-sign"></a>
-       {% endif %}
-     {% else %}
-       <a title="{% trans "Indirect parent role" %}" class="disabled role-remove icon-minus-sign"></a>
-     {% endif %}
+    {% if not forloop.last %} − {% endif %}
    {% endfor %}
    {% if not object.is_internal %}
-     <a rel="popup" href="{% url "a2-manager-role-add-parent" pk=object.pk %}" class="role-add icon-add-sign"></a>
+     <a href="{% url "a2-manager-role-add-parent" pk=object.pk %}" class="role-add icon-add-sign"></a>
    {% else %}
      <a title="{% trans "This role is technical, you cannot modify its permissions." %}" class="disabled role-add icon-add-sign"></a>
    {% endif %}
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_remove_child.html b/src/authentic2/manager/templates/authentic2/manager/role_remove_child.html
deleted file mode 100644
index eae32f0f..00000000
--- a/src/authentic2/manager/templates/authentic2/manager/role_remove_child.html
+++ /dev/null
@@ -1,23 +0,0 @@
-{% extends "authentic2/manager/base.html" %}
-{% load i18n %}
-
-{% block messages %}
-{% endblock %}
-
-{% block main %}
-  {% if title %}
-    <div id="appbar"><h2>{{ title }}</h2></div>
-  {% endif %}
-  <form method="post">
-    {% csrf_token %}
-    <div class="form-inner-container">
-      {% block caption %}
-      <p>{% blocktrans %}Do you want to remove child role {{ child }} ?{% endblocktrans %}</p>
-      {% endblock %}
-      <div class="buttons">
-        <button>{% trans "Remove" %}</button>
-        <a class="cancel" href="..">{% trans "Cancel" %}</a>
-      </div>
-    </div>
-  </form>
-{% endblock %}
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_remove_parent.html b/src/authentic2/manager/templates/authentic2/manager/role_remove_parent.html
deleted file mode 100644
index c6f48100..00000000
--- a/src/authentic2/manager/templates/authentic2/manager/role_remove_parent.html
+++ /dev/null
@@ -1,23 +0,0 @@
-{% extends "authentic2/manager/base.html" %}
-{% load i18n %}
-
-{% block messages %}
-{% endblock %}
-
-{% block main %}
-  {% if title %}
-    <div id="appbar"><h2>{{ title }}</h2></div>
-  {% endif %}
-  <form method="post">
-    {% csrf_token %}
-    <div class="form-inner-container">
-      {% block caption %}
-      <p>{% blocktrans %}Do you want to remove parent role {{ parent }} ?{% endblocktrans %}</p>
-      {% endblock %}
-      <div class="buttons">
-        <button>{% trans "Remove" %}</button>
-        <a class="cancel" href="..">{% trans "Cancel" %}</a>
-      </div>
-    </div>
-  </form>
-{% endblock %}
diff --git a/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html b/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html
new file mode 100644
index 00000000..7d7d9dd8
--- /dev/null
+++ b/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html
@@ -0,0 +1,25 @@
+{% extends "authentic2/manager/role_common.html" %}
+{% load i18n static django_tables2 %}
+
+{% block breadcrumb %}
+  {{ block.super }}
+  <a href="..">{{ object }}</a>
+  <a href="#">{% trans "Role inheritance" %}</a>
+{% endblock %}
+
+{% block extrascripts %}
+  {{ block.super }}
+  <script src="{% static "authentic2/manager/js/roles_ajax_checkbox.js" %}"></script>
+{% endblock %}
+
+{% block main %}
+ {% with row_link=0 %}
+   {% render_table table "authentic2/manager/table.html" %}
+ {% endwith %}
+{% endblock %}
+
+{% block sidebar %}
+  <aside id="sidebar">
+    {% include "authentic2/manager/search_form.html" %}
+  </aside>
+{% endblock %}
diff --git a/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html b/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html
index 6225e476..f73d20d2 100644
--- a/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html
+++ b/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html
@@ -1,5 +1,5 @@
 {% extends "authentic2/manager/user_common_roles.html" %}
-{% load django_tables2 %}
+{% load static django_tables2 %}
 
 {% block extrascripts %}
   {{ block.super }}
diff --git a/src/authentic2/manager/urls.py b/src/authentic2/manager/urls.py
index 4e5483bc..26225d3c 100644
--- a/src/authentic2/manager/urls.py
+++ b/src/authentic2/manager/urls.py
@@ -128,16 +128,6 @@ urlpatterns = required(
         url(r'^roles/(?P<pk>\d+)/$', role_views.members, name='a2-manager-role-members'),
         url(r'^roles/(?P<pk>\d+)/add-child/$', role_views.add_child, name='a2-manager-role-add-child'),
         url(r'^roles/(?P<pk>\d+)/add-parent/$', role_views.add_parent, name='a2-manager-role-add-parent'),
-        url(
-            r'^roles/(?P<pk>\d+)/remove-child/(?P<child_pk>\d+)/$',
-            role_views.remove_child,
-            name='a2-manager-role-remove-child',
-        ),
-        url(
-            r'^roles/(?P<pk>\d+)/remove-parent/(?P<parent_pk>\d+)/$',
-            role_views.remove_parent,
-            name='a2-manager-role-remove-parent',
-        ),
         url(
             r'^roles/(?P<pk>\d+)/add-admin-user/$',
             role_views.add_admin_user,
diff --git a/tests/test_manager.py b/tests/test_manager.py
index d282d6fd..58070471 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -861,15 +861,15 @@ def test_roles_widget(admin, app, db):
 
 
 def test_roles_for_change_widget(admin, app, db):
-    from authentic2.manager.forms import RoleParentsForm
+    from authentic2.manager.forms import RoleParentForm
 
     login(app, admin, '/manage/')
     Role.objects.create(name='admin 1')
     Role.objects.create(name='user 1')
 
-    form = RoleParentsForm(request=None)
+    form = RoleParentForm(request=None)
     assert form.as_p()
-    field_id = form.fields['roles'].widget.build_attrs({})['data-field_id']
+    field_id = form.fields['role'].widget.build_attrs({})['data-field_id']
     url = reverse('django_select2-json')
     response = app.get(url, params={'field_id': field_id, 'term': 'admin'})
     assert len(response.json['results']) == 1
@@ -940,28 +940,35 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role):
     simple_role.permissions.add(view_role_perm)
     simple_user.roles.add(simple_role)
     admin.roles.add(role)
+
     response = app.get('/manage/roles/%s/add-child/' % simple_role.pk)
-    form = response.form
-    form['roles'].force_value(role.pk)
-    form.submit().follow()
+    token = str(response.context['csrf_token'])
+    params = {'action': 'add', 'role': role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/roles/%s/add-child/' % simple_role.pk, params=params)
     assert role in simple_role.children()
 
-    url = '/manage/roles/%s/remove-child/%s/' % (simple_role.pk, role.pk)
-    token = str(response.context['csrf_token'])
-    app.post(url, params={'csrfmiddlewaretoken': token})
-    assert not role in simple_role.children()
+    params = {'action': 'remove', 'role': role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/roles/%s/add-child/' % simple_role.pk, params=params)
+    assert role not in simple_role.children()
 
     response = app.get('/manage/roles/%s/add-parent/' % role.pk)
-    form = response.form
-    form['roles'].force_value(simple_role.pk)
-    form.submit().follow()
+    token = str(response.context['csrf_token'])
+    params = {'action': 'add', 'role': simple_role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/roles/%s/add-parent/' % role.pk, params=params)
     assert simple_role in role.parents()
 
-    url = '/manage/roles/%s/remove-parent/%s/' % (role.pk, simple_role.pk)
-    token = str(response.context['csrf_token'])
-    app.post(url, params={'csrfmiddlewaretoken': token})
+    params = {'action': 'remove', 'role': simple_role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/roles/%s/add-parent/' % role.pk, params=params)
     assert simple_role not in role.parents()
 
+    # try to add arbitrary role
+    admin_role = Role.objects.get(slug='_a2-manager')
+    response = app.get('/manage/roles/%s/add-parent/' % role.pk)
+    token = str(response.context['csrf_token'])
+    params = {'action': 'add', 'role': admin_role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/roles/%s/add-parent/' % simple_role.pk, params=params)
+    assert admin_role not in role.parents()
+
     # user roles view works
     response = app.get('/manage/users/%s/roles/' % admin.pk)
     q = response.pyquery.remove_namespaces()
@@ -978,33 +985,13 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role):
     app.get('/manage/roles/%s/delete/' % simple_role.pk, status=403)
 
 
-def test_manager_permission_inheritance(app, simple_user, admin, simple_role):
-    admin_role = Role.objects.get(slug='_a2-manager')
-    view_role_perm = get_permission_model().objects.create(
-        operation=get_operation(VIEW_OP),
-        target_ct=ContentType.objects.get_for_model(Role),
-        target_id=simple_role.pk,
-    )
-    simple_role.permissions.add(view_role_perm)
-    simple_user.roles.add(simple_role)
-    login(app, simple_user, '/manage/')
-
-    response = app.get('/manage/roles/%s/add-parent/' % simple_role.pk)
-    form = response.form
-    form['roles'].force_value(admin_role.pk)
-    response = form.submit()
-
-    assert response.status_code == 200
-    assert not admin_role in simple_role.parents()
-
-
 def test_manager_widget_fields_validation(app, simple_user, simple_role):
     '''Verify that fields corresponding to widget implement queryset restrictions.'''
     from authentic2.manager.forms import (
         ChooseRoleForm,
         ChooseUserForm,
         ChooseUserRoleForm,
-        RoleParentsForm,
+        RoleParentForm,
         RolesForm,
         UsersForm,
     )
@@ -1056,8 +1043,8 @@ def test_manager_widget_fields_validation(app, simple_user, simple_role):
     assert error_message in form.errors['roles'][0]
 
     # For those we need manage_members permission
-    form = RoleParentsForm(request=request, data={'roles': [visible_role.pk]})
-    assert error_message in form.errors['roles'][0]
+    form = RoleParentForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
+    assert error_message in form.errors['role'][0]
 
     form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
     assert error_message in form.errors['role'][0]
@@ -1070,66 +1057,108 @@ def test_manager_widget_fields_validation(app, simple_user, simple_role):
     simple_role.permissions.add(change_role_perm)
     del simple_user._rbac_perms_cache
 
-    form = RoleParentsForm(request=request, data={'roles': [visible_role.pk]})
+    form = RoleParentForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
     assert form.is_valid()
 
     form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'})
     assert form.is_valid()
 
 
-def test_manager_role_widgets_choices(app, simple_user, simple_role):
-    def get_choices(response):
-        select2_json = request_select2(app, response)
-        assert select2_json['more'] is False
-        return {result['id'] for result in select2_json['results']}
+@pytest.mark.parametrize('relation', ['child', 'parent'])
+def test_manager_role_inheritance_list(app, admin, simple_role, ou1, relation):
+    first_role = Role.objects.create(name='first_role', ou=simple_role.ou)
+    second_role = Role.objects.create(name='second_role', ou=simple_role.ou)
+    third_role = Role.objects.create(name='third_role', ou=ou1)
 
+    if relation == 'child':
+        simple_role.add_child(first_role)
+        first_role.add_child(second_role)
+    elif relation == 'parent':
+        simple_role.add_parent(first_role)
+        first_role.add_parent(second_role)
+
+    response = login(app, admin)
+    response = app.get('/manage/roles/%s/add-%s/' % (simple_role.pk, relation))
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 3
+    assert {e.text_content() for e in q('table tbody td.name')} == {
+        first_role.name,
+        second_role.name,
+        third_role.name,
+    }
+
+    row = q('table tbody tr')[0]
+    name, ou, via, member = row.getchildren()
+    assert name.text_content() == 'first_role'
+    assert ou.text_content() == 'Default organizational unit'
+    assert not via.text_content()
+    member = member.find('input')
+    assert member.checked
+    assert member.attrib['class'] == 'role-member'
+
+    row = q('table tbody tr')[1]
+    name, ou, via, member = row.getchildren()
+    assert name.text_content() == 'second_role'
+    assert ou.text_content() == 'Default organizational unit'
+    assert via.text_content() == 'first_role'
+    member = member.find('input')
+    assert not member.checked
+    assert member.attrib['class'] == 'role-member indeterminate'
+
+    row = q('table tbody tr')[2]
+    name, ou, via, member = row.getchildren()
+    assert name.text_content() == 'third_role'
+    assert ou.text_content() == 'OU1'
+    assert not via.text_content()
+    member = member.find('input')
+    assert not member.checked
+    assert member.attrib['class'] == 'role-member'
+
+
+def test_manager_role_inheritance_list_search_permission(app, admin, simple_user, simple_role, ou1):
     visible_role = Role.objects.create(name='visible_role', ou=simple_user.ou)
-    Role.objects.create(name='invisible_role', ou=simple_user.ou)
+    visible_role_2 = Role.objects.create(name='visible_role_2', ou=ou1)
+    invisible_role = Role.objects.create(name='invisible_role', ou=simple_user.ou)
     admin_of_simple_role = simple_role.get_admin_role()
 
     admin_of_simple_role.members.add(simple_user)
-    view_role_perm = get_permission_model().objects.create(
-        operation=get_operation(VIEW_OP),
-        target_ct=ContentType.objects.get_for_model(Role),
-        target_id=visible_role.pk,
-    )
-    simple_role.permissions.add(view_role_perm)
+    for role in (visible_role, visible_role_2):
+        view_role_perm = get_permission_model().objects.create(
+            operation=get_operation(VIEW_OP),
+            target_ct=ContentType.objects.get_for_model(Role),
+            target_id=role.pk,
+        )
+        simple_role.permissions.add(view_role_perm)
     simple_user.roles.add(simple_role)
 
     response = login(app, simple_user, '/manage/roles/')
 
-    # all visible roles are shown
+    # all visible roles are shown, except current role
     response = app.get('/manage/roles/%s/add-child/' % simple_role.pk)
-    assert {visible_role.pk, simple_role.pk} == get_choices(response)
-
-    # all roles with manage_members permissions are shown
-    response = app.get('/manage/roles/%s/add-parent/' % simple_role.pk)
-    assert {simple_role.pk, admin_of_simple_role.pk} == get_choices(response)
-
-    response = app.get('/manage/roles/%s/add-parent/' % visible_role.pk)
-    assert {simple_role.pk, admin_of_simple_role.pk} == get_choices(response)
-
-
-def test_manager_widgets_field_id_other_user(app, admin, simple_user, simple_role):
-    other_role = Role.objects.create(name='visible_role', ou=simple_user.ou)
-    simple_role.get_admin_role().members.add(simple_user)
-
-    response = login(app, admin, '/manage/roles/%s/add-child/' % simple_role.pk)
-    select2_json = request_select2(app, response)
-    assert select2_json['more'] is False
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 2
+    assert {e.text_content() for e in q('table tbody td.name')} == {visible_role.name, visible_role_2.name}
 
-    # admin can see every roles
-    assert {simple_role.pk, other_role.pk} == {result['id'] for result in select2_json['results']}
+    # filter by ou
+    response.form['search-ou'] = ou1.pk
+    response = response.form.submit()
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 1
+    assert {e.text_content() for e in q('table tbody td.name')} == {visible_role_2.name}
 
-    login(app, simple_user)
-    # same request from the page served for admin
-    select2_json = request_select2(app, response)
-    # simple_user doesn't see all roles
-    assert simple_role.pk == select2_json['results'][0]['id']
+    # filter by name
+    response.form['search-text'] = '2'
+    response.form['search-ou'] = 'all'
+    response = response.form.submit()
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 1
+    assert {e.text_content() for e in q('table tbody td.name')} == {visible_role_2.name}
 
-    # anymous user receive 404
-    app.session.flush()
-    select2_json = request_select2(app, response, get_kwargs={'status': 404})
+    # all roles with manage_members permissions are shown
+    response = app.get('/manage/roles/%s/add-parent/' % visible_role.pk)
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 1
+    assert {e.text_content() for e in q('table tbody td.name')} == {simple_role.name}
 
 
 def test_display_parent_roles_on_role_page(app, superuser, settings):
-- 
2.20.1