Merge pull request #1073 from alphagov/ken-update-organisation-dao-model-rest

Update organisation dao model rest

app/dao/organisation_dao.py

@@ -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()

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)

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'

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": []
+}

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('/<uuid:org_id>', 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('/<uuid:organisation_id>', 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

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()

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)

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

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

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]