From 874ca5a8affdaf17eaef9f93f5a4799decc090fe Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 8 Nov 2017 10:57:16 +0000 Subject: [PATCH] Refactored code to remove user number handling --- app/v2/inbound_sms/get_inbound_sms.py | 21 +-- app/v2/inbound_sms/inbound_sms_schemas.py | 1 - .../v2/inbound_sms/test_get_inbound_sms.py | 123 ++---------------- .../inbound_sms/test_inbound_sms_schemas.py | 48 +++++-- 4 files changed, 50 insertions(+), 143 deletions(-) diff --git a/app/v2/inbound_sms/get_inbound_sms.py b/app/v2/inbound_sms/get_inbound_sms.py index 09a8c429f..478e4f03d 100644 --- a/app/v2/inbound_sms/get_inbound_sms.py +++ b/app/v2/inbound_sms/get_inbound_sms.py @@ -6,7 +6,6 @@ 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 @@ -15,35 +14,22 @@ from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_request def get_inbound_sms(): data = validate(request.args.to_dict(), get_inbound_sms_request) - 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)) - - 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=older_than, + older_than=data.get('older_than', None), 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, - user_number=user_number) + links=_build_links(paginated_inbound_sms) ), 200 -def _build_links(inbound_sms_list, user_number=None): +def _build_links(inbound_sms_list): _links = { 'current': url_for( "v2_inbound_sms.get_inbound_sms", - user_number=user_number, _external=True, ), } @@ -51,7 +37,6 @@ def _build_links(inbound_sms_list, user_number=None): if inbound_sms_list: _links['next'] = url_for( "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 1f257cea7..e6ef8cc98 100644 --- a/app/v2/inbound_sms/inbound_sms_schemas.py +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -7,7 +7,6 @@ get_inbound_sms_request = { "type": "object", "properties": { "older_than": uuid, - "user_number": {"type": "string"} }, "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 044bc5d64..e499089c3 100644 --- a/tests/app/v2/inbound_sms/test_get_inbound_sms.py +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -1,11 +1,10 @@ -import pytest from flask import json, url_for from tests import create_authorization_header from tests.app.db import create_inbound_sms -def test_get_all_inbound_sms_returns_200( +def test_get_inbound_sms_returns_200( client, sample_service ): all_inbound_sms = [ @@ -32,10 +31,7 @@ def test_get_all_inbound_sms_returns_200( assert json_response == expected_response -@pytest.mark.parametrize('user_number', [None, '447700900111']) -def test_get_inbound_sms_generate_page_links( - client, sample_service, mocker, user_number -): +def test_get_inbound_sms_generate_page_links(client, sample_service, mocker): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", {"API_PAGE_SIZE": 2} @@ -50,7 +46,7 @@ def test_get_inbound_sms_generate_page_links( auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - url_for('v2_inbound_sms.get_inbound_sms', user_number=user_number), + url_for('v2_inbound_sms.get_inbound_sms'), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -61,19 +57,14 @@ def test_get_inbound_sms_generate_page_links( assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( 'v2_inbound_sms.get_inbound_sms', - user_number=user_number, _external=True) == json_response['links']['current'] assert url_for( '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('user_number', [None, '447700900111']) -def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( - client, sample_service, mocker, user_number -): +def test_get_next_inbound_sms_will_get_correct_inbound_sms_list(client, sample_service, mocker): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", {"API_PAGE_SIZE": 2} @@ -88,7 +79,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('v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=reversed_inbound_sms[1].id), + path=url_for('v2_inbound_sms.get_inbound_sms', older_than=reversed_inbound_sms[1].id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -99,19 +90,14 @@ 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( 'v2_inbound_sms.get_inbound_sms', - user_number=user_number, _external=True) == json_response['links']['current'] assert url_for( '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('user_number', [None, '447700900111']) -def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( - client, sample_inbound_sms, mocker, user_number -): +def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list(client, sample_inbound_sms, mocker): mocker.patch.dict( "app.v2.inbound_sms.get_inbound_sms.current_app.config", {"API_PAGE_SIZE": 1} @@ -119,7 +105,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('v2_inbound_sms.get_inbound_sms', user_number=user_number, older_than=sample_inbound_sms.id), + path=url_for('v2_inbound_sms.get_inbound_sms', older_than=sample_inbound_sms.id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -129,58 +115,11 @@ def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( 'v2_inbound_sms.get_inbound_sms', - user_number=user_number, _external=True) == json_response['links']['current'] assert 'next' not in json_response['links'].keys() -def test_get_all_inbound_sms_for_no_inbound_sms_returns_200( - client, sample_service -): - auth_header = create_authorization_header(service_id=sample_service.id) - response = client.get( - path='/v2/received-text-messages', - headers=[('Content-Type', 'application/json'), auth_header]) - - assert response.status_code == 200 - assert response.headers['Content-type'] == 'application/json' - - json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] - - expected_response = [] - - assert json_response == expected_response - - -@pytest.mark.parametrize('requested_number', [ - '447700900111', - '+447700900111', - '07700900111' -]) -def test_get_inbound_sms_by_number_returns_200( - client, sample_service, requested_number -): - sample_inbound_sms1 = create_inbound_sms(service=sample_service, user_number='447700900111') - create_inbound_sms(service=sample_service, user_number='447700900112') - sample_inbound_sms2 = create_inbound_sms(service=sample_service, user_number='447700900111') - create_inbound_sms(service=sample_service, user_number='447700900113') - - auth_header = create_authorization_header(service_id=sample_service.id) - response = client.get( - path='/v2/received-text-messages?user_number={}'.format(requested_number), - headers=[('Content-Type', 'application/json'), auth_header]) - - assert response.status_code == 200 - assert response.headers['Content-type'] == 'application/json' - - json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] - - expected_response = [sample_inbound_sms2.serialize(), sample_inbound_sms1.serialize()] - - assert json_response == expected_response - - -def test_get_inbound_sms_for_no_inbound_sms_returns_200( +def test_get_inbound_sms_for_no_inbound_sms_returns_empty_list( client, sample_service ): auth_header = create_authorization_header(service_id=sample_service.id) @@ -201,7 +140,7 @@ def test_get_inbound_sms_for_no_inbound_sms_returns_200( 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', + path='/v2/received-text-messages?user_number=447700900000', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 400 @@ -212,46 +151,4 @@ def test_get_inbound_sms_with_invalid_query_string_returns_400(client, sample_se 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?user_number=447700900000', - headers=[('Content-Type', 'application/json'), auth_header]) - - assert response.status_code == 200 - assert response.headers['Content-type'] == 'application/json' - - json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] - expected_response = [] - - assert json_response == expected_response - - -@pytest.mark.parametrize('invalid_number,expected_message', [ - ('0077700', 'Not enough digits'), - ('123456789012', 'Not a UK mobile number'), - ('invalid_number', 'Must not contain letters or symbols') -]) -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?user_number={}'.format(invalid_number), - 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 == { - "errors": [ - { - "error": "BadRequestError", - "message": expected_message - } - ], - "status_code": 400 - } + 'Additional properties are not allowed (user_number was unexpected)' 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 c445de4a6..fe9613056 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -1,14 +1,17 @@ import pytest -from flask import json +from flask import json, url_for from jsonschema.exceptions import ValidationError -from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_response, get_inbound_sms_single_response +from app.v2.inbound_sms.inbound_sms_schemas import ( + get_inbound_sms_request, + get_inbound_sms_response, + get_inbound_sms_single_response +) from app.schema_validation import validate from tests import create_authorization_header from tests.app.db import create_inbound_sms - valid_inbound_sms = { "provider_date": "2017-11-02T15:07:57.199541Z", "provider_reference": "foo", @@ -43,28 +46,51 @@ 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) +def test_get_inbound_sms_contract(client, sample_service): + all_inbound_sms = [ + create_inbound_sms(service=sample_service, user_number='447700900113'), + create_inbound_sms(service=sample_service, user_number='447700900112'), + create_inbound_sms(service=sample_service, user_number='447700900111'), + ] + reversed_inbound_sms = sorted(all_inbound_sms, key=lambda sms: sms.created_at, reverse=True) + + auth_header = create_authorization_header(service_id=all_inbound_sms[0].service_id) 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] \ - == sample_inbound_sms.serialize() + validated_resp = validate(response_json, get_inbound_sms_response) + assert validated_resp['received_text_messages'] == [i.serialize() for i in reversed_inbound_sms] + assert validated_resp['links']['current'] == url_for( + 'v2_inbound_sms.get_inbound_sms', _external=True) + assert validated_resp['links']['next'] == url_for( + 'v2_inbound_sms.get_inbound_sms', older_than=all_inbound_sms[0].id, _external=True) -def test_valid_inbound_sms_json(): +@pytest.mark.parametrize('request_args', [ + {'older_than': "6ce466d0-fd6a-11e5-82f5-e0accb9d11a6"}, {}] +) +def test_valid_inbound_sms_request_json(client, request_args): + validate(request_args, get_inbound_sms_request) + + +def test_invalid_inbound_sms_request_json(client): + with pytest.raises(expected_exception=ValidationError): + validate({'user_number': '447700900111'}, get_inbound_sms_request) + + +def test_valid_inbound_sms_response_json(): assert validate(valid_inbound_sms, get_inbound_sms_single_response) == valid_inbound_sms -def test_valid_inbound_sms_list_json(): +def test_valid_inbound_sms_list_response_json(): validate(valid_inbound_sms_list, get_inbound_sms_response) -def test_invalid_inbound_sms_json(): +def test_invalid_inbound_sms_response_json(): with pytest.raises(expected_exception=ValidationError): validate(invalid_inbound_sms, get_inbound_sms_single_response) -def test_invalid_inbound_sms_list_json(): +def test_invalid_inbound_sms_list_response_json(): with pytest.raises(expected_exception=ValidationError): validate(invalid_inbound_sms_list, get_inbound_sms_response)