From 1b0b16fa74a9de519dda908682e1b421f5721851 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 7 Nov 2018 12:48:56 +0000 Subject: [PATCH] Update model and controller to handle parent_folder_id when creating a template. If the parent_folder_id then check if the folder exists and is for the same service. If it is add the folder to the template model object, the relationship will be persisted when the template is saved. If the folder does not exist or is for a different service, then return a ResultNotFound error. --- app/models.py | 4 +--- app/template/rest.py | 13 +++++++++---- tests/app/template/test_rest.py | 31 ++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/app/models.py b/app/models.py index f5bd68eae..b9b460bfd 100644 --- a/app/models.py +++ b/app/models.py @@ -888,15 +888,13 @@ class Template(TemplateBase): 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') - if fields.pop("parent_folder_id"): - fields['folder'] = folder + fields['folder'] = folder return cls(**fields) diff --git a/app/template/rest.py b/app/template/rest.py index 2084bb674..7a8606b45 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -48,8 +48,14 @@ def _content_count_greater_than_limit(content, template_type): return template.content_count > SMS_CHAR_COUNT_LIMIT -def validate_parent_folder(parent_folder_id, service_id): - return TemplateFolder.query.filter_by(service_id=service_id, id=parent_folder_id).one() +def validate_parent_folder(template_json): + if template_json.get("parent_folder_id"): + return TemplateFolder.query.filter_by( + service_id=template_json['service'], + id=template_json.pop("parent_folder_id") + ).one() + else: + return None @template_blueprint.route('', methods=['POST']) @@ -58,8 +64,7 @@ def create_template(service_id): # permissions needs to be placed here otherwise marshmallow will interfere with versioning permissions = fetched_service.permissions template_json = validate(request.get_json(), post_create_template_schema) - folder = validate_parent_folder(parent_folder_id=template_json['parent_folder_id'], - service_id=template_json['service']) + 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): diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index c92545af9..6cd9950b0 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, TemplateFolder +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 @@ -125,6 +132,28 @@ def test_create_template_should_return_404_if_folder_is_for_a_different_service( assert response.status_code == 404 +def test_create_template_should_return_404_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 == 404 + + def test_should_raise_error_if_service_does_not_exist_on_create(client, sample_user, fake_uuid): data = { 'name': 'my template',