From 15112b214838aebd8dd0ed5f722d948ffc9a1edb Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 9 Jun 2020 11:02:22 +0100 Subject: [PATCH 1/3] Update post_create_template_schema Updated the `post_create_template_schema` to check that the postage is one of our allowed values. --- app/template/template_schemas.py | 2 +- tests/app/template/test_rest.py | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/app/template/template_schemas.py b/app/template/template_schemas.py index 9f38262a5..bb069db30 100644 --- a/app/template/template_schemas.py +++ b/app/template/template_schemas.py @@ -18,7 +18,7 @@ post_create_template_schema = { "subject": {"type": "string"}, "created_by": uuid, "parent_folder_id": uuid, - "postage": {"type": "string"}, + "postage": {"type": "string", "format": "postage"}, }, "if": { "properties": { diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index cb92e0b66..5726231bd 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -778,6 +778,41 @@ def test_create_a_template_with_foreign_service_reply_to(admin_request, sample_u ) +@pytest.mark.parametrize('post_data, expected_errors', [ + ( + {}, + [ + {"error": "ValidationError", "message": "subject is a required property"}, + {"error": "ValidationError", "message": "name is a required property"}, + {"error": "ValidationError", "message": "template_type is a required property"}, + {"error": "ValidationError", "message": "content is a required property"}, + {"error": "ValidationError", "message": "service is a required property"}, + {"error": "ValidationError", "message": "created_by is a required property"}, + ] + ), + ( + {"name": "my template", "template_type": "sms", "content": "hi", "postage": "third", + "service": "1af43c02-b5a8-4923-ad7f-5279b75ff2d0", "created_by": "30587644-9083-44d8-a114-98887f07f1e3"}, + [ + {"error": "ValidationError", "message": "postage invalid. It must be either first or second."}, + ] + ), +]) +def test_create_template_validates_against_json_schema( + admin_request, + sample_service_full_permissions, + post_data, + expected_errors, +): + response = admin_request.post( + 'template.create_template', + service_id=sample_service_full_permissions.id, + _data=post_data, + _expected_status=400 + ) + assert response['errors'] == expected_errors + + @pytest.mark.parametrize('template_default, service_default', [('template address', 'service address'), (None, 'service address'), From 72be10c6811ecc032adf2f3fd532341fc36fa3a8 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 9 Jun 2020 17:44:09 +0100 Subject: [PATCH 2/3] Add JSON schema for updating template We did not have a JSON schema for updating a template. Since we will remove the postage constraint from the templates table, this adds a JSON schema for updating a template so that we can use it to check that the postage is one of the allowed values. --- app/template/rest.py | 3 ++- app/template/template_schemas.py | 25 +++++++++++++++++++++++-- tests/app/template/test_rest.py | 13 +++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index 80e33ff9c..10a9653d0 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -37,7 +37,7 @@ from app.models import SMS_TYPE, Template, SECOND_CLASS, LETTER_TYPE from app.notifications.validators import service_has_permission, check_reply_to from app.schema_validation import validate from app.schemas import (template_schema, template_history_schema) -from app.template.template_schemas import post_create_template_schema +from app.template.template_schemas import post_create_template_schema, post_update_template_schema from app.utils import get_public_notify_type_text template_blueprint = Blueprint('template', __name__, url_prefix='/service//template') @@ -110,6 +110,7 @@ def update_template(service_id, template_id): raise InvalidRequest(errors, 403) data = request.get_json() + validate(data, post_update_template_schema) # if redacting, don't update anything else if data.get('redact_personalisation') is True: diff --git a/app/template/template_schemas.py b/app/template/template_schemas.py index bb069db30..6c1033940 100644 --- a/app/template/template_schemas.py +++ b/app/template/template_schemas.py @@ -2,7 +2,7 @@ from app.models import ( TEMPLATE_PROCESS_TYPE, TEMPLATE_TYPES, ) -from app.schema_validation.definitions import uuid +from app.schema_validation.definitions import nullable_uuid, uuid post_create_template_schema = { "$schema": "http://json-schema.org/draft-07/schema#", @@ -13,7 +13,7 @@ post_create_template_schema = { "name": {"type": "string"}, "template_type": {"enum": TEMPLATE_TYPES}, "service": uuid, - "process_type": {"emun": TEMPLATE_PROCESS_TYPE}, + "process_type": {"enum": TEMPLATE_PROCESS_TYPE}, "content": {"type": "string"}, "subject": {"type": "string"}, "created_by": uuid, @@ -28,3 +28,24 @@ post_create_template_schema = { "then": {"required": ["subject"]}, "required": ["name", "template_type", "content", "service", "created_by"] } + +post_update_template_schema = { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "POST update existing template", + "type": "object", + "title": "payload for POST /service//template/", + "properties": { + "id": uuid, + "name": {"type": "string"}, + "template_type": {"enum": TEMPLATE_TYPES}, + "service": uuid, + "process_type": {"enum": TEMPLATE_PROCESS_TYPE}, + "content": {"type": "string"}, + "subject": {"type": "string"}, + "postage": {"type": "string", "format": "postage"}, + "reply_to": nullable_uuid, + "created_by": uuid, + "archived": {"type": "boolean"}, + "current_user": uuid + }, +} diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 5726231bd..1ed7905e8 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -884,6 +884,19 @@ def test_update_template_reply_to_set_to_blank(client, notify_db_session): assert th.service_letter_contact_id is None +def test_update_template_validates_postage(admin_request, sample_service_full_permissions): + template = create_template(service=sample_service_full_permissions, template_type='letter') + + response = admin_request.post( + 'template.update_template', + service_id=sample_service_full_permissions.id, + template_id=template.id, + _data={"postage": "third"}, + _expected_status=400 + ) + assert 'postage invalid' in response['errors'][0]['message'] + + def test_update_template_with_foreign_service_reply_to(client, sample_letter_template): auth_header = create_authorization_header() From 98a69684c540eb696fe088381025280ca9fc846a Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 10 Jun 2020 16:57:19 +0100 Subject: [PATCH 3/3] Update get_template_by_id_response & post_template_preview_response schemas To check the format of postage. Neither of these two schemas are used for validating - they seem to be added for reference. --- app/v2/template/template_schemas.py | 4 ++-- tests/app/v2/template/test_template_schemas.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index 378eb8121..47b1bcb99 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -37,7 +37,7 @@ get_template_by_id_response = { "body": {"type": "string"}, "subject": {"type": ["string", "null"]}, "name": {"type": "string"}, - "postage": {"type": "string"} + "postage": {"type": "string", "format": "postage"} }, "required": ["id", "type", "created_at", "updated_at", "version", "created_by", "body", "name"], } @@ -65,7 +65,7 @@ post_template_preview_response = { "version": {"type": "integer"}, "body": {"type": "string"}, "subject": {"type": ["string", "null"]}, - "postage": {"type": "string"}, + "postage": {"type": "string", "format": "postage"}, "html": {"type": ["string", "null"]}, }, "required": ["id", "type", "version", "body"], diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index de84ad661..1ac8a0cbc 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -35,6 +35,7 @@ valid_json_get_response_with_optionals = { 'body': 'some body', 'subject': "some subject", 'name': 'some name', + 'postage': 'first', } valid_request_args = [{"id": str(uuid.uuid4()), "version": 1}, {"id": str(uuid.uuid4())}] @@ -80,6 +81,7 @@ valid_json_post_response_with_optionals = { 'version': 1, 'body': "some body", 'subject': 'some subject', + 'postage': 'second', 'html': '

some body

', }