From 79ddf657bfd984b1ee79eb785f527e391ced30f7 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Fri, 26 Jan 2018 17:14:00 +0000
Subject: [PATCH] Combine invited and active team members

Right now these are two separate lists. Which makes it harder to add
improvements that will make large numbers of users easier to manage.
---
 app/main/views/manage_users.py            | 11 ++--
 app/templates/views/manage-users.html     | 75 +++++------------------
 tests/app/main/views/test_manage_users.py |  4 +-
 3 files changed, 24 insertions(+), 66 deletions(-)

diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py
index 707317541..ed6b26fed 100644
--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -28,15 +28,18 @@ from app.utils import user_has_permissions
 @login_required
 @user_has_permissions('view_activity', admin_override=True)
 def manage_users(service_id):
-    users = user_api_client.get_users_for_service(service_id=service_id)
-    invited_users = [invite for invite in invite_api_client.get_invites_for_service(service_id=service_id)
-                     if invite.status != 'accepted']
+    users = (
+        user_api_client.get_users_for_service(service_id=service_id) +
+        [
+            invite for invite in invite_api_client.get_invites_for_service(service_id=service_id)
+            if invite.status != 'accepted'
+        ]
+    )
 
     return render_template(
         'views/manage-users.html',
         users=users,
         current_user=current_user,
-        invited_users=invited_users
     )
 
 
diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html
index 5ccba1465..8cfda0874 100644
--- a/app/templates/views/manage-users.html
+++ b/app/templates/views/manage-users.html
@@ -30,15 +30,19 @@
     {% endif %}
   </div>
 
-  <h2 class="visually-hidden">
-    Active
-  </h2>
   <div class="user-list">
     {% for user in users %}
       <div class="user-list-item">
         <h3>
-          <span class="heading-small">{{ user.name }}</span>&ensp;<span class="hint">
-            {%- if user.email_address == current_user.email_address -%}
+          {%- if user.name -%}
+            <span class="heading-small">{{ user.name }}</span>&ensp;
+          {%- endif -%}
+          <span class="hint">
+            {%- if user.status == 'pending' -%}
+              {{ user.email_address }} (pending)
+            {%- elif user.status == 'cancelled' -%}
+              {{ user.email_address }} (cancelled invite)
+            {%- elif user.email_address == current_user.email_address -%}
               (you)
             {% else %}
               {{ user.email_address }}
@@ -74,66 +78,17 @@
             {% endif %}
           </div>
           {% if current_user.has_permissions('manage_users', admin_override=True) %}
-            {% if current_user.id != user.id %}
-              <li class="tick-cross-list-edit-link">
+            <li class="tick-cross-list-edit-link">
+              {% if user.status == 'pending' %}
+                <a href="{{ url_for('.cancel_invited_user', service_id=current_service.id, invited_user_id=user.id)}}">Cancel invitation</a>
+              {% elif user.status == 'active' and current_user.id != user.id %}
                 <a href="{{ url_for('.edit_user_permissions', service_id=current_service.id, user_id=user.id)}}">Edit permissions</a>
-              </li>
-            {% endif %}
+              {% endif %}
+            </li>
           {% endif %}
         </ul>
       </div>
     {% endfor %}
   </div>
 
-  {% if invited_users %}
-    <h2 class="heading-medium">
-      Invited
-    </h2>
-    <div class="user-list">
-      {% for user in invited_users %}
-        <div class="user-list-item">
-          <h3>
-            <span style="font-weight: bold">{{ user.email_address }}</span>
-          </h3>
-          <ul class="tick-cross-list">
-            <div class="tick-cross-list-permissions">
-              {{ tick_cross(
-                user.has_permissions('send_texts', 'send_emails', 'send_letters'),
-                'Send messages'
-              ) }}
-              {{ tick_cross(
-                user.has_permissions('manage_templates'),
-                'Add and edit templates'
-              ) }}
-              {{ tick_cross(
-                user.has_permissions('manage_users', 'manage_settings'),
-                'Manage service'
-              ) }}
-              {{ tick_cross(
-                user.has_permissions('manage_api_keys'),
-                'Access API keys'
-              ) }}
-            {% if 'email_auth' in current_service['permissions'] %}
-              <div class="tick-cross-list-hint">
-                {% if user.auth_type == 'sms_auth' %}
-                  Signs in with a text message code
-                {% else %}
-                  Signs in with an email link
-                {% endif %}
-              </div>
-            {% endif %}
-            </div>
-            <li class="tick-cross-list-edit-link">
-              {% if user.status == 'pending' and current_user.has_permissions('manage_users') %}
-                <a href="{{ url_for('.cancel_invited_user', service_id=current_service.id, invited_user_id=user.id)}}">Cancel invitation</a>
-              {% else %}
-                {{ user.status|title }}
-              {% endif %}
-            </li>
-          </ul>
-        </div>
-      {% endfor %}
-    </div>
-  {% endif %}
-
 {% endblock %}
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index 3fb303e31..94ee839c3 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -461,8 +461,8 @@ def test_manage_users_shows_invited_user(
     page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
 
     assert page.h1.string.strip() == 'Team members'
-    assert normalize_spaces(page.select('.user-list')[1].text) == (
-        'invited_user@test.gov.uk '
+    assert normalize_spaces(page.select('.user-list-item')[1].text) == (
+        'invited_user@test.gov.uk (pending) '
         'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can Access API keys '
         'Cancel invitation'
     )