From 1ee053fc9f3f422cd3b3889aa4a1d0ac702c2310 Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Thu, 6 Aug 2020 15:34:21 +0200
Subject: [PATCH] manager: group roles by OU on user page (#24397)

---
 .../templates/authentic2/manager/user_detail.html   |  4 ++--
 src/authentic2/manager/user_views.py                |  1 +
 tests/test_manager.py                               | 13 ++++++++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/authentic2/manager/templates/authentic2/manager/user_detail.html b/src/authentic2/manager/templates/authentic2/manager/user_detail.html
index e0a05e03..2acb73e0 100644
--- a/src/authentic2/manager/templates/authentic2/manager/user_detail.html
+++ b/src/authentic2/manager/templates/authentic2/manager/user_detail.html
@@ -65,25 +65,25 @@
 
   {% block other_actions %}{{ block.super }}{% endblock %}
 
   {% if roles_by_ou or can_change_roles %}
   <div class="user-roles">
     <h3>{% trans "Roles" %}</h3>
     <ul>
       {% for ou, ou_roles in roles_by_ou.items %}
-        {% if multiple_ou %}
+        {% if have_roles_on_multiple_ou %}
 	<li>{% if ou %}{{ ou }}{% else %}{% trans "All organizational units" %}{% endif %}
               <ul>
         {% endif %}
               {% for role in ou_roles %}
                 <li {% if role.description %}title="{{ role.description }}"{% endif %}>
                 <a href="{% url "a2-manager-role-members" pk=role.pk %}">{{ role }}</a></li>
               {% endfor %}
-        {% if multiple_ou %}
+        {% if have_roles_on_multiple_ou %}
               </ul>
           </li>
         {% endif %}
       {% endfor %}
     </ul>
 
     {% if can_change_roles %}
       <button onclick="window.location.href = '{% url "a2-manager-user-roles" pk=object.pk %}?search-ou={% firstof object.ou.pk default_ou.pk %}'; return false">{% trans "Modify roles" %}</button>
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 56adad77..3e1b27bc 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -336,16 +336,17 @@ class UserDetailView(OtherActionsMixin, BaseDetailView):
     def get_context_data(self, **kwargs):
         kwargs['default_ou'] = get_default_ou
         roles = self.object.roles_and_parents().order_by('ou__name', 'name')
         roles_by_ou = collections.OrderedDict()
         for role in roles:
             roles_by_ou.setdefault(role.ou.name if role.ou else '', []).append(role)
         kwargs['roles'] = roles
         kwargs['roles_by_ou'] = roles_by_ou
+        kwargs['have_roles_on_multiple_ou'] = len(roles_by_ou.keys()) > 1
         # show modify roles button only if something is possible
         kwargs['can_change_roles'] = self.has_perm_on_roles(self.request.user, self.object)
         user_data = []
         user_data += [data for datas in hooks.call_hooks('manager_user_data', self, self.object)
                       for data in datas]
         kwargs['user_data'] = user_data
         ctx = super(UserDetailView, self).get_context_data(**kwargs)
         return ctx
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 24b8e523..bdf6c0fa 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -127,21 +127,32 @@ def test_manager_user_password_reset(app, superuser, simple_user):
     resp = app.get('/logout/').maybe_follow()
     resp = app.get(relative_url, status=200)
     resp.form.set('new_password1', '1234==aA')
     resp.form.set('new_password2', '1234==aA')
     resp = resp.form.submit().follow()
     assert str(app.session['_auth_user_id']) == str(simple_user.pk)
 
 
-def test_manager_user_detail_by_uuid(app, superuser, simple_user):
+def test_manager_user_detail_by_uuid(app, superuser, simple_user, simple_role):
+    simple_user.roles.add(simple_role)
     url = reverse('a2-manager-user-by-uuid-detail', kwargs={'slug': simple_user.uuid})
     resp = login(app, superuser, url)
     assert '<h3>Actions</h3>' in resp.text
     assert simple_user.first_name.encode('utf-8') in resp.content
+    assert 'simple role' in resp.html.find('div', {'class': 'user-roles'}).ul.li.text
+
+    # if user has roles on multiple, roles are grouped by OU
+    simple_user.roles.add(Role.objects.create(name='global role', slug='global-role', ou=None))
+    resp = app.get(url)
+    html_roles = resp.html.find('div', {'class': 'user-roles'})
+    assert 'Default organizational unit' in html_roles.ul.find_all('li', recursive=False)[0].next
+    assert 'simple role' in html_roles.ul.find_all('li', recursive=False)[0].ul.li.text
+    assert 'All organizational units' in html_roles.ul.find_all('li', recursive=False)[1].next
+    assert 'global role' in html_roles.ul.find_all('li', recursive=False)[1].ul.li.text
 
 
 def test_manager_user_edit_by_uuid(app, superuser, simple_user):
     url = reverse('a2-manager-user-by-uuid-edit', kwargs={'slug': simple_user.uuid})
     resp = login(app, superuser, url)
     assert '<h3>Actions</h3>' not in resp.text
     assert simple_user.first_name.encode('utf-8') in resp.content
 
-- 
2.27.0

