From ecc4cde4dee3ad856e30ccef0b308ce04e9c6a06 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 3 Apr 2018 16:37:41 +0100 Subject: [PATCH] Update the get_notification_by_id to return a sensible message if the id is not a valid UUID. Previously "Result not found" would be returned when the id is not a valid uuid, which does not make sense. Now the message says "notification_id is not a valid UUID", this should be a clearer message for the client service. --- app/v2/notifications/get_notifications.py | 18 ++++++------------ app/v2/notifications/notification_schemas.py | 12 ++++++++++++ .../v2/notifications/test_get_notifications.py | 14 +++++++------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 14adb9028..d54df58cb 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -1,23 +1,17 @@ -import uuid - from flask import jsonify, request, url_for, current_app -from werkzeug.exceptions import abort - from app import api_user, authenticated_service from app.dao import notifications_dao from app.schema_validation import validate from app.v2.notifications import v2_notification_blueprint -from app.v2.notifications.notification_schemas import get_notifications_request +from app.v2.notifications.notification_schemas import get_notifications_request, notification_by_id -@v2_notification_blueprint.route("/", methods=['GET']) -def get_notification_by_id(id): - try: - casted_id = uuid.UUID(id) - except (ValueError, AttributeError): - abort(404) +@v2_notification_blueprint.route("/", methods=['GET']) +def get_notification_by_id(notification_id): + _data = {"notification_id": notification_id} + validate(_data, notification_by_id) notification = notifications_dao.get_notification_with_personalisation( - authenticated_service.id, casted_id, key_type=None + authenticated_service.id, notification_id, key_type=None ) return jsonify(notification.serialize()), 200 diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 506a54706..1e66ab7f6 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -20,6 +20,18 @@ template = { "required": ["id", "version", "uri"] } +notification_by_id = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "GET notification response schema", + "type": "object", + "title": "response v2/notification", + "properties": { + "notification_id": uuid + }, + "required": ["notification_id"] +} + + get_notification_response = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "GET notification response schema", diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index f2da004f7..fcfd22e15 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -210,15 +210,15 @@ def test_get_notification_by_id_invalid_id(client, sample_notification, id): path='/v2/notifications/{}'.format(id), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 404 + assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' json_response = json.loads(response.get_data(as_text=True)) - assert json_response == { - "message": "The requested URL was not found on the server. " - "If you entered the URL manually please check your spelling and try again.", - "result": "error" - } + assert json_response == {"errors": [ + {"error": "ValidationError", + "message": "notification_id is not a valid UUID" + }], + "status_code": 400} @pytest.mark.parametrize('created_at_month, estimated_delivery', [ @@ -628,7 +628,7 @@ def test_get_notifications_renames_letter_statuses(client, sample_letter_templat ) auth_header = create_authorization_header(service_id=letter_noti.service_id) response = client.get( - path=url_for('v2_notifications.get_notification_by_id', id=letter_noti.id), + path=url_for('v2_notifications.get_notification_by_id', notification_id=letter_noti.id), headers=[('Content-Type', 'application/json'), auth_header] )