diff --git a/app/service/rest.py b/app/service/rest.py index 3de1f7c4c..60141e87e 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -93,6 +93,7 @@ from app.models import ( from app.notifications.process_notifications import persist_notification, send_notification_to_queue from app.schema_validation import validate from app.service import statistics +from app.service.send_pdf_letter_schema import send_pdf_letter_request from app.service.service_data_retention_schema import ( add_service_data_retention_request, update_service_data_retention_request @@ -650,7 +651,8 @@ def create_one_off_notification(service_id): @service_blueprint.route('//send-pdf-letter', methods=['POST']) def create_pdf_letter(service_id): - resp = send_pdf_letter_notification(service_id, request.get_json()) + data = validate(request.get_json(), send_pdf_letter_request) + resp = send_pdf_letter_notification(service_id, data) return jsonify(resp), 201 diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 469cf473b..883c659dc 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -23,7 +23,6 @@ from app.models import ( SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, - POSTAGE_TYPES, NOTIFICATION_DELIVERED, UPLOAD_LETTERS, ) @@ -149,11 +148,6 @@ def send_pdf_letter_notification(service_id, post_data): allow_whitelisted_recipients=False, ) - postage = post_data.get('postage') - if postage not in POSTAGE_TYPES: - message = "postage must be set as 'first' or 'second'" - raise BadRequestError(message=message) - template = get_precompiled_letter_template(service.id) file_location = 'service-{}/{}.pdf'.format(service.id, post_data['file_id']) @@ -188,7 +182,7 @@ def send_pdf_letter_notification(service_id, post_data): client_reference=post_data['filename'], created_by_id=post_data['created_by'], billable_units=billable_units, - postage=postage, + postage=post_data['postage'], ) upload_filename = get_letter_pdf_filename( diff --git a/app/service/send_pdf_letter_schema.py b/app/service/send_pdf_letter_schema.py new file mode 100644 index 000000000..be9c26167 --- /dev/null +++ b/app/service/send_pdf_letter_schema.py @@ -0,0 +1,14 @@ +send_pdf_letter_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST send uploaded pdf letter", + "type": "object", + "title": "Send an uploaded pdf letter", + "properties": { + "postage": {"enum": ["first", "second"]}, + "filename": {"type": "string"}, + "created_by": {"type": "string"}, + "file_id": {"type": "string"}, + }, + + "required": ["postage", "filename", "created_by", "file_id"] +} diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 080d989d0..073d01a42 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2297,6 +2297,23 @@ def test_create_pdf_letter(mocker, sample_service_full_permissions, client, fake assert json_resp == {'id': fake_uuid} +def test_create_pdf_letter_validates_against_json_schema(sample_service_full_permissions, client): + response = client.post( + url_for('service.create_pdf_letter', service_id=sample_service_full_permissions.id), + data=json.dumps({}), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + json_resp = json.loads(response.get_data(as_text=True)) + + assert response.status_code == 400 + assert json_resp['errors'] == [ + {'error': 'ValidationError', 'message': 'postage is a required property'}, + {'error': 'ValidationError', 'message': 'filename is a required property'}, + {'error': 'ValidationError', 'message': 'created_by is a required property'}, + {'error': 'ValidationError', 'message': 'file_id is a required property'} + ] + + def test_get_notification_for_service_includes_template_redacted(admin_request, sample_notification): resp = admin_request.get( 'service.get_notification_for_service', diff --git a/tests/app/service/test_send_pdf_letter_notification.py b/tests/app/service/test_send_pdf_letter_notification.py index e1fe8ef94..7b4c5bd49 100644 --- a/tests/app/service/test_send_pdf_letter_notification.py +++ b/tests/app/service/test_send_pdf_letter_notification.py @@ -51,26 +51,6 @@ def test_send_pdf_letter_notification_validates_created_by( send_pdf_letter_notification(sample_service_full_permissions.id, post_data) -def test_send_pdf_letter_notification_validates_postage( - sample_service_full_permissions, fake_uuid, notify_user -): - user = sample_service_full_permissions.users[0] - post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid, 'postage': 'third'} - - with pytest.raises(BadRequestError): - send_pdf_letter_notification(sample_service_full_permissions.id, post_data) - - -def test_send_pdf_letter_notification_requires_postage( - sample_service_full_permissions, fake_uuid, notify_user -): - user = sample_service_full_permissions.users[0] - post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid} - - with pytest.raises(BadRequestError): - send_pdf_letter_notification(sample_service_full_permissions.id, post_data) - - def test_send_pdf_letter_notification_raises_error_if_service_in_trial_mode( mocker, sample_service_full_permissions,