diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index ee8f27943..fe643ad61 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -42,6 +42,4 @@ def build_error_message(errors): def __format_message(e): - s = e.message.split("'") - msg = "{}{}".format(s[1], s[2]) - return msg if not e.cause else "{} {}".format(e.path[0], e.cause.message) + return e.message.replace("'", "") if not e.cause else "{} {}".format(e.path[0], e.cause.message) diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 274df52d7..5158e563f 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -2,8 +2,9 @@ from flask import jsonify, request, url_for from app import api_user from app.dao import notifications_dao -from app.schemas import notifications_filter_schema +from app.schema_validation import validate from app.v2.notifications import notification_blueprint +from app.v2.notifications.notification_schemas import get_notifications_request @notification_blueprint.route("/", methods=['GET']) @@ -17,7 +18,12 @@ def get_notification_by_id(id): @notification_blueprint.route("", methods=['GET']) def get_notifications(): - data = notifications_filter_schema.load(request.args).data + _data = request.args.to_dict(flat=False) + if 'older_than' in _data: + # flat=False makes everything a list, but we only ever allow one value for "older_than" + _data['older_than'] = _data['older_than'][0] + + data = validate(_data, get_notifications_request) paginated_notifications = notifications_dao.get_notifications_for_service( str(api_user.service_id), @@ -29,11 +35,11 @@ def get_notifications(): def _build_links(notifications): _links = { - 'current': url_for(".get_notifications", _external=True, **request.args.to_dict(flat=False)), + 'current': url_for(".get_notifications", _external=True, **data), } if len(notifications): - next_query_params = dict(request.args.to_dict(flat=False), older_than=notifications[-1].id) + next_query_params = dict(data, older_than=notifications[-1].id) _links['next'] = url_for(".get_notifications", _external=True, **next_query_params) return _links diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 934d6d400..016cb7503 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,3 +1,4 @@ +from app.models import NOTIFICATION_STATUS_TYPES, TEMPLATE_TYPES from app.schema_validation.definitions import (uuid, personalisation) # this may belong in a templates module @@ -72,6 +73,28 @@ get_notification_response = { ] } +get_notifications_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "schema for query parameters allowed when getting list of notifications", + "type": "object", + "properties": { + "status": { + "type": "array", + "items": { + "enum": NOTIFICATION_STATUS_TYPES + } + }, + "template_type": { + "type": "array", + "items": { + "enum": TEMPLATE_TYPES + } + }, + "older_than": uuid + }, + "additionalProperties": False, +} + get_notifications_response = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "GET list of notifications response schema", diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 3c1b93134..adc8e48f3 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -156,6 +156,40 @@ def test_get_all_notifications_filter_by_single_status(client, notify_db, notify assert json_response['notifications'][0]['status'] == "pending" +@pytest.mark.parametrize('invalid_statuses, valid_statuses', [ + # one invalid status + (["elephant"], []), + # multiple invalid statuses + (["elephant", "giraffe", "cheetah"], []), + # one bad status and one good status + (["elephant"], ["created"]), +]) +def test_get_all_notifications_filter_by_status_invalid_status( + client, notify_db, notify_db_session, invalid_statuses, valid_statuses +): + notification = create_sample_notification(notify_db, notify_db_session, status="pending") + create_sample_notification(notify_db, notify_db_session) + + auth_header = create_authorization_header(service_id=notification.service_id) + response = client.get( + path='/v2/notifications?{}'.format( + "&".join(["status={}".format(status) for status in invalid_statuses + valid_statuses]) + ), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_response = json.loads(response.get_data(as_text=True)) + partial_error_message = "is not one of " \ + "[created, sending, delivered, pending, failed, technical-failure, temporary-failure, permanent-failure]" + + assert response.status_code == 400 + assert response.headers['Content-type'] == "application/json" + + assert json_response['status_code'] == 400 + assert len(json_response['errors']) == len(invalid_statuses) + for index, invalid_status in enumerate(invalid_statuses): + assert json_response['errors'][index]['message'] == "{} {}".format(invalid_status, partial_error_message) + + def test_get_all_notifications_filter_by_multiple_statuses(client, notify_db, notify_db_session): notifications = [ create_sample_notification(notify_db, notify_db_session, status=_status)