Refactored code to remove user number handling

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,
         )

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,
 }

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)'

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)