From 70f9dfc0f62d38e4b81da05ecd02d5e6bc22469d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 21 Feb 2018 16:39:17 +0000 Subject: [PATCH] Added more tests --- app/dao/organisation_dao.py | 10 ++-- .../accept_organisation_invite.py | 6 +-- app/organisation/rest.py | 1 - tests/app/dao/test_organisation_dao.py | 49 +++++++++++++++++-- .../test_accept_organisation_invite.py | 1 - tests/app/organisation/test_rest.py | 44 ++++++++++++++++- 6 files changed, 97 insertions(+), 14 deletions(-) diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index b31c1a583..a4edab289 100644 --- a/app/dao/organisation_dao.py +++ b/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 diff --git a/app/organisation/accept_organisation_invite.py b/app/organisation/accept_organisation_invite.py index c18e61df4..20c580a09 100644 --- a/app/organisation/accept_organisation_invite.py +++ b/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 - diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 124a82619..3bf3972b4 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -97,7 +97,6 @@ def get_organisation_services(organisation_id): @organisation_blueprint.route('//users/', 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 diff --git a/tests/app/dao/test_organisation_dao.py b/tests/app/dao/test_organisation_dao.py index a4b90fb80..045de8b5c 100644 --- a/tests/app/dao/test_organisation_dao.py +++ b/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) diff --git a/tests/app/organisation/test_accept_organisation_invite.py b/tests/app/organisation/test_accept_organisation_invite.py index 80f166129..be2db4ca6 100644 --- a/tests/app/organisation/test_accept_organisation_invite.py +++ b/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() - diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 676d72a22..55db57330 100644 --- a/tests/app/organisation/test_rest.py +++ b/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)