Merge pull request #180 from alphagov/remove_user_from_service

Added endpoint for removing user from service, all tests passing.

app/dao/permissions_dao.py

@@ -57,6 +57,10 @@ class PermissionDAO(DAOClass):
             permission = Permission(permission=name, user=user, service=service)
             self.create_instance(permission, _commit=False)
 
+    def remove_user_service_permissions(self, user, service):
+        query = self.get_query(filter_by_dict={'user': user.id, 'service': service.id})
+        query.delete()
+
     def set_user_permission(self, user, permissions):
         try:
             query = self.get_query(filter_by_dict={'user': user.id})

app/dao/services_dao.py

@@ -45,6 +45,14 @@ def dao_add_user_to_service(service, user):
 
 
 def dao_remove_user_from_service(service, user):
-    service.users.remove(user)
+    try:
-    db.session.add(service)
+        from app.dao.permissions_dao import permission_dao
-    db.session.commit()
+        permission_dao.remove_user_service_permissions(user, service)
+        service.users.remove(user)
+        db.session.add(service)
+    except Exception as e:
+        # Proper clean up
+        db.session.rollback()
+        raise e
+    else:
+        db.session.commit()

app/errors.py

@@ -40,6 +40,8 @@ def register_errors(blueprint):
 
     @blueprint.app_errorhandler(500)
     def internal_server_error(e):
+        if current_app.config.get('DEBUG'):
+            raise e
         if isinstance(e, str):
             current_app.logger.exception(e)
         elif isinstance(e, Exception):
@@ -58,5 +60,7 @@ def register_errors(blueprint):
 
     @blueprint.app_errorhandler(SQLAlchemyError)
     def db_error(e):
+        if current_app.config.get('DEBUG'):
+            raise e
         current_app.logger.exception(e)
         return jsonify(result='error', message=str(e)), 500

app/service/rest.py

@@ -20,7 +20,8 @@ from app.dao.services_dao import (
     dao_create_service,
     dao_update_service,
     dao_fetch_all_services_by_user,
-    dao_add_user_to_service
+    dao_add_user_to_service,
+    dao_remove_user_from_service
 )
 
 from app.dao.users_dao import get_model_users
@@ -159,6 +160,22 @@ def add_user_to_service(service_id, user_id):
     return jsonify(data=data), 201
 
 
+@service.route('/<uuid:service_id>/users/<user_id>', methods=['DELETE'])
+def remove_user_from_service(service_id, user_id):
+    service = dao_fetch_service_by_id(service_id)
+    user = get_model_users(user_id=user_id)
+    if user not in service.users:
+        return jsonify(
+            result='error',
+            message='User not found'), 404
+    elif len(service.users) == 1:
+        return jsonify(
+            result='error',
+            message='You cannot remove the only user for a service'), 400
+    dao_remove_user_from_service(service, user)
+    return jsonify({}), 204
+
+
 def _process_permissions(user, service, permission_groups):
     from app.permissions_utils import get_permissions_by_group
     from app.dao.permissions_dao import permission_dao

config.py

@@ -84,7 +84,7 @@ class Development(Config):
 
 
 class Test(Development):
-    pass
+    DEBUG = True
 
 configs = {
     'live': 'config_live.Live',

tests/app/conftest.py

@@ -4,7 +4,7 @@ from app import (email_safe, db)
 from app.models import (
     User, Service, Template, ApiKey, Job, Notification, InvitedUser, Permission)
 from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code)
-from app.dao.services_dao import dao_create_service
+from app.dao.services_dao import (dao_create_service, dao_add_user_to_service)
 from app.dao.templates_dao import dao_create_template
 from app.dao.api_key_dao import save_model_api_key
 from app.dao.jobs_dao import dao_create_job
@@ -116,6 +116,9 @@ def sample_service(notify_db,
     if not service:
         service = Service(**data)
         dao_create_service(service, user)
+    else:
+        if user not in service.users:
+            dao_add_user_to_service(service, user)
     return service
 
 
@@ -394,7 +397,7 @@ def sample_service_permission(notify_db,
     if user is None:
         user = sample_user(notify_db, notify_db_session)
     if service is None:
-        service = sample_service(notify_db, notify_db_session)
+        service = sample_service(notify_db, notify_db_session, user=user)
     data = {
         'user': user,
         'service': service,

tests/app/service/test_rest.py

@@ -5,7 +5,9 @@ from app.dao.users_dao import save_model_user
 from app.dao.services_dao import dao_remove_user_from_service
 from app.models import User
 from tests import create_authorization_header
-from tests.app.conftest import sample_service as create_service
+from tests.app.conftest import sample_service as create_sample_service
+from tests.app.conftest import sample_service_permission as create_sample_service_permission
+from tests.app.conftest import sample_user as create_sample_user
 
 
 def test_get_service_list(notify_api, service_factory):
@@ -358,7 +360,7 @@ def test_should_not_update_service_with_duplicate_name(notify_api,
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             service_name = "another name"
-            another_service = create_service(
+            another_service = create_sample_service(
                 notify_db,
                 notify_db_session,
                 service_name=service_name,
@@ -837,3 +839,72 @@ def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db
             assert resp.status_code == 404
             assert result['result'] == 'error'
             assert result['message'] == expected_message
+
+
+def test_remove_user_from_service(notify_api, notify_db, notify_db_session, sample_service_permission):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            second_user = create_sample_user(
+                notify_db,
+                notify_db_session,
+                email="new@digital.cabinet-office.gov.uk")
+            # Simulates successfully adding a user to the service
+            second_permission = create_sample_service_permission(
+                notify_db,
+                notify_db_session,
+                user=second_user)
+            endpoint = url_for(
+                'service.remove_user_from_service',
+                service_id=str(second_permission.service.id),
+                user_id=str(second_permission.user.id))
+            auth_header = create_authorization_header(
+                path=endpoint,
+                method='DELETE'
+            )
+            resp = client.delete(
+                endpoint,
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert resp.status_code == 204
+
+
+def test_remove_user_from_service(notify_api, notify_db, notify_db_session, sample_service_permission):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            second_user = create_sample_user(
+                notify_db,
+                notify_db_session,
+                email="new@digital.cabinet-office.gov.uk")
+            endpoint = url_for(
+                'service.remove_user_from_service',
+                service_id=str(sample_service_permission.service.id),
+                user_id=str(second_user.id))
+            auth_header = create_authorization_header(
+                path=endpoint,
+                method='DELETE'
+            )
+            resp = client.delete(
+                endpoint,
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert resp.status_code == 404
+
+
+def test_cannot_remove_only_user_from_service(notify_api,
+                                              notify_db,
+                                              notify_db_session,
+                                              sample_service_permission):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            endpoint = url_for(
+                'service.remove_user_from_service',
+                service_id=str(sample_service_permission.service.id),
+                user_id=str(sample_service_permission.user.id))
+            auth_header = create_authorization_header(
+                path=endpoint,
+                method='DELETE'
+            )
+            resp = client.delete(
+                endpoint,
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert resp.status_code == 400
+            result = json.loads(resp.get_data(as_text=True))
+            assert result['message'] == 'You cannot remove the only user for a service'