diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py deleted file mode 100644 index 804fd8a1d..000000000 --- a/app/dao/organisation_dao.py +++ /dev/null @@ -1,9 +0,0 @@ -from app.models import Organisation - - -def dao_get_organisations(): - return Organisation.query.all() - - -def dao_get_organisation_by_id(org_id): - return Organisation.query.filter_by(id=org_id).one() diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py new file mode 100644 index 000000000..71b8bfa85 --- /dev/null +++ b/app/dao/organisations_dao.py @@ -0,0 +1,23 @@ +from app import db +from app.dao.dao_utils import transactional +from app.models import Organisation + + +def dao_get_organisations(): + return Organisation.query.all() + + +def dao_get_organisation_by_id(org_id): + return Organisation.query.filter_by(id=org_id).one() + + +@transactional +def dao_create_organisation(organisation): + db.session.add(organisation) + + +@transactional +def dao_update_organisation(organisation, **kwargs): + for key, value in kwargs.items(): + setattr(organisation, key, value) + db.session.add(organisation) diff --git a/app/models.py b/app/models.py index 8fda806b8..0e3079c80 100644 --- a/app/models.py +++ b/app/models.py @@ -134,9 +134,19 @@ class Organisation(db.Model): __tablename__ = 'organisation' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) colour = db.Column(db.String(7), nullable=True) - logo = db.Column(db.String(255), nullable=True) + logo = db.Column(db.String(255), nullable=False) name = db.Column(db.String(255), nullable=True) + def serialize(self): + serialized = { + "id": str(self.id), + "colour": self.colour, + "logo": self.logo, + "name": self.name, + } + + return serialized + DVLA_ORG_HM_GOVERNMENT = '001' DVLA_ORG_LAND_REGISTRY = '500' diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py new file mode 100644 index 000000000..60b523057 --- /dev/null +++ b/app/organisation/organisation_schema.py @@ -0,0 +1,23 @@ +post_create_organisation_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST schema for getting organisation", + "type": "object", + "properties": { + "colour": {"type": ["string", "null"]}, + "name": {"type": ["string", "null"]}, + "logo": {"type": ["string", "null"]} + }, + "required": ["logo"] +} + +post_update_organisation_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST schema for getting organisation", + "type": "object", + "properties": { + "colour": {"type": ["string", "null"]}, + "name": {"type": ["string", "null"]}, + "logo": {"type": ["string", "null"]} + }, + "required": [] +} diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 1bdfbff74..250c0d205 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,8 +1,15 @@ -from flask import Blueprint, jsonify +from flask import Blueprint, jsonify, request -from app.dao.organisation_dao import dao_get_organisations, dao_get_organisation_by_id -from app.schemas import organisation_schema +from app.dao.organisations_dao import ( + dao_create_organisation, + dao_get_organisations, + dao_get_organisation_by_id, + dao_update_organisation +) from app.errors import register_errors +from app.models import Organisation +from app.organisation.organisation_schema import post_create_organisation_schema, post_update_organisation_schema +from app.schema_validation import validate organisation_blueprint = Blueprint('organisation', __name__) register_errors(organisation_blueprint) @@ -10,11 +17,35 @@ register_errors(organisation_blueprint) @organisation_blueprint.route('', methods=['GET']) def get_organisations(): - data = organisation_schema.dump(dao_get_organisations(), many=True).data - return jsonify(organisations=data) + organisations = [o.serialize() for o in dao_get_organisations()] + return jsonify(organisations=organisations) @organisation_blueprint.route('/', methods=['GET']) def get_organisation_by_id(org_id): - data = organisation_schema.dump(dao_get_organisation_by_id(org_id)).data - return jsonify(organisation=data) + organisation = dao_get_organisation_by_id(org_id) + return jsonify(organisation=organisation.serialize()) + + +@organisation_blueprint.route('', methods=['POST']) +def create_organisation(): + data = request.get_json() + + validate(data, post_create_organisation_schema) + + organisation = Organisation(**data) + + dao_create_organisation(organisation) + return jsonify(data=organisation.serialize()), 201 + + +@organisation_blueprint.route('/', methods=['POST']) +def update_organisation(organisation_id): + data = request.get_json() + + validate(data, post_update_organisation_schema) + + fetched_organisation = dao_get_organisation_by_id(organisation_id) + dao_update_organisation(fetched_organisation, **data) + + return jsonify(data=fetched_organisation.serialize()), 200 diff --git a/app/schemas.py b/app/schemas.py index bfdda4f28..50eeffb93 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -600,12 +600,6 @@ class EventSchema(BaseSchema): strict = True -class OrganisationSchema(BaseSchema): - class Meta: - model = models.Organisation - strict = True - - class DaySchema(ma.Schema): class Meta: @@ -658,7 +652,6 @@ service_history_schema = ServiceHistorySchema() api_key_history_schema = ApiKeyHistorySchema() template_history_schema = TemplateHistorySchema() event_schema = EventSchema() -organisation_schema = OrganisationSchema() provider_details_schema = ProviderDetailsSchema() provider_details_history_schema = ProviderDetailsHistorySchema() day_schema = DaySchema() diff --git a/migrations/versions/0108_change_logo_not_nullable.py b/migrations/versions/0108_change_logo_not_nullable.py new file mode 100644 index 000000000..7167d097b --- /dev/null +++ b/migrations/versions/0108_change_logo_not_nullable.py @@ -0,0 +1,27 @@ +"""empty message + +Revision ID: 0108_change_logo_not_nullable +Revises: 0107_drop_template_stats +Create Date: 2017-07-06 10:14:35.188404 + +""" + +# revision identifiers, used by Alembic. +revision = '0108_change_logo_not_nullable' +down_revision = '0107_drop_template_stats' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +def upgrade(): + op.alter_column('organisation', 'logo', + existing_type=sa.VARCHAR(length=255), + nullable=False) + + +def downgrade(): + op.alter_column('organisation', 'logo', + existing_type=sa.VARCHAR(length=255), + nullable=True) diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py new file mode 100644 index 000000000..bc5d8c0a6 --- /dev/null +++ b/tests/app/dao/test_organisations_dao.py @@ -0,0 +1,71 @@ +import pytest +from sqlalchemy.exc import IntegrityError + +from app.dao.organisations_dao import ( + dao_create_organisation, + dao_get_organisations, + dao_get_organisation_by_id, + dao_update_organisation, +) +from app.models import Organisation + +from tests.app.db import create_organisation + + +def test_create_organisation(notify_db, notify_db_session): + organisation = create_organisation() + + assert Organisation.query.count() == 1 + organisation_from_db = Organisation.query.first() + assert organisation == organisation_from_db + + +def test_create_organisation_without_name_or_colour_is_valid(notify_db, notify_db_session): + organisation = create_organisation(name=None, colour=None) + + assert Organisation.query.count() == 1 + organisation_from_db = Organisation.query.first() + assert organisation == organisation_from_db + + +def test_create_organisation_without_logo_raises_error(notify_db, notify_db_session): + with pytest.raises(IntegrityError) as excinfo: + create_organisation(logo=None) + assert 'column "logo" violates not-null constraint' in str(excinfo.value) + assert Organisation.query.count() == 0 + + +def test_get_organisations_gets_all_organisations(notify_db, notify_db_session): + org_1 = create_organisation(name='test_org_1') + org_2 = create_organisation(name='test_org_2') + + organisations = dao_get_organisations() + + assert len(organisations) == 2 + assert org_1 == organisations[0] + assert org_2 == organisations[1] + + +def test_get_organisation_by_id_gets_correct_organisation(notify_db, notify_db_session): + organisation = create_organisation() + + organisation_from_db = dao_get_organisation_by_id(organisation.id) + + assert organisation_from_db == organisation + + +def test_update_organisation(notify_db, notify_db_session): + updated_name = 'new name' + organisation = create_organisation() + + organisations_1 = Organisation.query.all() + + assert len(organisations_1) == 1 + assert organisations_1[0].name != updated_name + + dao_update_organisation(organisations_1[0], name=updated_name) + + organisations_2 = Organisation.query.all() + + assert len(organisations_2) == 1 + assert organisations_2[0].name == updated_name diff --git a/tests/app/db.py b/tests/app/db.py index c212a6612..ea9e4cbdd 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -13,6 +13,7 @@ from app.models import ( ServicePermission, Job, InboundSms, + Organisation, EMAIL_TYPE, SMS_TYPE, KEY_TYPE_NORMAL, @@ -23,6 +24,7 @@ from app.dao.templates_dao import dao_create_template from app.dao.services_dao import dao_create_service from app.dao.service_permissions_dao import dao_add_service_permission from app.dao.inbound_sms_dao import dao_create_inbound_sms +from app.dao.organisations_dao import dao_create_organisation def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk", state='active'): @@ -225,3 +227,15 @@ def create_service_inbound_api( ) save_service_inbound_api(service_inbound_api) return service_inbound_api + + +def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'): + data = { + 'colour': colour, + 'logo': logo, + 'name': name + } + organisation = Organisation(**data) + dao_create_organisation(organisation) + + return organisation diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index f024edd31..ea5926f5d 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -1,39 +1,113 @@ from flask import json +import pytest from app.models import Organisation from tests import create_authorization_header -def test_get_organisations(notify_api, notify_db, notify_db_session): +def test_get_organisations(admin_request, notify_db, notify_db_session): org1 = Organisation(colour='#FFFFFF', logo='/path/image.png', name='Org1') org2 = Organisation(colour='#000000', logo='/path/other.png', name='Org2') notify_db.session.add_all([org1, org2]) notify_db.session.commit() - with notify_api.test_request_context(), notify_api.test_client() as client: - auth_header = create_authorization_header() - response = client.get('/organisation', headers=[auth_header]) + organisations = admin_request.get( + 'organisation.get_organisations' + )['organisations'] - assert response.status_code == 200 - organisations = json.loads(response.get_data(as_text=True))['organisations'] assert len(organisations) == 2 assert {org['id'] for org in organisations} == {str(org1.id), str(org2.id)} -def test_get_organisation_by_id(notify_api, notify_db, notify_db_session): +def test_get_organisation_by_id(admin_request, notify_db, notify_db_session): org = Organisation(colour='#FFFFFF', logo='/path/image.png', name='My Org') notify_db.session.add(org) notify_db.session.commit() - with notify_api.test_request_context(), notify_api.test_client() as client: - auth_header = create_authorization_header() - response = client.get('/organisation/{}'.format(org.id), headers=[auth_header]) + response = admin_request.get( + 'organisation.get_organisation_by_id', + _expected_status=200, + org_id=org.id + ) - assert response.status_code == 200 - organisation = json.loads(response.get_data(as_text=True))['organisation'] - assert set(organisation.keys()) == {'colour', 'logo', 'name', 'id'} - assert organisation['colour'] == '#FFFFFF' - assert organisation['logo'] == '/path/image.png' - assert organisation['name'] == 'My Org' - assert organisation['id'] == str(org.id) + assert set(response['organisation'].keys()) == {'colour', 'logo', 'name', 'id'} + assert response['organisation']['colour'] == '#FFFFFF' + assert response['organisation']['logo'] == '/path/image.png' + assert response['organisation']['name'] == 'My Org' + assert response['organisation']['id'] == str(org.id) + + +def test_post_create_organisation(admin_request, notify_db_session): + data = { + 'name': 'test organisation', + 'colour': '#0000ff', + 'logo': '/images/test_x2.png' + } + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=201 + ) + assert data['name'] == response['data']['name'] + assert data['colour'] == response['data']['colour'] + assert data['logo'] == response['data']['logo'] + + +def test_post_create_organisation_without_logo_raises_error(admin_request, notify_db_session): + data = { + 'name': 'test organisation', + 'colour': '#0000ff', + } + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=400 + ) + assert response['errors'][0]['message'] == "logo is a required property" + + +def test_post_create_organisation_without_name_or_colour_is_valid(admin_request, notify_db_session): + data = { + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=201 + ) + + assert response['data']['logo'] == data['logo'] + assert response['data']['name'] is None + assert response['data']['colour'] is None + + +@pytest.mark.parametrize('data_update', [ + ({'name': 'test organisation 1'}), + ({'logo': 'images/text_x3.png', 'colour': '#ffffff'}), +]) +def test_post_update_organisation_updates_field(admin_request, notify_db_session, data_update): + data = { + 'name': 'test organisation', + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=201 + ) + + org_id = response['data']['id'] + + response = admin_request.post( + 'organisation.update_organisation', + _data=data_update, + organisation_id=org_id + ) + + organisations = Organisation.query.all() + + assert len(organisations) == 1 + assert str(organisations[0].id) == org_id + for key in data_update.keys(): + assert getattr(organisations[0], key) == data_update[key]