Merge pull request #337 from alphagov/remove_user_from_service

Functionality added and all tests working.

app/__init__.py

@@ -200,6 +200,12 @@ def register_errorhandlers(application):
     def handle_no_permissions(error):
         return _error_response(401)
 
+    @application.errorhandler(500)
+    def handle_exception(error):
+        if current_app.config.get('DEBUG', None):
+            raise error
+        return _error_response(500)
+
     @application.errorhandler(Exception)
     def handle_bad_request(error):
         # We want the Flask in browser stacktrace

app/main/views/manage_users.py

@@ -5,7 +5,8 @@ from flask import (
     render_template,
     redirect,
     url_for,
-    flash
+    flash,
+    abort
 )
 
 from flask_login import (
@@ -13,6 +14,7 @@ from flask_login import (
     current_user
 )
 
+from notifications_python_client import HTTPError
 from app.main import main
 from app.main.forms import (
     InviteUserForm,
@@ -20,6 +22,7 @@ from app.main.forms import (
 )
 from app.main.dao.services_dao import get_service_by_id
 from app import user_api_client
+from app import service_api_client
 from app import invite_api_client
 from app.utils import user_has_permissions
 
@@ -106,6 +109,45 @@ def edit_user_permissions(service_id, user_id):
     )
 
 
+@main.route("/services/<service_id>/users/<user_id>/delete", methods=['GET', 'POST'])
+@login_required
+@user_has_permissions('manage_users', admin_override=True)
+def remove_user_from_service(service_id, user_id):
+    user = user_api_client.get_user(user_id)
+    service = get_service_by_id(service_id)
+    # Need to make the email address read only, or a disabled field?
+    # Do it through the template or the form class?
+    form = PermissionsForm(**{
+        role: user.has_permissions(permissions=permissions) for role, permissions in roles.items()
+        })
+
+    if request.method == 'POST':
+        try:
+            service_api_client.remove_user_from_service(service_id, user_id)
+        except HTTPError as e:
+            msg = "You cannot remove the only user for a service"
+            if e.status_code == 400 and msg in e.message:
+                flash(msg, 'info')
+                return redirect(url_for(
+                    '.manage_users',
+                    service_id=service_id))
+            else:
+                abort(500, e)
+
+        return redirect(url_for(
+            '.manage_users',
+            service_id=service_id
+        ))
+
+    flash('Are you sure you want to remove {}?'.format(user.name), 'remove')
+    return render_template(
+        'views/edit-user-permissions.html',
+        user=user,
+        form=form,
+        service_id=service_id
+    )
+
+
 @main.route("/services/<service_id>/cancel-invited-user/<invited_user_id>", methods=['GET'])
 @user_has_permissions('manage_users', admin_override=True)
 def cancel_invited_user(service_id, invited_user_id):

app/notify_client/api_client.py

@@ -70,6 +70,15 @@ class ServiceAPIClient(NotificationsAPIClient):
         endpoint = "/service/{0}".format(service_id)
         return self.post(endpoint, data)
 
+    def remove_user_from_service(self, service_id, user_id):
+        """
+        Remove a user from a service
+        """
+        endpoint = '/service/{service_id}/users/{user_id}'.format(
+            service_id=service_id,
+            user_id=user_id)
+        return self.delete(endpoint)
+
     def create_service_template(self, name, type_, content, service_id, subject=None):
         """
         Create a service template.

app/templates/flash_messages.html

@@ -5,7 +5,7 @@
       {{ banner(
         message,
         'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous',
-        delete_button="Yes, delete" if 'delete' == category else None,
+        delete_button="Yes, {}".format(category) if category in ['delete', 'remove'] else None,
         with_tick=True if category == 'default_with_tick' else False
       )}}
     {% endfor %}

app/templates/views/edit-user-permissions.html

@@ -21,11 +21,11 @@ Manage users – GOV.UK Notify
 
       {% include 'views/manage-users/permissions.html' %}
 
-      {{ page_footer(
+        {{ page_footer(
-        'Save',
+          'Save',
-        back_link=url_for('.manage_users', service_id=service_id),
+          delete_link=url_for('.remove_user_from_service', service_id=service_id, user_id=user.id) if user or None,
-        back_link_text='Cancel'
+          delete_link_text='Remove user from service'
-      ) }}
+        ) }}
 
     </form>
   </div>

app/templates/views/manage-users.html

@@ -45,7 +45,7 @@ Manage users – GOV.UK Notify
     {% call field(align='right') %}
       {% if current_user.has_permissions(['manage_users']) %}
         {% if current_user.id != item.id %}
-          <a href="{{ url_for('.edit_user_permissions', service_id=service_id, user_id=item.id)}}">Edit permission</a>
+          <a href="{{ url_for('.edit_user_permissions', service_id=service_id, user_id=item.id)}}">Edit</a>
         {% endif %}
       {% endif %}
     {% endcall %}

tests/app/main/views/test_manage_users.py

@@ -302,3 +302,48 @@ def test_no_permission_manage_users_page(app_,
             assert url_for('.invite_user', service_id=service_one['id']) not in resp_text
             assert "Edit permission" not in resp_text
             assert "Manage team" not in resp_text
+
+
+def test_get_remove_user_from_service(app_,
+                                      api_user_active,
+                                      mock_login,
+                                      mock_get_user_by_email,
+                                      mock_get_service,
+                                      mock_get_users_by_service,
+                                      mock_get_user,
+                                      mock_has_permissions):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            service = mock_get_service("12345")['data']
+            client.login(api_user_active)
+            response = client.get(
+                url_for(
+                    'main.remove_user_from_service',
+                    service_id=service['id'],
+                    user_id=api_user_active.id))
+            assert response.status_code == 200
+            assert "Are you sure you want to remove" in response.get_data(as_text=True)
+            assert "Remove user from service" in response.get_data(as_text=True)
+
+
+def test_remove_user_from_service(app_,
+                                  api_user_active,
+                                  mock_login,
+                                  mock_get_user_by_email,
+                                  mock_get_service,
+                                  mock_get_users_by_service,
+                                  mock_get_user,
+                                  mock_has_permissions,
+                                  mock_remove_user_from_service):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            service = mock_get_service("12345")['data']
+            client.login(api_user_active)
+            response = client.post(
+                url_for(
+                    'main.remove_user_from_service',
+                    service_id=service['id'],
+                    user_id=api_user_active.id))
+            assert response.status_code == 302
+            assert response.location == url_for(
+                'main.manage_users', service_id=service['id'], _external=True)

tests/conftest.py

@@ -757,3 +757,8 @@ def mock_add_user_to_service(mocker, service_one, api_user_active):
 @pytest.fixture(scope='function')
 def mock_set_user_permissions(mocker):
     return mocker.patch('app.user_api_client.set_user_permissions', return_value=None)
+
+
+@pytest.fixture(scope='function')
+def mock_remove_user_from_service(mocker):
+    return mocker.patch('app.service_api_client.remove_user_from_service', return_value=None)