From 79e33073c9bf90bf2dbd89ade8a037673bf5920a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 20 Jul 2017 15:23:46 +0100 Subject: [PATCH] raise 404 when unknown url consistent with other endpoints. also refactor of notification_schema to separate some fns to a different file --- app/v2/notifications/create_response.py | 45 ++++++++++++++++++ app/v2/notifications/notification_schemas.py | 46 ------------------- app/v2/notifications/post_notifications.py | 9 ++-- .../test_post_letter_notifications.py | 5 +- .../notifications/test_post_notifications.py | 6 +-- 5 files changed, 55 insertions(+), 56 deletions(-) create mode 100644 app/v2/notifications/create_response.py diff --git a/app/v2/notifications/create_response.py b/app/v2/notifications/create_response.py new file mode 100644 index 000000000..7eecfb1b9 --- /dev/null +++ b/app/v2/notifications/create_response.py @@ -0,0 +1,45 @@ + +def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + 'from_number': from_number, + 'body': content + } + return noti + + +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "from_email": email_from, + "body": content, + "subject": subject + } + return noti + + +def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for): + noti = __create_notification_response(notification, url_root, scheduled_for) + noti['content'] = { + "body": content, + "subject": subject + } + return noti + + +def __create_notification_response(notification, url_root, scheduled_for): + return { + "id": notification.id, + "reference": notification.client_reference, + "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), + 'template': { + "id": notification.template_id, + "version": notification.template_version, + "uri": "{}services/{}/templates/{}".format( + url_root, + str(notification.service_id), + str(notification.template_id) + ) + }, + "scheduled_for": scheduled_for if scheduled_for else None + } diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index fbd8fba53..7f61f0ee0 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -232,49 +232,3 @@ post_letter_response = { }, "required": ["id", "content", "uri", "template"] } - - -def create_post_sms_response_from_notification(notification, content, from_number, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - 'from_number': from_number, - 'body': content - } - return noti - - -def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - "from_email": email_from, - "body": content, - "subject": subject - } - return noti - - -def create_post_letter_response_from_notification(notification, content, subject, url_root, scheduled_for): - noti = __create_notification_response(notification, url_root, scheduled_for) - noti['content'] = { - "body": content, - "subject": subject - } - return noti - - -def __create_notification_response(notification, url_root, scheduled_for): - return { - "id": notification.id, - "reference": notification.client_reference, - "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), - 'template': { - "id": notification.template_id, - "version": notification.template_version, - "uri": "{}services/{}/templates/{}".format( - url_root, - str(notification.service_id), - str(notification.template_id) - ) - }, - "scheduled_for": scheduled_for if scheduled_for else None - } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ea02f3c7f..f39f54345 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -1,6 +1,6 @@ import functools -from flask import request, jsonify, current_app +from flask import request, jsonify, current_app, abort from app import api_user, authenticated_service from app.config import QueueNames @@ -18,12 +18,13 @@ from app.notifications.validators import ( validate_template ) from app.schema_validation import validate -from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import ( post_sms_request, post_email_request, - post_letter_request, + post_letter_request +) +from app.v2.notifications.create_response import ( create_post_sms_response_from_notification, create_post_email_response_from_notification, create_post_letter_response_from_notification @@ -39,7 +40,7 @@ def post_notification(notification_type): elif notification_type == LETTER_TYPE: form = validate(request.get_json(), post_letter_request) else: - raise BadRequestError(message='Unknown notification type {}'.format(notification_type)) + abort(404) check_service_has_permission(notification_type, authenticated_service.permissions) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 7c8f2b34d..d65c6c5d8 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -1,3 +1,4 @@ + import uuid from flask import url_for, json @@ -145,7 +146,7 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( {'service_permissions': [EMAIL_TYPE, SMS_TYPE]}, {'restricted': True} ]) -def test_post_letter_notification_returns_400_if_not_allowed_to_send_notification( +def test_post_letter_notification_returns_403_if_not_allowed_to_send_notification( client, notify_db_session, service_args @@ -159,7 +160,7 @@ def test_post_letter_notification_returns_400_if_not_allowed_to_send_notificatio } error_json = letter_request(client, data, service_id=service.id, _expected_status=400) - assert error_json['status_code'] == 400 + assert error_json['status_code'] == 403 assert error_json['errors'] == [ {'error': 'BadRequestError', 'message': 'Cannot send letters'} ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 597cdf1b7..40475e030 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -442,8 +442,6 @@ def test_post_notification_raises_bad_request_if_not_valid_notification_type(cli data='{}', headers=[('Content-Type', 'application/json'), auth_header] ) - assert response.status_code == 400 + assert response.status_code == 404 error_json = json.loads(response.get_data(as_text=True)) - assert error_json['errors'] == [ - {'error': 'BadRequestError', 'message': 'Unknown notification type foo'} - ] + assert 'The requested URL was not found on the server.' in error_json['message']