Added more tests

app/dao/organisation_dao.py

@@ -1,6 +1,10 @@
 from app import db
 from app.dao.dao_utils import transactional
-from app.models import Organisation, InvitedOrganisationUser, User
+from app.models import (
+    Organisation,
+    InvitedOrganisationUser,
+    User
+)
 
 
 def dao_get_organisations():
@@ -55,10 +59,10 @@ def dao_get_users_for_organisation(organisation_id):
     ).order_by(User.created_at).all()
 
 
+@transactional
 def dao_add_user_to_organisation(organisation_id, user_id):
     organisation = dao_get_organisation_by_id(organisation_id)
-    user = User.query.get(user_id)
+    user = User.query.filter_by(id=user_id).filter_by(state='active').one()
     organisation.users.append(user)
     db.session.add(organisation)
-    db.session.commit()
     return user

app/organisation/accept_organisation_invite.py

@@ -22,11 +22,9 @@ def accept_organisation_invitation(token):
                                       current_app.config['DANGEROUS_SALT'],
                                       max_age_seconds)
     except SignatureExpired:
-        errors = {'invitation':
-                  ['Your invitation to GOV.UK Notify has expired. '
-                   'Please ask the person that invited you to send you another one']}
+        errors = {'invitation': ['Your invitation to GOV.UK Notify has expired. '
+                                 'Please ask the person that invited you to send you another one']}
         raise InvalidRequest(errors, status_code=400)
     invited_user = dao_get_invited_organisation_user(invited_user_id)
 
     return jsonify(data=invited_user.serialize()), 200
-

app/organisation/rest.py

@@ -97,7 +97,6 @@ def get_organisation_services(organisation_id):
 
 @organisation_blueprint.route('/<uuid:organisation_id>/users/<uuid:user_id>', methods=['POST'])
 def add_user_to_organisation(organisation_id, user_id):
-    current_app.logger.info("ADDING new user")
     new_org_user = dao_add_user_to_organisation(organisation_id, user_id)
     return jsonify(data=user_schema.dump(new_org_user).data), 200
 

tests/app/dao/test_organisation_dao.py

@@ -16,12 +16,12 @@ from app.dao.organisation_dao import (
 )
 from app.models import Organisation
 
-from tests.app.db import create_organisation, create_service, create_invited_org_user, create_user
+from tests.app.db import create_organisation, create_service, create_user
 
 
 def test_get_organisations_gets_all_organisations_alphabetically_with_active_organisations_first(
-    notify_db,
-    notify_db_session
+        notify_db,
+        notify_db_session
 ):
     m_active_org = create_organisation(name='m_active_organisation')
     z_inactive_org = create_organisation(name='z_inactive_organisation', active=False)
@@ -134,3 +134,46 @@ def test_dao_get_users_for_organisation(sample_organisation):
     assert results[0] == first
     assert results[1] == second
 
+
+def test_dao_get_users_for_organisation_returns_empty_list(sample_organisation):
+    results = dao_get_users_for_organisation(organisation_id=sample_organisation.id)
+    assert len(results) == 0
+
+
+def test_dao_get_users_for_organisation_only_returns_active_users(sample_organisation):
+    first = create_user(email='first@invited.com')
+    second = create_user(email='another@invited.com')
+
+    dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=first.id)
+    dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=second.id)
+
+    second.state = 'inactive'
+
+    results = dao_get_users_for_organisation(organisation_id=sample_organisation.id)
+    assert len(results) == 1
+    assert results[0] == first
+
+
+def test_add_user_to_organisation_returns_user(sample_organisation):
+    org_user = create_user()
+    assert not org_user.organisations
+
+    added_user = dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=org_user.id)
+    assert len(added_user.organisations) == 1
+    assert added_user.organisations[0] == sample_organisation
+
+
+def test_add_user_to_organisation_when_user_does_not_exist(sample_organisation):
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=uuid.uuid4())
+
+
+def test_add_user_to_organisation_when_organisation_does_not_exist(sample_user):
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        dao_add_user_to_organisation(organisation_id=uuid.uuid4(), user_id=sample_user.id)
+
+
+def test_add_user_to_organisation_raises_exception_when_user_is_not_active(sample_organisation):
+    first = create_user(state='inactive')
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=first.id)

tests/app/organisation/test_accept_organisation_invite.py

@@ -15,4 +15,3 @@ def test_accept_organisation_invitation(client, sample_invited_org_user):
     assert response.status_code == 200
     json_resp = json.loads(response.get_data(as_text=True))
     assert json_resp['data'] == sample_invited_org_user.serialize()
-

tests/app/organisation/test_rest.py

@@ -1,6 +1,8 @@
+import uuid
+
 from app.models import Organisation
-from app.dao.organisation_dao import dao_add_service_to_organisation
-from tests.app.db import create_organisation, create_service
+from app.dao.organisation_dao import dao_add_service_to_organisation, dao_add_user_to_organisation
+from tests.app.db import create_organisation, create_service, create_user
 
 
 def test_get_all_organisations(admin_request, notify_db_session):
@@ -270,3 +272,41 @@ def test_rest_get_organisation_services_inactive_services_at_end(
     assert response[0]['name'] == service.name
     assert response[1]['name'] == inactive_service.name
     assert response[2]['name'] == inactive_service_1.name
+
+
+def test_add_user_to_organisation_returns_added_user(admin_request, sample_organisation, sample_user):
+    response = admin_request.post(
+        'organisation.add_user_to_organisation',
+        organisation_id=str(sample_organisation.id),
+        user_id=str(sample_user.id),
+        _expected_status=200
+    )
+
+    assert response['data']['id'] == str(sample_user.id)
+    assert len(response['data']['organisations']) == 1
+    assert response['data']['organisations'][0] == str(sample_organisation.id)
+
+
+def test_add_user_to_organisation_returns_404_if_user_does_not_exist(admin_request, sample_organisation):
+    admin_request.post(
+        'organisation.add_user_to_organisation',
+        organisation_id=str(sample_organisation.id),
+        user_id=str(uuid.uuid4()),
+        _expected_status=404
+    )
+
+
+def test_get_organisation_users_returns_users_for_organisation(admin_request, sample_organisation):
+    first = create_user(email='first@invited.com')
+    second = create_user(email='another@invited.com')
+    dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=first.id)
+    dao_add_user_to_organisation(organisation_id=sample_organisation.id, user_id=second.id)
+
+    response = admin_request.get(
+        'organisation.get_organisation_users',
+        organisation_id=sample_organisation.id,
+        _expected_status=200
+    )
+
+    assert len(response['data']) == 2
+    assert response['data'][0]['id'] == str(first.id)