From f5da3574b564e473e9d60271695d7efa7c347541 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 24 Mar 2017 10:23:47 +0000 Subject: [PATCH 01/10] Add get all templates schema --- app/v2/template/template_schemas.py | 46 ++++++++ app/v2/template_schema.py | 15 +++ .../app/v2/template/test_template_schemas.py | 109 ++++++++++++++++-- 3 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 app/v2/template_schema.py diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index 6f08d9cfc..0a326ecdd 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -1,5 +1,6 @@ from app.models import SMS_TYPE, TEMPLATE_TYPES from app.schema_validation.definitions import uuid, personalisation +from app.v2.template_schema import template get_template_by_id_request = { @@ -67,6 +68,51 @@ post_template_preview_response = { "required": ["id", "type", "version", "body"] } +get_all_template_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "request schema for parameters allowed when getting all templates", + "type": "object", + "properties": { + "type": {"enum": TEMPLATE_TYPES}, + }, + "required": ["type"], + "additionalProperties": False, +} + +get_all_template_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "GET response schema when getting all templates", + "type": "object", + "properties": { + "links": { + "type": "object", + "properties": { + "self": { + "type": "string", + "format": "uri" + }, + "next": { + "type": "string", + "format": "uri" + } + }, + "additionalProperties": False, + "required": ["self"], + }, + "templates": { + "type": "array", + "items": { + "type": "object", + "$ref": "#/definitions/template" + } + } + }, + "required": ["links", "templates"], + "definitions": { + "template": template + } +} + def create_post_template_preview_response(template, template_object): subject = template_object.subject if template.template_type != SMS_TYPE else None diff --git a/app/v2/template_schema.py b/app/v2/template_schema.py new file mode 100644 index 000000000..24252f083 --- /dev/null +++ b/app/v2/template_schema.py @@ -0,0 +1,15 @@ +from app.schema_validation.definitions import uuid + +# this may belong in a templates module +template = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "template schema", + "type": "object", + "title": "notification content", + "properties": { + "id": uuid, + "version": {"type": "integer"}, + "uri": {"type": "string", "format": "uri"} + }, + "required": ["id", "version", "uri"] +} diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index c89802942..9e7ea5c11 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -3,12 +3,14 @@ import uuid import pytest from flask import json -from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, TEMPLATE_TYPES +from app.models import EMAIL_TYPE, SMS_TYPE, TEMPLATE_TYPES from app.v2.template.template_schemas import ( get_template_by_id_response, get_template_by_id_request, post_template_preview_request, - post_template_preview_response + post_template_preview_response, + get_all_template_request, + get_all_template_response ) from app.schema_validation import validate from jsonschema.exceptions import ValidationError @@ -55,9 +57,7 @@ invalid_json_post_args = [ ({"id": str(uuid.uuid4()), "personalisation": "invalid_personalisation"}, ["personalisation should contain key value pairs"]), ({"personalisation": "invalid_personalisation"}, - ["id is a required property", - "personalisation is a required property", - "personalisation should contain key value pairs"]) + ["id is a required property", "personalisation should contain key value pairs"]) ] valid_json_post_response = { @@ -77,6 +77,74 @@ valid_json_post_response_with_optionals = { 'subject': 'some subject' } +valid_json_get_all_response = [ + { + 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, + {"id": str(uuid.uuid4()), "version": 2, "uri": "http://template/id"} + ] + }, + { + 'links': {'self': 'http://some.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] + }, + { + 'links': {'self': 'http://some.path'}, + "templates": [] + } +] + +invalid_json_get_all_response = [ + ({ + 'links': {'self': 'invalid_uri'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"} + ] + }, ['links invalid_uri is not a valid URI.']), + ({ + 'links': {'self': 'http://some.path'}, + "templates": [ + {"id": 'invalid_id', "version": 1, "uri": "http://template/id"} + ] + }, ['templates is not a valid UUID']), + ({ + 'links': {'self': 'http://some.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 'invalid_version', "uri": "http://template/id"} + ] + }, ['templates invalid_version is not of type integer']), + ({ + 'links': {'self': 'http://some.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "invalid_uri"} + ] + }, ['templates invalid_uri is not a valid URI.']), + ({ + 'links': {'self': 'http://some.path'} + }, ['templates is a required property']), + ({ + 'links': {'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] + }, ['links self is a required property']), + ({ + 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"version": 1, "uri": "http://template/id"}] + }, ['templates id is a required property']), + ({ + 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "uri": "http://template/id"}] + }, ['templates version is a required property']), + ({ + 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1}] + }, ['templates uri is a required property']), + ({ + 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"version": 1}] + }, ['templates id is a required property', 'templates uri is a required property']), +] + @pytest.mark.parametrize("args", valid_request_args) def test_get_template_request_schema_against_valid_args_is_valid(args): @@ -111,16 +179,16 @@ def test_post_template_preview_against_valid_args_is_valid(): assert validate(valid_json_post_args, post_template_preview_request) == valid_json_post_args -@pytest.mark.parametrize("args,error_message", invalid_json_post_args) -def test_post_template_preview_against_invalid_args_is_invalid(args, error_message): +@pytest.mark.parametrize("args,error_messages", invalid_json_post_args) +def test_post_template_preview_against_invalid_args_is_invalid(args, error_messages): with pytest.raises(ValidationError) as e: validate(args, post_template_preview_request) errors = json.loads(str(e.value)) assert errors['status_code'] == 400 - + assert len(errors['errors']) == len(error_messages) for error in errors['errors']: - assert error['message'] in error_message + assert error['message'] in error_messages @pytest.mark.parametrize("template_type", TEMPLATE_TYPES) @@ -129,3 +197,26 @@ def test_post_template_preview_response_schema_is_valid(response, template_type) response['type'] = template_type assert validate(response, post_template_preview_response) == response + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_valid_args_is_valid(template_type): + data = {'type': template_type} + assert validate(data, get_all_template_request) == data + + +@pytest.mark.parametrize("response", valid_json_get_all_response) +def test_valid_get_all_templates_response_schema_is_valid(response): + assert validate(response, get_all_template_response) == response + + +@pytest.mark.parametrize("response,error_messages", invalid_json_get_all_response) +def test_invalid_get_all_templates_response_schema_is_invalid(response, error_messages): + with pytest.raises(ValidationError) as e: + validate(response, get_all_template_response) + errors = json.loads(str(e.value)) + + assert errors['status_code'] == 400 + assert len(errors['errors']) == len(error_messages) + for error in errors['errors']: + assert error['message'] in error_messages From 89e244ccd2cf8c76ad94afbf7cbe462a224d6c85 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 28 Mar 2017 10:41:25 +0100 Subject: [PATCH 02/10] First commit --- app/__init__.py | 4 + app/models.py | 3 +- app/v2/template/get_template.py | 5 +- app/v2/templates/__init__.py | 7 + app/v2/templates/get_templates.py | 38 +++++ app/v2/templates/templates_schemas.py | 49 ++++++ tests/app/v2/template/test_get_template.py | 3 +- .../app/v2/template/test_template_schemas.py | 95 +----------- tests/app/v2/templates/test_get_templates.py | 117 +++++++++++++++ .../v2/templates/test_templates_schemas.py | 142 ++++++++++++++++++ 10 files changed, 361 insertions(+), 102 deletions(-) create mode 100644 app/v2/templates/__init__.py create mode 100644 app/v2/templates/get_templates.py create mode 100644 app/v2/templates/templates_schemas.py create mode 100644 tests/app/v2/templates/test_get_templates.py create mode 100644 tests/app/v2/templates/test_templates_schemas.py diff --git a/app/__init__.py b/app/__init__.py index b64f1e745..8e774cccb 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -156,6 +156,7 @@ def register_v2_blueprints(application): from app.v2.notifications.post_notifications import v2_notification_blueprint as post_notifications from app.v2.notifications.get_notifications import v2_notification_blueprint as get_notifications from app.v2.template.get_template import v2_template_blueprint as get_template + from app.v2.templates.get_templates import v2_templates_blueprint as get_templates from app.v2.template.post_template import v2_template_blueprint as post_template from app.authentication.auth import requires_auth @@ -165,6 +166,9 @@ def register_v2_blueprints(application): get_notifications.before_request(requires_auth) application.register_blueprint(get_notifications) + get_templates.before_request(requires_auth) + application.register_blueprint(get_templates) + get_template.before_request(requires_auth) application.register_blueprint(get_template) diff --git a/app/models.py b/app/models.py index 7b1b62cc2..f829ca429 100644 --- a/app/models.py +++ b/app/models.py @@ -321,9 +321,8 @@ class Template(db.Model): ) def serialize(self): - serialized = { - "id": self.id, + "id": str(self.id), "type": self.template_type, "created_at": self.created_at.strftime(DATETIME_FORMAT), "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None, diff --git a/app/v2/template/get_template.py b/app/v2/template/get_template.py index 8bfbddc5a..fef0e1b2e 100644 --- a/app/v2/template/get_template.py +++ b/app/v2/template/get_template.py @@ -1,8 +1,5 @@ -import uuid - -from flask import jsonify, request +from flask import jsonify from jsonschema.exceptions import ValidationError -from werkzeug.exceptions import abort from app import api_user from app.dao import templates_dao diff --git a/app/v2/templates/__init__.py b/app/v2/templates/__init__.py new file mode 100644 index 000000000..6e0989dd4 --- /dev/null +++ b/app/v2/templates/__init__.py @@ -0,0 +1,7 @@ +from flask import Blueprint + +from app.v2.errors import register_errors + +v2_templates_blueprint = Blueprint("v2_templates", __name__, url_prefix='/v2/templates') + +register_errors(v2_templates_blueprint) diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py new file mode 100644 index 000000000..0fd2170d2 --- /dev/null +++ b/app/v2/templates/get_templates.py @@ -0,0 +1,38 @@ +import json + +from flask import jsonify, request, current_app, url_for +from jsonschema.exceptions import ValidationError + +from app import api_user +from app.dao import templates_dao +from app.schema_validation import validate +from app.v2.templates import v2_templates_blueprint +from app.v2.templates.templates_schemas import get_all_template_request + + +@v2_templates_blueprint.route("/", methods=['GET']) +def get_templates(): + _data = request.args.to_dict() + + data = validate(_data, get_all_template_request) + + templates = templates_dao.dao_get_all_templates_for_service( + api_user.service_id, + older_than=data.get('older_than'), + page_size=current_app.config.get('API_PAGE_SIZE')) + + def _build_links(templates): + _links = { + 'current': url_for(".get_templates", _external=True, **data), + } + + if len(templates): + next_query_params = dict(data, older_than=templates[-1].id) + _links['next'] = url_for(".get_templates", _external=True, **next_query_params) + + return _links + + return jsonify( + templates=[template.serialize() for template in templates], + links=_build_links(templates) + ), 200 diff --git a/app/v2/templates/templates_schemas.py b/app/v2/templates/templates_schemas.py new file mode 100644 index 000000000..5b1899b67 --- /dev/null +++ b/app/v2/templates/templates_schemas.py @@ -0,0 +1,49 @@ +from app.models import TEMPLATE_TYPES +from app.schema_validation.definitions import uuid +from app.v2.template_schema import template + + +get_all_template_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "request schema for parameters allowed when getting all templates", + "type": "object", + "properties": { + "type": {"enum": TEMPLATE_TYPES}, + "older_than": uuid + }, + "additionalProperties": False, +} + +get_all_template_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "GET response schema when getting all templates", + "type": "object", + "properties": { + "links": { + "type": "object", + "properties": { + "current": { + "type": "string", + "format": "uri" + }, + "next": { + "type": "string", + "format": "uri" + } + }, + "additionalProperties": False, + "required": ["current"], + }, + "templates": { + "type": "array", + "items": { + "type": "object", + "$ref": "#/definitions/template" + } + } + }, + "required": ["links", "templates"], + "definitions": { + "template": template + } +} diff --git a/tests/app/v2/template/test_get_template.py b/tests/app/v2/template/test_get_template.py index ef3c9f437..8b2dd697f 100644 --- a/tests/app/v2/template/test_get_template.py +++ b/tests/app/v2/template/test_get_template.py @@ -1,10 +1,9 @@ import pytest -import uuid from flask import json from app import DATETIME_FORMAT -from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, TEMPLATE_TYPES +from app.models import EMAIL_TYPE, TEMPLATE_TYPES from tests import create_authorization_header from tests.app.db import create_template diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index 9e7ea5c11..4517ad7e8 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -8,9 +8,7 @@ from app.v2.template.template_schemas import ( get_template_by_id_response, get_template_by_id_request, post_template_preview_request, - post_template_preview_response, - get_all_template_request, - get_all_template_response + post_template_preview_response ) from app.schema_validation import validate from jsonschema.exceptions import ValidationError @@ -77,74 +75,6 @@ valid_json_post_response_with_optionals = { 'subject': 'some subject' } -valid_json_get_all_response = [ - { - 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, - "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, - {"id": str(uuid.uuid4()), "version": 2, "uri": "http://template/id"} - ] - }, - { - 'links': {'self': 'http://some.path'}, - "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] - }, - { - 'links': {'self': 'http://some.path'}, - "templates": [] - } -] - -invalid_json_get_all_response = [ - ({ - 'links': {'self': 'invalid_uri'}, - "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"} - ] - }, ['links invalid_uri is not a valid URI.']), - ({ - 'links': {'self': 'http://some.path'}, - "templates": [ - {"id": 'invalid_id', "version": 1, "uri": "http://template/id"} - ] - }, ['templates is not a valid UUID']), - ({ - 'links': {'self': 'http://some.path'}, - "templates": [ - {"id": str(uuid.uuid4()), "version": 'invalid_version', "uri": "http://template/id"} - ] - }, ['templates invalid_version is not of type integer']), - ({ - 'links': {'self': 'http://some.path'}, - "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "invalid_uri"} - ] - }, ['templates invalid_uri is not a valid URI.']), - ({ - 'links': {'self': 'http://some.path'} - }, ['templates is a required property']), - ({ - 'links': {'next': 'http://some.other.path'}, - "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] - }, ['links self is a required property']), - ({ - 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, - "templates": [{"version": 1, "uri": "http://template/id"}] - }, ['templates id is a required property']), - ({ - 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, - "templates": [{"id": str(uuid.uuid4()), "uri": "http://template/id"}] - }, ['templates version is a required property']), - ({ - 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, - "templates": [{"id": str(uuid.uuid4()), "version": 1}] - }, ['templates uri is a required property']), - ({ - 'links': {'self': 'http://some.path', 'next': 'http://some.other.path'}, - "templates": [{"version": 1}] - }, ['templates id is a required property', 'templates uri is a required property']), -] - @pytest.mark.parametrize("args", valid_request_args) def test_get_template_request_schema_against_valid_args_is_valid(args): @@ -197,26 +127,3 @@ def test_post_template_preview_response_schema_is_valid(response, template_type) response['type'] = template_type assert validate(response, post_template_preview_response) == response - - -@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) -def test_get_all_template_request_schema_against_valid_args_is_valid(template_type): - data = {'type': template_type} - assert validate(data, get_all_template_request) == data - - -@pytest.mark.parametrize("response", valid_json_get_all_response) -def test_valid_get_all_templates_response_schema_is_valid(response): - assert validate(response, get_all_template_response) == response - - -@pytest.mark.parametrize("response,error_messages", invalid_json_get_all_response) -def test_invalid_get_all_templates_response_schema_is_invalid(response, error_messages): - with pytest.raises(ValidationError) as e: - validate(response, get_all_template_response) - errors = json.loads(str(e.value)) - - assert errors['status_code'] == 400 - assert len(errors['errors']) == len(error_messages) - for error in errors['errors']: - assert error['message'] in error_messages diff --git a/tests/app/v2/templates/test_get_templates.py b/tests/app/v2/templates/test_get_templates.py new file mode 100644 index 000000000..7df477153 --- /dev/null +++ b/tests/app/v2/templates/test_get_templates.py @@ -0,0 +1,117 @@ +import pytest + +from flask import json + +from app import DATETIME_FORMAT +from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, TEMPLATE_TYPES +from tests import create_authorization_header +from tests.app.db import create_template + + +def test_get_all_templates(client, sample_service): + num_templates = 3 + templates = [] + for i in range(num_templates): + for tmp_type in TEMPLATE_TYPES: + templates.append(create_template(sample_service, template_type=tmp_type)) + + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.get(path='/v2/templates/?', + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response['templates']) == num_templates * len(TEMPLATE_TYPES) + + # need to reverse index as get all templates returns list sorted by descending date + for i in range(len(json_response['templates'])): + reverse_index = len(json_response['templates']) - 1 - i + assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) + assert json_response['templates'][reverse_index]['body'] == templates[i].content + assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + + +@pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) +def test_get_all_templates_for_type(client, sample_service, tmp_type): + num_templates = 3 + templates = [] + for i in range(num_templates): + templates.append(create_template(sample_service, template_type=tmp_type)) + + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.get(path='/v2/templates/?type={}'.format(tmp_type), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response['templates']) == num_templates + + # need to reverse index as get all templates returns list sorted by descending date + for i in range(len(json_response['templates'])): + reverse_index = len(json_response['templates']) - 1 - i + assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) + assert json_response['templates'][reverse_index]['body'] == templates[i].content + assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + + +@pytest.mark.parametrize("tmp_type", [EMAIL_TYPE, SMS_TYPE]) +def test_get_all_templates_older_than_parameter(client, sample_service, tmp_type): + num_templates = 5 + templates = [] + for i in range(num_templates): + template = create_template(sample_service, template_type=tmp_type) + templates.append(template) + + num_templates_older = 3 + + # only get the first #num_templates_older templates + older_than_id = templates[num_templates_older].id + + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.get(path='/v2/templates/?type={}&older_than={}'.format(tmp_type, older_than_id), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response['templates']) == num_templates_older + + # need to reverse index as get all templates returns list sorted by descending date + for i in range(num_templates_older): + reverse_index = num_templates_older - 1 - i + assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) + assert json_response['templates'][reverse_index]['body'] == templates[i].content + assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + + assert str(older_than_id) in json_response['links']['current'] + assert str(templates[0].id) in json_response['links']['next'] + + +@pytest.mark.parametrize("tmp_type", [EMAIL_TYPE, SMS_TYPE]) +def test_get_all_templates_none_existent_older_than_parameter(client, sample_service, tmp_type, fake_uuid): + num_templates = 2 + templates = [] + for i in range(num_templates): + template = create_template(sample_service, template_type=tmp_type) + templates.append(template) + + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.get(path='/v2/templates/?type={}&older_than={}'.format(tmp_type, fake_uuid), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response['templates']) == 0 diff --git a/tests/app/v2/templates/test_templates_schemas.py b/tests/app/v2/templates/test_templates_schemas.py new file mode 100644 index 000000000..a629ee65c --- /dev/null +++ b/tests/app/v2/templates/test_templates_schemas.py @@ -0,0 +1,142 @@ +import uuid + +import pytest +from flask import json + +from app.models import EMAIL_TYPE, SMS_TYPE, TEMPLATE_TYPES +from app.v2.templates.templates_schemas import ( + get_all_template_request, + get_all_template_response +) +from app.schema_validation import validate +from jsonschema.exceptions import ValidationError + + +valid_json_get_all_response = [ + { + 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, + {"id": str(uuid.uuid4()), "version": 2, "uri": "http://template/id"} + ] + }, + { + 'links': {'current': 'http://some.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] + }, + { + 'links': {'current': 'http://some.path'}, + "templates": [] + } +] + +invalid_json_get_all_response = [ + ({ + 'links': {'current': 'invalid_uri'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"} + ] + }, ['links invalid_uri is not a valid URI.']), + ({ + 'links': {'current': 'http://some.path'}, + "templates": [ + {"id": 'invalid_id', "version": 1, "uri": "http://template/id"} + ] + }, ['templates is not a valid UUID']), + ({ + 'links': {'current': 'http://some.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 'invalid_version', "uri": "http://template/id"} + ] + }, ['templates invalid_version is not of type integer']), + ({ + 'links': {'current': 'http://some.path'}, + "templates": [ + {"id": str(uuid.uuid4()), "version": 1, "uri": "invalid_uri"} + ] + }, ['templates invalid_uri is not a valid URI.']), + ({ + 'links': {'current': 'http://some.path'} + }, ['templates is a required property']), + ({ + 'links': {'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] + }, ['links current is a required property']), + ({ + 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"version": 1, "uri": "http://template/id"}] + }, ['templates id is a required property']), + ({ + 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "uri": "http://template/id"}] + }, ['templates version is a required property']), + ({ + 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"id": str(uuid.uuid4()), "version": 1}] + }, ['templates uri is a required property']), + ({ + 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, + "templates": [{"version": 1}] + }, ['templates id is a required property', 'templates uri is a required property']), +] + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_no_args_is_valid(template_type): + data = {} + assert validate(data, get_all_template_request) == data + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_valid_args_is_valid(template_type): + data = {'type': template_type} + assert validate(data, get_all_template_request) == data + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_valid_args_with_optional_is_valid(template_type, fake_uuid): + data = {'type': template_type, 'older_than': fake_uuid} + assert validate(data, get_all_template_request) == data + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_invalid_args_is_invalid(template_type): + data = {'type': 'unknown'} + + with pytest.raises(ValidationError) as e: + validate(data, get_all_template_request) + errors = json.loads(str(e.value)) + + assert errors['status_code'] == 400 + assert len(errors['errors']) == 1 + assert errors['errors'][0]['message'] == 'type unknown is not one of [sms, email, letter]' + + +@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) +def test_get_all_template_request_schema_against_invalid_args_with_optional_is_invalid(template_type): + data = {'type': template_type, 'older_than': 'invalid_uuid'} + + with pytest.raises(ValidationError) as e: + validate(data, get_all_template_request) + errors = json.loads(str(e.value)) + + assert errors['status_code'] == 400 + assert len(errors['errors']) == 1 + assert errors['errors'][0]['message'] == 'older_than is not a valid UUID' + + +@pytest.mark.parametrize("response", valid_json_get_all_response) +def test_valid_get_all_templates_response_schema_is_valid(response): + assert validate(response, get_all_template_response) == response + + +@pytest.mark.parametrize("response,error_messages", invalid_json_get_all_response) +def test_invalid_get_all_templates_response_schema_is_invalid(response, error_messages): + with pytest.raises(ValidationError) as e: + validate(response, get_all_template_response) + errors = json.loads(str(e.value)) + + assert errors['status_code'] == 400 + assert len(errors['errors']) == len(error_messages) + for error in errors['errors']: + assert error['message'] in error_messages From cb7cd233d175062b36327686804a0fb1320ccb13 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 28 Mar 2017 11:08:11 +0100 Subject: [PATCH 03/10] Rrefactored schema --- app/v2/templates/templates_schemas.py | 21 +-------- .../v2/templates/test_templates_schemas.py | 43 +------------------ 2 files changed, 3 insertions(+), 61 deletions(-) diff --git a/app/v2/templates/templates_schemas.py b/app/v2/templates/templates_schemas.py index 5b1899b67..0520b70c2 100644 --- a/app/v2/templates/templates_schemas.py +++ b/app/v2/templates/templates_schemas.py @@ -1,5 +1,4 @@ from app.models import TEMPLATE_TYPES -from app.schema_validation.definitions import uuid from app.v2.template_schema import template @@ -8,8 +7,7 @@ get_all_template_request = { "description": "request schema for parameters allowed when getting all templates", "type": "object", "properties": { - "type": {"enum": TEMPLATE_TYPES}, - "older_than": uuid + "type": {"enum": TEMPLATE_TYPES} }, "additionalProperties": False, } @@ -19,21 +17,6 @@ get_all_template_response = { "description": "GET response schema when getting all templates", "type": "object", "properties": { - "links": { - "type": "object", - "properties": { - "current": { - "type": "string", - "format": "uri" - }, - "next": { - "type": "string", - "format": "uri" - } - }, - "additionalProperties": False, - "required": ["current"], - }, "templates": { "type": "array", "items": { @@ -42,7 +25,7 @@ get_all_template_response = { } } }, - "required": ["links", "templates"], + "required": ["templates"], "definitions": { "template": template } diff --git a/tests/app/v2/templates/test_templates_schemas.py b/tests/app/v2/templates/test_templates_schemas.py index a629ee65c..f60f4366b 100644 --- a/tests/app/v2/templates/test_templates_schemas.py +++ b/tests/app/v2/templates/test_templates_schemas.py @@ -14,68 +14,46 @@ from jsonschema.exceptions import ValidationError valid_json_get_all_response = [ { - 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, "templates": [ {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, {"id": str(uuid.uuid4()), "version": 2, "uri": "http://template/id"} ] }, { - 'links': {'current': 'http://some.path'}, "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] }, { - 'links': {'current': 'http://some.path'}, "templates": [] } ] invalid_json_get_all_response = [ ({ - 'links': {'current': 'invalid_uri'}, - "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"} - ] - }, ['links invalid_uri is not a valid URI.']), - ({ - 'links': {'current': 'http://some.path'}, "templates": [ {"id": 'invalid_id', "version": 1, "uri": "http://template/id"} ] }, ['templates is not a valid UUID']), ({ - 'links': {'current': 'http://some.path'}, "templates": [ {"id": str(uuid.uuid4()), "version": 'invalid_version', "uri": "http://template/id"} ] }, ['templates invalid_version is not of type integer']), ({ - 'links': {'current': 'http://some.path'}, "templates": [ {"id": str(uuid.uuid4()), "version": 1, "uri": "invalid_uri"} ] }, ['templates invalid_uri is not a valid URI.']), + ({}, ['templates is a required property']), ({ - 'links': {'current': 'http://some.path'} - }, ['templates is a required property']), - ({ - 'links': {'next': 'http://some.other.path'}, - "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] - }, ['links current is a required property']), - ({ - 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, "templates": [{"version": 1, "uri": "http://template/id"}] }, ['templates id is a required property']), ({ - 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, "templates": [{"id": str(uuid.uuid4()), "uri": "http://template/id"}] }, ['templates version is a required property']), ({ - 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, "templates": [{"id": str(uuid.uuid4()), "version": 1}] }, ['templates uri is a required property']), ({ - 'links': {'current': 'http://some.path', 'next': 'http://some.other.path'}, "templates": [{"version": 1}] }, ['templates id is a required property', 'templates uri is a required property']), ] @@ -93,12 +71,6 @@ def test_get_all_template_request_schema_against_valid_args_is_valid(template_ty assert validate(data, get_all_template_request) == data -@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) -def test_get_all_template_request_schema_against_valid_args_with_optional_is_valid(template_type, fake_uuid): - data = {'type': template_type, 'older_than': fake_uuid} - assert validate(data, get_all_template_request) == data - - @pytest.mark.parametrize("template_type", TEMPLATE_TYPES) def test_get_all_template_request_schema_against_invalid_args_is_invalid(template_type): data = {'type': 'unknown'} @@ -112,19 +84,6 @@ def test_get_all_template_request_schema_against_invalid_args_is_invalid(templat assert errors['errors'][0]['message'] == 'type unknown is not one of [sms, email, letter]' -@pytest.mark.parametrize("template_type", TEMPLATE_TYPES) -def test_get_all_template_request_schema_against_invalid_args_with_optional_is_invalid(template_type): - data = {'type': template_type, 'older_than': 'invalid_uuid'} - - with pytest.raises(ValidationError) as e: - validate(data, get_all_template_request) - errors = json.loads(str(e.value)) - - assert errors['status_code'] == 400 - assert len(errors['errors']) == 1 - assert errors['errors'][0]['message'] == 'older_than is not a valid UUID' - - @pytest.mark.parametrize("response", valid_json_get_all_response) def test_valid_get_all_templates_response_schema_is_valid(response): assert validate(response, get_all_template_response) == response From e2ad8ba50d1abfd1f699781075b62b645c5d1de1 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 28 Mar 2017 13:14:45 +0100 Subject: [PATCH 04/10] Renamed get template test --- tests/app/v2/template/test_get_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/v2/template/test_get_template.py b/tests/app/v2/template/test_get_template.py index 8b2dd697f..0f028a967 100644 --- a/tests/app/v2/template/test_get_template.py +++ b/tests/app/v2/template/test_get_template.py @@ -12,7 +12,7 @@ valid_version_params = [None, 1] @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) @pytest.mark.parametrize("version", valid_version_params) -def test_get_email_template_by_id_returns_200(client, sample_service, tmp_type, version): +def test_get_template_by_id_returns_200(client, sample_service, tmp_type, version): template = create_template(sample_service, template_type=tmp_type) auth_header = create_authorization_header(service_id=sample_service.id) From d290a2e0ad9b3e6fa33dd6095ceb2f92faa20898 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 28 Mar 2017 13:15:39 +0100 Subject: [PATCH 05/10] Removed paging from get all templates --- app/v2/templates/get_templates.py | 27 ++------ tests/app/v2/templates/test_get_templates.py | 72 ++++++-------------- 2 files changed, 24 insertions(+), 75 deletions(-) diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py index 0fd2170d2..a5f4e6f0c 100644 --- a/app/v2/templates/get_templates.py +++ b/app/v2/templates/get_templates.py @@ -1,6 +1,4 @@ -import json - -from flask import jsonify, request, current_app, url_for +from flask import jsonify, request from jsonschema.exceptions import ValidationError from app import api_user @@ -12,27 +10,10 @@ from app.v2.templates.templates_schemas import get_all_template_request @v2_templates_blueprint.route("/", methods=['GET']) def get_templates(): - _data = request.args.to_dict() + validate(request.args.to_dict(), get_all_template_request) - data = validate(_data, get_all_template_request) - - templates = templates_dao.dao_get_all_templates_for_service( - api_user.service_id, - older_than=data.get('older_than'), - page_size=current_app.config.get('API_PAGE_SIZE')) - - def _build_links(templates): - _links = { - 'current': url_for(".get_templates", _external=True, **data), - } - - if len(templates): - next_query_params = dict(data, older_than=templates[-1].id) - _links['next'] = url_for(".get_templates", _external=True, **next_query_params) - - return _links + templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id) return jsonify( - templates=[template.serialize() for template in templates], - links=_build_links(templates) + templates=[template.serialize() for template in templates] ), 200 diff --git a/tests/app/v2/templates/test_get_templates.py b/tests/app/v2/templates/test_get_templates.py index 7df477153..f7ddd1c8f 100644 --- a/tests/app/v2/templates/test_get_templates.py +++ b/tests/app/v2/templates/test_get_templates.py @@ -2,13 +2,12 @@ import pytest from flask import json -from app import DATETIME_FORMAT -from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, TEMPLATE_TYPES +from app.models import TEMPLATE_TYPES from tests import create_authorization_header from tests.app.db import create_template -def test_get_all_templates(client, sample_service): +def test_get_all_templates_returns_200(client, sample_service): num_templates = 3 templates = [] for i in range(num_templates): @@ -17,9 +16,10 @@ def test_get_all_templates(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id) - response = client.get(path='/v2/templates/?', + response = client.get(path='/v2/templates/', headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' json_response = json.loads(response.get_data(as_text=True)) @@ -35,7 +35,7 @@ def test_get_all_templates(client, sample_service): @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) -def test_get_all_templates_for_type(client, sample_service, tmp_type): +def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tmp_type): num_templates = 3 templates = [] for i in range(num_templates): @@ -46,6 +46,7 @@ def test_get_all_templates_for_type(client, sample_service, tmp_type): response = client.get(path='/v2/templates/?type={}'.format(tmp_type), headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' json_response = json.loads(response.get_data(as_text=True)) @@ -60,58 +61,25 @@ def test_get_all_templates_for_type(client, sample_service, tmp_type): assert json_response['templates'][reverse_index]['type'] == templates[i].template_type -@pytest.mark.parametrize("tmp_type", [EMAIL_TYPE, SMS_TYPE]) -def test_get_all_templates_older_than_parameter(client, sample_service, tmp_type): - num_templates = 5 - templates = [] - for i in range(num_templates): - template = create_template(sample_service, template_type=tmp_type) - templates.append(template) - - num_templates_older = 3 - - # only get the first #num_templates_older templates - older_than_id = templates[num_templates_older].id - +def test_get_all_templates_for_invalid_type_returns_400(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id) - response = client.get(path='/v2/templates/?type={}&older_than={}'.format(tmp_type, older_than_id), + invalid_type = 'coconut' + + response = client.get(path='/v2/templates/?type={}'.format(invalid_type), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 200 + assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' json_response = json.loads(response.get_data(as_text=True)) - assert len(json_response['templates']) == num_templates_older - - # need to reverse index as get all templates returns list sorted by descending date - for i in range(num_templates_older): - reverse_index = num_templates_older - 1 - i - assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) - assert json_response['templates'][reverse_index]['body'] == templates[i].content - assert json_response['templates'][reverse_index]['type'] == templates[i].template_type - - assert str(older_than_id) in json_response['links']['current'] - assert str(templates[0].id) in json_response['links']['next'] - - -@pytest.mark.parametrize("tmp_type", [EMAIL_TYPE, SMS_TYPE]) -def test_get_all_templates_none_existent_older_than_parameter(client, sample_service, tmp_type, fake_uuid): - num_templates = 2 - templates = [] - for i in range(num_templates): - template = create_template(sample_service, template_type=tmp_type) - templates.append(template) - - auth_header = create_authorization_header(service_id=sample_service.id) - - response = client.get(path='/v2/templates/?type={}&older_than={}'.format(tmp_type, fake_uuid), - headers=[('Content-Type', 'application/json'), auth_header]) - - assert response.status_code == 200 - assert response.headers['Content-type'] == 'application/json' - - json_response = json.loads(response.get_data(as_text=True)) - - assert len(json_response['templates']) == 0 + assert json_response == { + 'status_code': 400, + 'errors': [ + { + 'message': 'type coconut is not one of [sms, email, letter]', + 'error': 'ValidationError' + } + ] + } From a5e514c3564372bdd3f07e812258d15528df2cdf Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 4 Apr 2017 14:04:56 +0100 Subject: [PATCH 06/10] Refactored to use template response as template --- app/v2/template/template_schemas.py | 46 ----- app/v2/template_schema.py | 15 -- app/v2/templates/get_templates.py | 2 + app/v2/templates/templates_schemas.py | 2 +- .../v2/templates/test_templates_schemas.py | 159 ++++++++++++++++-- 5 files changed, 149 insertions(+), 75 deletions(-) delete mode 100644 app/v2/template_schema.py diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index 0a326ecdd..6f08d9cfc 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -1,6 +1,5 @@ from app.models import SMS_TYPE, TEMPLATE_TYPES from app.schema_validation.definitions import uuid, personalisation -from app.v2.template_schema import template get_template_by_id_request = { @@ -68,51 +67,6 @@ post_template_preview_response = { "required": ["id", "type", "version", "body"] } -get_all_template_request = { - "$schema": "http://json-schema.org/draft-04/schema#", - "description": "request schema for parameters allowed when getting all templates", - "type": "object", - "properties": { - "type": {"enum": TEMPLATE_TYPES}, - }, - "required": ["type"], - "additionalProperties": False, -} - -get_all_template_response = { - "$schema": "http://json-schema.org/draft-04/schema#", - "description": "GET response schema when getting all templates", - "type": "object", - "properties": { - "links": { - "type": "object", - "properties": { - "self": { - "type": "string", - "format": "uri" - }, - "next": { - "type": "string", - "format": "uri" - } - }, - "additionalProperties": False, - "required": ["self"], - }, - "templates": { - "type": "array", - "items": { - "type": "object", - "$ref": "#/definitions/template" - } - } - }, - "required": ["links", "templates"], - "definitions": { - "template": template - } -} - def create_post_template_preview_response(template, template_object): subject = template_object.subject if template.template_type != SMS_TYPE else None diff --git a/app/v2/template_schema.py b/app/v2/template_schema.py deleted file mode 100644 index 24252f083..000000000 --- a/app/v2/template_schema.py +++ /dev/null @@ -1,15 +0,0 @@ -from app.schema_validation.definitions import uuid - -# this may belong in a templates module -template = { - "$schema": "http://json-schema.org/draft-04/schema#", - "description": "template schema", - "type": "object", - "title": "notification content", - "properties": { - "id": uuid, - "version": {"type": "integer"}, - "uri": {"type": "string", "format": "uri"} - }, - "required": ["id", "version", "uri"] -} diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py index a5f4e6f0c..348ed13cc 100644 --- a/app/v2/templates/get_templates.py +++ b/app/v2/templates/get_templates.py @@ -14,6 +14,8 @@ def get_templates(): templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id) + print(templates) + return jsonify( templates=[template.serialize() for template in templates] ), 200 diff --git a/app/v2/templates/templates_schemas.py b/app/v2/templates/templates_schemas.py index 0520b70c2..d5b335874 100644 --- a/app/v2/templates/templates_schemas.py +++ b/app/v2/templates/templates_schemas.py @@ -1,5 +1,5 @@ from app.models import TEMPLATE_TYPES -from app.v2.template_schema import template +from app.v2.template.template_schemas import get_template_by_id_response as template get_all_template_request = { diff --git a/tests/app/v2/templates/test_templates_schemas.py b/tests/app/v2/templates/test_templates_schemas.py index f60f4366b..d42233513 100644 --- a/tests/app/v2/templates/test_templates_schemas.py +++ b/tests/app/v2/templates/test_templates_schemas.py @@ -15,12 +15,38 @@ from jsonschema.exceptions import ValidationError valid_json_get_all_response = [ { "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, - {"id": str(uuid.uuid4()), "version": 2, "uri": "http://template/id"} + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-01-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + }, + { + 'id': str(uuid.uuid4()), + 'type': EMAIL_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 2, + 'created_by': 'someone@test.com', + 'body': 'some body' + } ] }, { - "templates": [{"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}] + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 2, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] }, { "templates": [] @@ -30,32 +56,139 @@ valid_json_get_all_response = [ invalid_json_get_all_response = [ ({ "templates": [ - {"id": 'invalid_id', "version": 1, "uri": "http://template/id"} + { + 'id': 'invalid_id', + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } ] }, ['templates is not a valid UUID']), ({ "templates": [ - {"id": str(uuid.uuid4()), "version": 'invalid_version', "uri": "http://template/id"} + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 'invalid_version', + 'created_by': 'someone@test.com', + 'body': 'some body' + } ] }, ['templates invalid_version is not of type integer']), ({ "templates": [ - {"id": str(uuid.uuid4()), "version": 1, "uri": "invalid_uri"} + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': 'invalid_created_at', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } ] - }, ['templates invalid_uri is not a valid URI.']), + }, ['templates invalid_created_at is not a date-time']), ({}, ['templates is a required property']), ({ - "templates": [{"version": 1, "uri": "http://template/id"}] + "templates": [ + { + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] }, ['templates id is a required property']), ({ - "templates": [{"id": str(uuid.uuid4()), "uri": "http://template/id"}] + "templates": [ + { + 'id': str(uuid.uuid4()), + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] + }, ['templates type is a required property']), + ({ + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] + }, ['templates created_at is a required property']), + ({ + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'version': 1, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] + }, ['templates updated_at is a required property']), + ({ + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] }, ['templates version is a required property']), ({ - "templates": [{"id": str(uuid.uuid4()), "version": 1}] - }, ['templates uri is a required property']), + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'body': 'some body' + } + ] + }, ['templates created_by is a required property']), ({ - "templates": [{"version": 1}] - }, ['templates id is a required property', 'templates uri is a required property']), + "templates": [ + { + 'id': str(uuid.uuid4()), + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'version': 1, + 'created_by': 'someone@test.com' + } + ] + }, ['templates body is a required property']), + ({ + "templates": [ + { + 'type': SMS_TYPE, + 'created_at': '2017-02-10T18:25:43.511Z', + 'updated_at': None, + 'created_by': 'someone@test.com', + 'body': 'some body' + } + ] + }, ['templates id is a required property', 'templates version is a required property']), ] From 5ef0ecf9bf7956bf7359414ef58e128859750d59 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 4 Apr 2017 14:23:35 +0100 Subject: [PATCH 07/10] Added subject to get all templates email template --- tests/app/v2/templates/test_templates_schemas.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/v2/templates/test_templates_schemas.py b/tests/app/v2/templates/test_templates_schemas.py index d42233513..df798ce82 100644 --- a/tests/app/v2/templates/test_templates_schemas.py +++ b/tests/app/v2/templates/test_templates_schemas.py @@ -31,6 +31,7 @@ valid_json_get_all_response = [ 'updated_at': None, 'version': 2, 'created_by': 'someone@test.com', + 'subject': 'test subject', 'body': 'some body' } ] From c33b876c664178de92099b6553a6030789bdaaa4 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 18 Apr 2017 10:46:22 +0100 Subject: [PATCH 08/10] Remove get all template print --- app/v2/templates/get_templates.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py index 348ed13cc..a5f4e6f0c 100644 --- a/app/v2/templates/get_templates.py +++ b/app/v2/templates/get_templates.py @@ -14,8 +14,6 @@ def get_templates(): templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id) - print(templates) - return jsonify( templates=[template.serialize() for template in templates] ), 200 From ed953e992d27ee7aa04ac8dee5d738db570d61c6 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 18 Apr 2017 14:22:59 +0100 Subject: [PATCH 09/10] Add check for subject when email type --- tests/app/v2/templates/test_get_templates.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/app/v2/templates/test_get_templates.py b/tests/app/v2/templates/test_get_templates.py index f7ddd1c8f..5f706f0b9 100644 --- a/tests/app/v2/templates/test_get_templates.py +++ b/tests/app/v2/templates/test_get_templates.py @@ -2,7 +2,7 @@ import pytest from flask import json -from app.models import TEMPLATE_TYPES +from app.models import TEMPLATE_TYPES, EMAIL_TYPE from tests import create_authorization_header from tests.app.db import create_template @@ -12,7 +12,8 @@ def test_get_all_templates_returns_200(client, sample_service): templates = [] for i in range(num_templates): for tmp_type in TEMPLATE_TYPES: - templates.append(create_template(sample_service, template_type=tmp_type)) + subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else '' + templates.append(create_template(sample_service, template_type=tmp_type, subject=subject)) auth_header = create_authorization_header(service_id=sample_service.id) @@ -32,6 +33,8 @@ def test_get_all_templates_returns_200(client, sample_service): assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) assert json_response['templates'][reverse_index]['body'] == templates[i].content assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + if templates[i].template_type == EMAIL_TYPE: + assert json_response['templates'][reverse_index]['subject'] == templates[i].subject @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) @@ -39,7 +42,8 @@ def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tm num_templates = 3 templates = [] for i in range(num_templates): - templates.append(create_template(sample_service, template_type=tmp_type)) + subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else '' + templates.append(create_template(sample_service, template_type=tmp_type, subject=subject)) auth_header = create_authorization_header(service_id=sample_service.id) @@ -59,6 +63,8 @@ def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tm assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) assert json_response['templates'][reverse_index]['body'] == templates[i].content assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + if templates[i].template_type == EMAIL_TYPE: + assert json_response['templates'][reverse_index]['subject'] == templates[i].subject def test_get_all_templates_for_invalid_type_returns_400(client, sample_service): From fc0cfa5dbf954347e339a89302a5a3057fda4141 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 18 Apr 2017 16:19:25 +0100 Subject: [PATCH 10/10] Corrected code to handle template types --- app/dao/templates_dao.py | 11 ++++++++- app/v2/templates/get_templates.py | 4 +-- tests/app/v2/templates/test_get_templates.py | 26 +++++++++++++++++++- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 567e763f3..493c7c072 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -42,7 +42,16 @@ def dao_get_template_by_id(template_id, version=None): return Template.query.filter_by(id=template_id).one() -def dao_get_all_templates_for_service(service_id): +def dao_get_all_templates_for_service(service_id, template_type=None): + if template_type is not None: + return Template.query.filter_by( + service_id=service_id, + template_type=template_type, + archived=False + ).order_by( + desc(Template.created_at) + ).all() + return Template.query.filter_by( service_id=service_id, archived=False diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py index a5f4e6f0c..101ace2f9 100644 --- a/app/v2/templates/get_templates.py +++ b/app/v2/templates/get_templates.py @@ -10,9 +10,9 @@ from app.v2.templates.templates_schemas import get_all_template_request @v2_templates_blueprint.route("/", methods=['GET']) def get_templates(): - validate(request.args.to_dict(), get_all_template_request) + data = validate(request.args.to_dict(), get_all_template_request) - templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id) + templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id, data.get('type')) return jsonify( templates=[template.serialize() for template in templates] diff --git a/tests/app/v2/templates/test_get_templates.py b/tests/app/v2/templates/test_get_templates.py index 5f706f0b9..180752271 100644 --- a/tests/app/v2/templates/test_get_templates.py +++ b/tests/app/v2/templates/test_get_templates.py @@ -62,11 +62,35 @@ def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tm reverse_index = len(json_response['templates']) - 1 - i assert json_response['templates'][reverse_index]['id'] == str(templates[i].id) assert json_response['templates'][reverse_index]['body'] == templates[i].content - assert json_response['templates'][reverse_index]['type'] == templates[i].template_type + assert json_response['templates'][reverse_index]['type'] == tmp_type if templates[i].template_type == EMAIL_TYPE: assert json_response['templates'][reverse_index]['subject'] == templates[i].subject +@pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) +def test_get_correct_num_templates_for_valid_type_returns_200(client, sample_service, tmp_type): + num_templates = 3 + + templates = [] + for i in range(num_templates): + templates.append(create_template(sample_service, template_type=tmp_type)) + + for other_type in TEMPLATE_TYPES: + if other_type != tmp_type: + templates.append(create_template(sample_service, template_type=other_type)) + + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.get(path='/v2/templates/?type={}'.format(tmp_type), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response['templates']) == num_templates + + def test_get_all_templates_for_invalid_type_returns_400(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id)