diff --git a/app/errors.py b/app/errors.py index 7e7790bc8..5a0d4710a 100644 --- a/app/errors.py +++ b/app/errors.py @@ -1,10 +1,11 @@ from flask import ( jsonify, - current_app -) + current_app, + json) from sqlalchemy.exc import SQLAlchemyError, DataError from sqlalchemy.orm.exc import NoResultFound from marshmallow import ValidationError +from jsonschema import ValidationError as JsonSchemaValidationError from app.authentication.auth import AuthError from app.notifications import SendNotificationToQueueError @@ -50,6 +51,11 @@ def register_errors(blueprint): current_app.logger.error(error) return jsonify(result='error', message=error.messages), 400 + @blueprint.errorhandler(JsonSchemaValidationError) + def validation_error(error): + current_app.logger.exception(error) + return jsonify(json.loads(error.message)), 400 + @blueprint.errorhandler(InvalidRequest) def invalid_data(error): response = jsonify(error.to_dict()) diff --git a/app/inbound_sms/inbound_sms_schemas.py b/app/inbound_sms/inbound_sms_schemas.py new file mode 100644 index 000000000..c0e644d68 --- /dev/null +++ b/app/inbound_sms/inbound_sms_schemas.py @@ -0,0 +1,9 @@ +get_inbound_sms_for_service_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "schema for parameters allowed when searching for to field=", + "type": "object", + "properties": { + "phone_number": {"type": "string", "format": "phone_number"}, + "limit": {"type": ["integer", "null"], "minimum": 1} + } +} diff --git a/app/inbound_sms/rest.py b/app/inbound_sms/rest.py index 1eca4b089..a78e3c237 100644 --- a/app/inbound_sms/rest.py +++ b/app/inbound_sms/rest.py @@ -1,8 +1,9 @@ from flask import ( Blueprint, jsonify, - request -) + request, + current_app, json) +from jsonschema import ValidationError from notifications_utils.recipients import validate_and_format_phone_number @@ -12,6 +13,9 @@ from app.dao.inbound_sms_dao import ( dao_get_inbound_sms_by_id ) from app.errors import register_errors +from app.schema_validation import validate + +from app.inbound_sms.inbound_sms_schemas import get_inbound_sms_for_service_schema inbound_sms = Blueprint( 'inbound_sms', @@ -22,18 +26,25 @@ inbound_sms = Blueprint( register_errors(inbound_sms) -@inbound_sms.route('') +@inbound_sms.route('', methods=['POST', 'GET']) def get_inbound_sms_for_service(service_id): - limit = request.args.get('limit') - user_number = request.args.get('user_number') - if user_number: - # we use this to normalise to an international phone number - user_number = validate_and_format_phone_number(user_number, international=True) + if request.method == 'GET': + limit = request.args.get('limit') + user_number = request.args.get('user_number') - results = dao_get_inbound_sms_for_service(service_id, limit, user_number) + if user_number: + # we use this to normalise to an international phone number + user_number = validate_and_format_phone_number(user_number, international=True) - return jsonify(data=[row.serialize() for row in results]) + results = dao_get_inbound_sms_for_service(service_id, limit, user_number) + + return jsonify(data=[row.serialize() for row in results]) + else: + form = validate(request.get_json(), get_inbound_sms_for_service_schema) + results = dao_get_inbound_sms_for_service(service_id, form.get('limit'), form.get('phone_number')) + + return jsonify(data=[row.serialize() for row in results]) @inbound_sms.route('/summary') @@ -49,6 +60,6 @@ def get_inbound_sms_summary_for_service(service_id): @inbound_sms.route('/', methods=['GET']) def get_inbound_by_id(service_id, inbound_sms_id): - inbound_sms = dao_get_inbound_sms_by_id(service_id, inbound_sms_id) + message = dao_get_inbound_sms_by_id(service_id, inbound_sms_id) - return jsonify(inbound_sms.serialize()), 200 + return jsonify(message.serialize()), 200 diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index b4c55cd83..5edc3bbcb 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -1,11 +1,163 @@ from datetime import datetime import pytest +from flask import json from freezegun import freeze_time +from tests import create_authorization_header from tests.app.db import create_inbound_sms, create_service +def test_get_inbound_sms_with_no_params(client, sample_service): + one = create_inbound_sms(sample_service) + two = create_inbound_sms(sample_service) + + auth_header = create_authorization_header() + + data = {} + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + sms = json_resp['data'] + + assert len(sms) == 2 + assert {inbound['id'] for inbound in sms} == {str(one.id), str(two.id)} + assert sms[0]['content'] == 'Hello' + assert set(sms[0].keys()) == { + 'id', + 'created_at', + 'service_id', + 'notify_number', + 'user_number', + 'content', + 'provider_date', + 'provider_reference' + } + + +def test_get_inbound_sms_with_limit(client, sample_service): + with freeze_time('2017-01-01'): + one = create_inbound_sms(sample_service) + with freeze_time('2017-01-02'): + two = create_inbound_sms(sample_service) + + auth_header = create_authorization_header() + + data = {'limit': 1} + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + sms = json_resp['data'] + + assert len(sms) == 1 + assert sms[0]['id'] == str(two.id) + + +def test_get_inbound_sms_should_error_with_invalid_limit(client, sample_service): + + auth_header = create_authorization_header() + + data = {'limit': 'limit'} + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + error_resp = json.loads(response.get_data(as_text=True)) + assert error_resp['status_code'] == 400 + assert error_resp['errors'] == [{ + 'error': 'ValidationError', + 'message': "limit limit is not of type integer, null" + }] + + +def test_get_inbound_sms_should_error_with_invalid_phone_number(client, sample_service): + + auth_header = create_authorization_header() + + data = {'phone_number': 'invalid phone number'} + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + error_resp = json.loads(response.get_data(as_text=True)) + assert error_resp['status_code'] == 400 + assert error_resp['errors'] == [{ + 'error': 'ValidationError', + 'message': "phone_number Must not contain letters or symbols" + }] + + +@pytest.mark.parametrize('user_number', [ + '(07700) 900-001', + '+4407700900001', + '447700900001', +]) +def test_get_inbound_sms_filters_user_number(client, sample_service, user_number): + # user_number in the db is international and normalised + one = create_inbound_sms(sample_service, user_number='447700900001') + two = create_inbound_sms(sample_service, user_number='447700900002') + + auth_header = create_authorization_header() + + data = { + 'limit': 1, + 'phone_number': user_number + } + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + sms = json_resp['data'] + assert len(sms) == 1 + assert sms[0]['id'] == str(one.id) + assert sms[0]['user_number'] == str(one.user_number) + + +def test_get_inbound_sms_filters_international_user_number(admin_request, sample_service): + # user_number in the db is international and normalised + one = create_inbound_sms(sample_service, user_number='12025550104') + two = create_inbound_sms(sample_service) + + auth_header = create_authorization_header() + + data = { + 'limit': 1, + 'phone_number': '+1 (202) 555-0104' + } + + response = client.post( + path='/service/{}/inbound-sms'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + sms = json_resp['data'] + + assert len(sms) == 1 + assert sms[0]['id'] == str(one.id) + assert sms[0]['user_number'] == str(one.user_number) + + +############################################################## +# REMOVE ONCE ADMIN MIGRATED AND GET ENDPOINT REMOVED +############################################################## + + def test_get_inbound_sms(admin_request, sample_service): one = create_inbound_sms(sample_service) two = create_inbound_sms(sample_service) @@ -82,6 +234,10 @@ def test_get_inbound_sms_filters_international_user_number(admin_request, sample assert sms['data'][0]['user_number'] == str(one.user_number) +############################## +# End delete section +############################## + def test_get_inbound_sms_summary(admin_request, sample_service): other_service = create_service(service_name='other_service') with freeze_time('2017-01-01'):