From b814c5ff26675e4a7446be4c4393990245847c5f Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 7 Jul 2017 10:00:38 +0100 Subject: [PATCH] Refactor organisation rest to remove marshmallow --- app/models.py | 18 +++++++-------- app/organisation/organisation_schema.py | 11 +++++++++ app/organisation/rest.py | 30 ++++++++++++------------- app/schemas.py | 7 ------ tests/app/organisation/test_rest.py | 6 ++--- 5 files changed, 37 insertions(+), 35 deletions(-) create mode 100644 app/organisation/organisation_schema.py diff --git a/app/models.py b/app/models.py index 43e8b7ebe..0e3079c80 100644 --- a/app/models.py +++ b/app/models.py @@ -137,17 +137,15 @@ class Organisation(db.Model): logo = db.Column(db.String(255), nullable=False) name = db.Column(db.String(255), nullable=True) - @classmethod - def from_json(cls, data): - """ - Assumption: data has been validated appropriately. + def serialize(self): + serialized = { + "id": str(self.id), + "colour": self.colour, + "logo": self.logo, + "name": self.name, + } - Returns a Organisation object based on the provided data. - """ - # validate json with marshmallow - fields = data.copy() - - return cls(**fields) + return serialized DVLA_ORG_HM_GOVERNMENT = '001' diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py new file mode 100644 index 000000000..f04e55d9f --- /dev/null +++ b/app/organisation/organisation_schema.py @@ -0,0 +1,11 @@ +post_organisation_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST schema for getting organisation", + "type": "object", + "properties": { + "colour": {"type": ["string", "null"], "format": "string"}, + "name": {"type": ["string", "null"], "minimum": 1}, + "logo": {"type": ["string", "null"], "minimum": 1} + }, + "required": ["logo"] +} diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 8d7756f65..a4260377a 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,12 +1,17 @@ from flask import Blueprint, jsonify, request -from app.dao.organisations_dao import dao_get_organisations, dao_get_organisation_by_id, dao_create_organisation -from app.schemas import organisation_schema +from app.dao.organisations_dao import ( + dao_get_organisations, + dao_get_organisation_by_id, + dao_create_organisation +) from app.errors import ( InvalidRequest, register_errors ) from app.models import Organisation +from app.organisation.organisation_schema import post_organisation_schema +from app.schema_validation import validate organisation_blueprint = Blueprint('organisation', __name__) register_errors(organisation_blueprint) @@ -14,28 +19,23 @@ 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 post_organisation(): data = request.get_json() - if not data.get('logo', None): - errors = {'logo': ['Missing data for required field.']} - raise InvalidRequest(errors, status_code=400) - # validate json with marshmallow - organisation_schema.load(request.get_json()) + validate(data, post_organisation_schema) - # unpack valid json into service object - valid_organisation = Organisation.from_json(data) + organisation = Organisation(**data) - dao_create_organisation(valid_organisation) - return jsonify(data=organisation_schema.dump(valid_organisation).data), 201 + dao_create_organisation(organisation) + return jsonify(data=organisation.serialize()), 201 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/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index ec9d3de58..6e5b0b763 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -70,12 +70,12 @@ def test_create_organisation_without_logo_raises_error(client, notify_db, notify data=json.dumps(data) ) assert response.status_code == 400 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['errors'][0]['message'] == "logo is a required property" def test_create_organisation_without_name_or_colour_is_valid(client, notify_db, notify_db_session): data = { - 'name': None, - 'colour': None, 'logo': 'images/text_x2.png' } auth_header = create_authorization_header() @@ -85,4 +85,4 @@ def test_create_organisation_without_name_or_colour_is_valid(client, notify_db, headers=[('Content-Type', 'application/json'), auth_header], data=json.dumps(data) ) - assert response.status_code == 400 + assert response.status_code == 201