From 0510311d63aaa7feb2ffd78a03b99214b2560c76 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 15 Jan 2021 14:35:23 +0000 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20require=20template=20when=20con?= =?UTF-8?q?tent=20is=20provided?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that the admin app can create broadcasts without a template it needs to be allowed to create broadcasts from content instead. --- .../broadcast_message_schema.py | 7 ++- app/broadcast_message/rest.py | 22 ++++++--- tests/app/broadcast_message/test_rest.py | 45 +++++++++++++++---- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py index 3d617533a..532227f64 100644 --- a/app/broadcast_message/broadcast_message_schema.py +++ b/app/broadcast_message/broadcast_message_schema.py @@ -15,8 +15,13 @@ create_broadcast_message_schema = { 'finishes_at': {'type': 'string', 'format': 'datetime'}, 'areas': {"type": "array", "items": {"type": "string"}}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, + 'content': {'type': 'string', 'minLength': 1, 'maxLength': 1395}, }, - 'required': ['template_id', 'service_id', 'created_by'], + 'required': ['service_id', 'created_by'], + 'oneOf': [ + {'required': ['template_id']}, + {'required': ['content']}, + ], 'additionalProperties': False } diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 1af62e0dd..946741f92 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -99,22 +99,30 @@ def create_broadcast_message(service_id): validate(data, create_broadcast_message_schema) service = dao_fetch_service_by_id(data['service_id']) user = get_user_by_id(data['created_by']) - template = dao_get_template_by_id_and_service_id(data['template_id'], data['service_id']) - personalisation = data.get('personalisation', {}) + template_id = data.get('template_id') + + if template_id: + template = dao_get_template_by_id_and_service_id( + template_id, data['service_id'] + ) + content = template._as_utils_template_with_personalisation( + personalisation + ).content_with_placeholders_filled_in + else: + template, content = None, data['content'] + broadcast_message = BroadcastMessage( service_id=service.id, - template_id=template.id, - template_version=template.version, + template_id=template_id, + template_version=template.version if template else None, personalisation=personalisation, areas={"areas": data.get("areas", []), "simple_polygons": data.get("simple_polygons", [])}, status=BroadcastStatusType.DRAFT, starts_at=_parse_nullable_datetime(data.get('starts_at')), finishes_at=_parse_nullable_datetime(data.get('finishes_at')), created_by_id=user.id, - content=template._as_utils_template_with_personalisation( - personalisation - ).content_with_placeholders_filled_in, + content=content, ) dao_save_object(broadcast_message) diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index f1dfb0d03..df7e7cde2 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -143,9 +143,9 @@ def test_create_broadcast_message(admin_request, sample_broadcast_service): ( {}, [ - {'error': 'ValidationError', 'message': 'template_id is a required property'}, {'error': 'ValidationError', 'message': 'service_id is a required property'}, - {'error': 'ValidationError', 'message': 'created_by is a required property'} + {'error': 'ValidationError', 'message': 'created_by is a required property'}, + {'error': 'ValidationError', 'message': '{} is not valid under any of the given schemas'}, ] ), ( @@ -178,12 +178,30 @@ def test_create_broadcast_message_400s_if_json_schema_fails_validation( @freeze_time('2020-01-01') -def test_create_broadcast_message_400s_if_content_provided_but_no_template(admin_request, sample_broadcast_service): - # we don't currently support this, but might in the future +def test_create_broadcast_message_can_be_created_from_content(admin_request, sample_broadcast_service): response = admin_request.post( 'broadcast_message.create_broadcast_message', _data={ - 'template_id': None, + 'content': 'Some tailor made broadcast content', + 'service_id': str(sample_broadcast_service.id), + 'created_by': str(sample_broadcast_service.created_by_id), + }, + service_id=sample_broadcast_service.id, + _expected_status=201 + ) + assert response['content'] == 'Some tailor made broadcast content' + assert response['template_id'] is None + + +def test_create_broadcast_message_400s_if_content_and_template_provided( + admin_request, + sample_broadcast_service, +): + template = create_template(sample_broadcast_service, BROADCAST_TYPE) + response = admin_request.post( + 'broadcast_message.create_broadcast_message', + _data={ + 'template_id': str(template.id), 'content': 'Some tailor made broadcast content', 'service_id': str(sample_broadcast_service.id), 'created_by': str(sample_broadcast_service.created_by_id), @@ -191,11 +209,20 @@ def test_create_broadcast_message_400s_if_content_provided_but_no_template(admin service_id=sample_broadcast_service.id, _expected_status=400 ) - assert response['errors'] ==[ - {'error': 'ValidationError', 'message': 'template_id is not a valid UUID'}, - {'error': 'ValidationError', 'message': 'Additional properties are not allowed (content was unexpected)'}, - ] + assert len(response['errors']) == 1 + assert response['errors'][0]['error'] == 'ValidationError' + # The error message for oneOf is ugly, non-deterministic in ordering + # and contains some UUID, so let’s just pick out the important bits + assert ( + ' is valid under each of ' + ) in response['errors'][0]['message'] + assert ( + '{required: [content]}' + ) in response['errors'][0]['message'] + assert ( + '{required: [template_id]}' + ) in response['errors'][0]['message'] @pytest.mark.parametrize('status', [ BroadcastStatusType.DRAFT,