diff --git a/app/v2/inbound_sms/get_inbound_sms.py b/app/v2/inbound_sms/get_inbound_sms.py index 563491a55..09a8c429f 100644 --- a/app/v2/inbound_sms/get_inbound_sms.py +++ b/app/v2/inbound_sms/get_inbound_sms.py @@ -1,35 +1,33 @@ -import uuid - from flask import jsonify, request, url_for, current_app -from sqlalchemy.orm.exc import NoResultFound -from werkzeug.exceptions import abort from notifications_utils.recipients import validate_and_format_phone_number from notifications_utils.recipients import InvalidPhoneError from app import authenticated_service from app.dao import inbound_sms_dao +from app.schema_validation import validate from app.v2.errors import BadRequestError from app.v2.inbound_sms import v2_inbound_sms_blueprint +from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_request -@v2_inbound_sms_blueprint.route("/", methods=['GET']) -def get_inbound_sms_by_number(user_number): - _data = request.args.to_dict(flat=False) +@v2_inbound_sms_blueprint.route("", methods=['GET']) +def get_inbound_sms(): + data = validate(request.args.to_dict(), get_inbound_sms_request) - # flat=False makes everything a list, but we only ever allow one value for "older_than" - if 'older_than' in _data: - _data['older_than'] = _data['older_than'][0] + if data.get('user_number'): + try: + data['user_number'] = validate_and_format_phone_number(data.get('user_number')) + except InvalidPhoneError as e: + raise BadRequestError(message=str(e)) - try: - user_number = validate_and_format_phone_number(user_number) - except InvalidPhoneError as e: - raise BadRequestError(message=str(e)) + user_number = data.get('user_number', None) + older_than = data.get('older_than', None) paginated_inbound_sms = inbound_sms_dao.dao_get_paginated_inbound_sms_for_service( authenticated_service.id, user_number=user_number, - older_than=_data.get('older_than'), + older_than=older_than, page_size=current_app.config.get('API_PAGE_SIZE') ) @@ -37,36 +35,14 @@ def get_inbound_sms_by_number(user_number): received_text_messages=[i.serialize() for i in paginated_inbound_sms], links=_build_links( paginated_inbound_sms, - endpoint='get_inbound_sms_by_number', - user_number=user_number - ) + user_number=user_number) ), 200 -@v2_inbound_sms_blueprint.route("", methods=['GET']) -def get_all_inbound_sms(): - _data = request.args.to_dict(flat=False) - - # flat=False makes everything a list, but we only ever allow one value for "older_than" - if 'older_than' in _data: - _data['older_than'] = _data['older_than'][0] - - paginated_inbound_sms = inbound_sms_dao.dao_get_paginated_inbound_sms_for_service( - authenticated_service.id, - older_than=_data.get('older_than'), - page_size=current_app.config.get('API_PAGE_SIZE') - ) - - return jsonify( - received_text_messages=[i.serialize() for i in paginated_inbound_sms], - links=_build_links(paginated_inbound_sms, endpoint='get_all_inbound_sms') - ), 200 - - -def _build_links(inbound_sms_list, endpoint, user_number=None): +def _build_links(inbound_sms_list, user_number=None): _links = { 'current': url_for( - "v2_inbound_sms.{}".format(endpoint), + "v2_inbound_sms.get_inbound_sms", user_number=user_number, _external=True, ), @@ -74,7 +50,7 @@ def _build_links(inbound_sms_list, endpoint, user_number=None): if inbound_sms_list: _links['next'] = url_for( - "v2_inbound_sms.{}".format(endpoint), + "v2_inbound_sms.get_inbound_sms", user_number=user_number, older_than=inbound_sms_list[-1].id, _external=True, diff --git a/app/v2/inbound_sms/inbound_sms_schemas.py b/app/v2/inbound_sms/inbound_sms_schemas.py index d21c2b27b..1f257cea7 100644 --- a/app/v2/inbound_sms/inbound_sms_schemas.py +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -1,6 +1,18 @@ from app.schema_validation.definitions import uuid +get_inbound_sms_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "schema for query parameters allowed when getting list of received text messages", + "type": "object", + "properties": { + "older_than": uuid, + "user_number": {"type": "string"} + }, + "additionalProperties": False, +} + + get_inbound_sms_single_response = { "$schema": "http://json-schema.org/draft-04/schema#", "description": "GET inbound sms schema response", @@ -29,6 +41,7 @@ get_inbound_sms_single_response = { "user_number", "created_at", "service_id", "notify_number", "content" ], + "additionalProperties": False, } get_inbound_sms_response = { @@ -60,5 +73,6 @@ get_inbound_sms_response = { "required": ["received_text_messages", "links"], "definitions": { "inbound_sms": get_inbound_sms_single_response - } + }, + "additionalProperties": False, } diff --git a/tests/app/v2/inbound_sms/test_get_inbound_sms.py b/tests/app/v2/inbound_sms/test_get_inbound_sms.py index a45a01928..044bc5d64 100644 --- a/tests/app/v2/inbound_sms/test_get_inbound_sms.py +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -1,8 +1,6 @@ -import datetime import pytest from flask import json, url_for -from app import DATETIME_FORMAT from tests import create_authorization_header from tests.app.db import create_inbound_sms @@ -34,12 +32,9 @@ def test_get_all_inbound_sms_returns_200( assert json_response == expected_response -@pytest.mark.parametrize('inbound_sms_path,user_number', [ - ('v2_inbound_sms.get_all_inbound_sms', None), - ('v2_inbound_sms.get_inbound_sms_by_number', '447700900111') -]) +@pytest.mark.parametrize('user_number', [None, '447700900111']) def test_get_inbound_sms_generate_page_links( - client, sample_service, mocker, inbound_sms_path, user_number + client, sample_service, mocker, user_number ): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", @@ -55,7 +50,7 @@ def test_get_inbound_sms_generate_page_links( auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path=url_for(inbound_sms_path, user_number=user_number), + url_for('v2_inbound_sms.get_inbound_sms', user_number=user_number), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -65,22 +60,19 @@ def test_get_inbound_sms_generate_page_links( assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( - inbound_sms_path, + 'v2_inbound_sms.get_inbound_sms', user_number=user_number, _external=True) == json_response['links']['current'] assert url_for( - inbound_sms_path, + 'v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=reversed_inbound_sms[1].id, _external=True) == json_response['links']['next'] -@pytest.mark.parametrize('inbound_sms_path,user_number', [ - ('v2_inbound_sms.get_all_inbound_sms', None), - ('v2_inbound_sms.get_inbound_sms_by_number', '447700900111') -]) +@pytest.mark.parametrize('user_number', [None, '447700900111']) def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( - client, sample_service, mocker, inbound_sms_path, user_number + client, sample_service, mocker, user_number ): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", @@ -96,7 +88,7 @@ def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path=url_for(inbound_sms_path, user_number=user_number, older_than=reversed_inbound_sms[1].id), + path=url_for('v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=reversed_inbound_sms[1].id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -106,22 +98,19 @@ def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( - inbound_sms_path, + 'v2_inbound_sms.get_inbound_sms', user_number=user_number, _external=True) == json_response['links']['current'] assert url_for( - inbound_sms_path, + 'v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=reversed_inbound_sms[3].id, _external=True) == json_response['links']['next'] -@pytest.mark.parametrize('inbound_sms_path,user_number', [ - ('v2_inbound_sms.get_all_inbound_sms', None), - ('v2_inbound_sms.get_inbound_sms_by_number', '447700900111') -]) +@pytest.mark.parametrize('user_number', [None, '447700900111']) def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( - client, sample_inbound_sms, mocker, inbound_sms_path, user_number + client, sample_inbound_sms, mocker, user_number ): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", @@ -130,7 +119,7 @@ def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( auth_header = create_authorization_header(service_id=sample_inbound_sms.service.id) response = client.get( - path=url_for(inbound_sms_path, user_number=user_number, older_than=sample_inbound_sms.id), + path=url_for('v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=sample_inbound_sms.id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -139,7 +128,7 @@ def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( expected_inbound_sms_list = [] assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( - inbound_sms_path, + 'v2_inbound_sms.get_inbound_sms', user_number=user_number, _external=True) == json_response['links']['current'] assert 'next' not in json_response['links'].keys() @@ -178,7 +167,7 @@ def test_get_inbound_sms_by_number_returns_200( auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/received-text-messages/{}'.format(requested_number), + path='/v2/received-text-messages?user_number={}'.format(requested_number), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -209,10 +198,27 @@ def test_get_inbound_sms_for_no_inbound_sms_returns_200( assert json_response == expected_response +def test_get_inbound_sms_with_invalid_query_string_returns_400(client, sample_service): + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.get( + path='/v2/received-text-messages?usernumber=447700900000', + headers=[('Content-Type', 'application/json'), auth_header]) + + 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['status_code'] == 400 + assert json_response['errors'][0]['error'] == 'ValidationError' + assert json_response['errors'][0]['message'] == \ + 'Additional properties are not allowed (usernumber was unexpected)' + + def test_get_inbound_sms_by_nonexistent_number(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/received-text-messages/447700900000', + path='/v2/received-text-messages?user_number=447700900000', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -233,7 +239,7 @@ def test_get_inbound_sms_by_invalid_number( client, sample_service, invalid_number, expected_message): auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/received-text-messages/{}'.format(invalid_number), + path='/v2/received-text-messages?user_number={}'.format(invalid_number), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 400 diff --git a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py index 47db068ea..c445de4a6 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -6,6 +6,7 @@ from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_response, get from app.schema_validation import validate from tests import create_authorization_header +from tests.app.db import create_inbound_sms valid_inbound_sms = { @@ -44,7 +45,7 @@ invalid_inbound_sms_list = { def test_get_inbound_sms_contract(client, sample_inbound_sms): auth_header = create_authorization_header(service_id=sample_inbound_sms.service_id) - response = client.get('/v2/received-text-messages/{}'.format(sample_inbound_sms.user_number), headers=[auth_header]) + response = client.get('/v2/received-text-messages', headers=[auth_header]) response_json = json.loads(response.get_data(as_text=True)) assert validate(response_json, get_inbound_sms_response)['received_text_messages'][0] \