diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 3d3dd9bc1..2a9164429 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -21,11 +21,16 @@ def dao_create_template(template): template.id = uuid.uuid4() # must be set now so version history model can use same id template.archived = False - template.template_redacted = TemplateRedacted( - template=template, - redact_personalisation=False, - updated_by=template.created_by - ) + redacted_dict = { + "template": template, + "redact_personalisation": False, + } + if template.created_by: + redacted_dict.update({"updated_by": template.created_by}) + else: + redacted_dict.update({"updated_by_id": template.created_by_id}) + + template.template_redacted = TemplateRedacted(**redacted_dict) db.session.add(template) diff --git a/app/models.py b/app/models.py index 3f2877dff..b9b460bfd 100644 --- a/app/models.py +++ b/app/models.py @@ -884,6 +884,19 @@ class Template(TemplateBase): _external=True ) + @classmethod + def from_json(cls, data, folder): + """ + Assumption: data has been validated appropriately. + Returns a Template object based on the provided data. + """ + fields = data.copy() + + fields['created_by_id'] = fields.pop('created_by') + fields['service_id'] = fields.pop('service') + fields['folder'] = folder + return cls(**fields) + class TemplateRedacted(db.Model): __tablename__ = 'template_redacted' diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 44cb0f9ce..382d9229c 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -3,7 +3,7 @@ from datetime import datetime, timedelta from uuid import UUID from iso8601 import iso8601, ParseError -from jsonschema import (Draft4Validator, ValidationError, FormatChecker) +from jsonschema import (Draft7Validator, ValidationError, FormatChecker) from notifications_utils.recipients import (validate_phone_number, validate_email_address, InvalidPhoneError, InvalidEmailError) @@ -43,7 +43,7 @@ def validate(json_to_validate, schema): "https://en.wikipedia.org/wiki/ISO_8601") return True - validator = Draft4Validator(schema, format_checker=format_checker) + validator = Draft7Validator(schema, format_checker=format_checker) errors = list(validator.iter_errors(json_to_validate)) if errors.__len__() > 0: raise ValidationError(build_error_message(errors)) diff --git a/app/template/rest.py b/app/template/rest.py index 64a353c0a..8cbb4125f 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -12,9 +12,11 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT from notifications_utils.pdf import extract_page_from_pdf from notifications_utils.template import SMSMessageTemplate from requests import post as requests_post +from sqlalchemy.orm.exc import NoResultFound from app.dao.notifications_dao import get_notification_by_id from app.dao.services_dao import dao_fetch_service_by_id +from app.dao.template_folder_dao import dao_get_template_folder_by_id_and_service_id from app.dao.templates_dao import ( dao_update_template, dao_create_template, @@ -29,9 +31,11 @@ from app.errors import ( InvalidRequest ) from app.letters.utils import get_letter_pdf -from app.models import SMS_TYPE +from app.models import SMS_TYPE, Template 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.utils import get_template_instance, get_public_notify_type_text template_blueprint = Blueprint('template', __name__, url_prefix='/service//template') @@ -46,12 +50,27 @@ def _content_count_greater_than_limit(content, template_type): return template.content_count > SMS_CHAR_COUNT_LIMIT +def validate_parent_folder(template_json): + if template_json.get("parent_folder_id"): + try: + return dao_get_template_folder_by_id_and_service_id( + template_folder_id=template_json.pop("parent_folder_id"), + service_id=template_json['service'] + ) + except NoResultFound: + raise InvalidRequest("parent_folder_id not found", status_code=400) + else: + return None + + @template_blueprint.route('', methods=['POST']) def create_template(service_id): fetched_service = dao_fetch_service_by_id(service_id=service_id) - # permissions needs to be placed here otherwise marshmallow will intefere with versioning + # permissions needs to be placed here otherwise marshmallow will interfere with versioning permissions = fetched_service.permissions - new_template = template_schema.load(request.get_json()).data + template_json = validate(request.get_json(), post_create_template_schema) + folder = validate_parent_folder(template_json=template_json) + new_template = Template.from_json(template_json, folder) if not service_has_permission(new_template.template_type, permissions): message = "Creating {} templates is not allowed".format( @@ -60,6 +79,7 @@ def create_template(service_id): raise InvalidRequest(errors, 403) new_template.service = fetched_service + over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type) if over_limit: message = 'Content has a character count greater than the limit of {}'.format(SMS_CHAR_COUNT_LIMIT) @@ -69,6 +89,7 @@ def create_template(service_id): check_reply_to(service_id, new_template.reply_to, new_template.template_type) dao_create_template(new_template) + return jsonify(data=template_schema.dump(new_template).data), 201 diff --git a/app/template/template_schemas.py b/app/template/template_schemas.py new file mode 100644 index 000000000..f63ad4575 --- /dev/null +++ b/app/template/template_schemas.py @@ -0,0 +1,29 @@ +from app.models import ( + TEMPLATE_PROCESS_TYPE, + TEMPLATE_TYPES, +) +from app.schema_validation.definitions import uuid + +post_create_template_schema = { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "POST create new template", + "type": "object", + "title": "payload for POST /service//template", + "properties": { + "name": {"type": "string"}, + "template_type": {"enum": TEMPLATE_TYPES}, + "service": uuid, + "process_type": {"emun": TEMPLATE_PROCESS_TYPE}, + "content": {"type": "string"}, + "subject": {"type": "string"}, + "created_by": uuid, + "parent_folder_id": uuid + }, + "if": { + "properties": { + "template_type": {"enum": ["email", "letter"]} + } + }, + "then": {"required": ["subject"]}, + "required": ["name", "template_type", "content", "service", "created_by"] +} diff --git a/requirements-app.txt b/requirements-app.txt index e75740910..99535815d 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -13,7 +13,7 @@ click-datetime==0.2 eventlet==0.23.0 gunicorn==19.7.1 iso8601==0.1.12 -jsonschema==2.6.0 +jsonschema==3.0.0a3 marshmallow-sqlalchemy==0.14.1 marshmallow==2.16.0 psycopg2-binary==2.7.5 diff --git a/requirements.txt b/requirements.txt index 37613bb91..7388d5cee 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,7 @@ click-datetime==0.2 eventlet==0.23.0 gunicorn==19.7.1 iso8601==0.1.12 -jsonschema==2.6.0 +jsonschema==3.0.0a3 marshmallow-sqlalchemy==0.14.1 marshmallow==2.16.0 psycopg2-binary==2.7.5 diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 50c5d7f37..7cf46aa80 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -2,6 +2,7 @@ import base64 import json import random import string +import uuid from datetime import datetime, timedelta import botocore @@ -12,7 +13,13 @@ from freezegun import freeze_time from notifications_utils import SMS_CHAR_COUNT_LIMIT -from app.models import Template, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, TemplateHistory +from app.models import ( + EMAIL_TYPE, + LETTER_TYPE, + SMS_TYPE, + Template, + TemplateHistory +) from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template from tests import create_authorization_header @@ -21,7 +28,10 @@ from tests.app.conftest import ( sample_template_without_email_permission, sample_template_without_letter_permission, sample_template_without_sms_permission) -from tests.app.db import create_service, create_letter_contact, create_template, create_notification +from tests.app.db import ( + create_service, create_letter_contact, create_template, create_notification, + create_template_folder, +) from tests.conftest import set_config_values @@ -71,6 +81,81 @@ def test_should_create_a_new_template_for_a_service( assert sorted(json_resp['data']) == sorted(template_schema.dump(template).data) +def test_should_create_a_new_template_for_a_service_adds_folder_relationship( + client, sample_service +): + parent_folder = create_template_folder(service=sample_service, name='parent folder') + + data = { + 'name': 'my template', + 'template_type': 'sms', + 'content': 'template content', + 'service': str(sample_service.id), + 'created_by': str(sample_service.users[0].id), + 'parent_folder_id': str(parent_folder.id) + } + data = json.dumps(data) + auth_header = create_authorization_header() + + response = client.post( + '/service/{}/template'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=data + ) + assert response.status_code == 201 + template = Template.query.filter(Template.name == 'my template').first() + assert template.folder == parent_folder + + +def test_create_template_should_return_400_if_folder_is_for_a_different_service( + client, sample_service +): + service2 = create_service(service_name='second service') + parent_folder = create_template_folder(service=service2) + + data = { + 'name': 'my template', + 'template_type': 'sms', + 'content': 'template content', + 'service': str(sample_service.id), + 'created_by': str(sample_service.users[0].id), + 'parent_folder_id': str(parent_folder.id) + } + data = json.dumps(data) + auth_header = create_authorization_header() + + response = client.post( + '/service/{}/template'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=data + ) + assert response.status_code == 400 + assert json.loads(response.get_data(as_text=True))['message'] == 'parent_folder_id not found' + + +def test_create_template_should_return_400_if_folder_does_not_exist( + client, sample_service +): + data = { + 'name': 'my template', + 'template_type': 'sms', + 'content': 'template content', + 'service': str(sample_service.id), + 'created_by': str(sample_service.users[0].id), + 'parent_folder_id': str(uuid.uuid4()) + } + data = json.dumps(data) + auth_header = create_authorization_header() + + response = client.post( + '/service/{}/template'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=data + ) + assert response.status_code == 400 + assert json.loads(response.get_data(as_text=True))['message'] == 'parent_folder_id not found' + + def test_should_raise_error_if_service_does_not_exist_on_create(client, sample_user, fake_uuid): data = { 'name': 'my template', @@ -172,7 +257,8 @@ def test_should_error_if_created_by_missing(client, sample_user, sample_service) ) json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 - assert json_resp['result'] == 'error' + assert json_resp["errors"][0]["error"] == 'ValidationError' + assert json_resp["errors"][0]["message"] == 'created_by is a required property' def test_should_be_error_if_service_does_not_exist_on_update(client, fake_uuid): @@ -211,9 +297,8 @@ def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, data=data ) json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 400 - assert json_resp['result'] == 'error' - assert json_resp['message'] == {'subject': ['Invalid template subject']} + assert json_resp['errors'][0]['error'] == "ValidationError" + assert json_resp['errors'][0]["message"] == 'subject is a required property' def test_update_should_update_a_template(client, sample_user, sample_template):