From 85b8e24e17d8f71029d191547648dc1df69f4c67 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 3 Nov 2017 16:35:22 +0000 Subject: [PATCH 01/15] Add V2 inbound_sms API - had to update the serialization in the model so that the date time is appended with the UTC timezone - test has been added to ensure that the schema will validate the response correctly --- app/__init__.py | 4 + app/models.py | 4 +- app/v2/inbound_sms/__init__.py | 6 + app/v2/inbound_sms/get_inbound_sms.py | 36 +++++ app/v2/inbound_sms/inbound_sms_schemas.py | 51 +++++++ tests/app/conftest.py | 6 + tests/app/db.py | 6 +- tests/app/v2/inbound_sms/__init__.py | 0 .../v2/inbound_sms/test_get_inbound_sms.py | 131 ++++++++++++++++++ .../inbound_sms/test_inbound_sms_schemas.py | 75 ++++++++++ 10 files changed, 315 insertions(+), 4 deletions(-) create mode 100644 app/v2/inbound_sms/__init__.py create mode 100644 app/v2/inbound_sms/get_inbound_sms.py create mode 100644 app/v2/inbound_sms/inbound_sms_schemas.py create mode 100644 tests/app/v2/inbound_sms/__init__.py create mode 100644 tests/app/v2/inbound_sms/test_get_inbound_sms.py create mode 100644 tests/app/v2/inbound_sms/test_inbound_sms_schemas.py diff --git a/app/__init__.py b/app/__init__.py index 31a601f3c..b93abfe6e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -174,6 +174,7 @@ def register_blueprint(application): def register_v2_blueprints(application): + from app.v2.inbound_sms.get_inbound_sms import v2_inbound_sms_blueprint as get_inbound_sms from app.v2.notifications.post_notifications import v2_notification_blueprint as post_notifications from app.v2.notifications.get_notifications import v2_notification_blueprint as get_notifications from app.v2.template.get_template import v2_template_blueprint as get_template @@ -196,6 +197,9 @@ def register_v2_blueprints(application): post_template.before_request(requires_auth) application.register_blueprint(post_template) + get_inbound_sms.before_request(requires_auth) + application.register_blueprint(get_inbound_sms) + def init_app(app): @app.before_request diff --git a/app/models.py b/app/models.py index 74b0af79a..ea7b93a4e 100644 --- a/app/models.py +++ b/app/models.py @@ -1405,12 +1405,12 @@ class InboundSms(db.Model): def serialize(self): return { 'id': str(self.id), - 'created_at': self.created_at.isoformat(), + 'created_at': self.created_at.strftime(DATETIME_FORMAT), 'service_id': str(self.service_id), 'notify_number': self.notify_number, 'user_number': self.user_number, 'content': self.content, - 'provider_date': self.provider_date and self.provider_date.isoformat(), + 'provider_date': self.provider_date and self.provider_date.strftime(DATETIME_FORMAT), 'provider_reference': self.provider_reference } diff --git a/app/v2/inbound_sms/__init__.py b/app/v2/inbound_sms/__init__.py new file mode 100644 index 000000000..d48a37f94 --- /dev/null +++ b/app/v2/inbound_sms/__init__.py @@ -0,0 +1,6 @@ +from flask import Blueprint +from app.v2.errors import register_errors + +v2_inbound_sms_blueprint = Blueprint("v2_inbound_sms", __name__, url_prefix='/v2/inbound_sms') + +register_errors(v2_inbound_sms_blueprint) diff --git a/app/v2/inbound_sms/get_inbound_sms.py b/app/v2/inbound_sms/get_inbound_sms.py new file mode 100644 index 000000000..10ae26885 --- /dev/null +++ b/app/v2/inbound_sms/get_inbound_sms.py @@ -0,0 +1,36 @@ +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.v2.errors import BadRequestError +from app.v2.inbound_sms import v2_inbound_sms_blueprint + + +@v2_inbound_sms_blueprint.route("/", methods=['GET']) +def get_inbound_sms_by_number(user_number): + try: + validate_and_format_phone_number(user_number) + except InvalidPhoneError as e: + raise BadRequestError(message=str(e)) + + inbound_sms = inbound_sms_dao.dao_get_inbound_sms_for_service( + authenticated_service.id, user_number=user_number + ) + + return jsonify(inbound_sms_list=[i.serialize() for i in inbound_sms]), 200 + + +@v2_inbound_sms_blueprint.route("", methods=['GET']) +def get_all_inbound_sms(): + all_inbound_sms = inbound_sms_dao.dao_get_inbound_sms_for_service( + authenticated_service.id + ) + + return jsonify(inbound_sms_list=[i.serialize() for i in all_inbound_sms]), 200 diff --git a/app/v2/inbound_sms/inbound_sms_schemas.py b/app/v2/inbound_sms/inbound_sms_schemas.py new file mode 100644 index 000000000..8dedda195 --- /dev/null +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -0,0 +1,51 @@ +from app.schema_validation.definitions import uuid + + +get_inbound_sms_single_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "GET inbound sms schema response", + "type": "object", + "title": "GET response v2/inbound_sms", + "properties": { + "provider_date": { + "format": "date-time", + "type": "string", + "description": "Date+time sent by provider" + }, + "provider_reference": {"type": ["string", "null"]}, + "user_number": {"type": "string"}, + "created_at": { + "format": "date-time", + "type": "string", + "description": "Date+time created at" + }, + "service_id": uuid, + "id": uuid, + "notify_number": {"type": "string"}, + "content": {"type": "string"}, + }, + "required": [ + "id", "provider_date", "provider_reference", + "user_number", "created_at", "service_id", + "notify_number", "content" + ], +} + +get_inbound_sms_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "GET list of inbound sms response schema", + "type": "object", + "properties": { + "inbound_sms_list": { + "type": "array", + "items": { + "type": "object", + "$ref": "#/definitions/inbound_sms" + } + }, + }, + "required": ["inbound_sms_list"], + "definitions": { + "inbound_sms": get_inbound_sms_single_response + } +} diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e03b2b7dd..081cdf455 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -47,6 +47,7 @@ from tests.app.db import ( create_api_key, create_inbound_number, create_letter_contact, + create_inbound_sms, ) @@ -1014,6 +1015,11 @@ def sample_inbound_numbers(notify_db, notify_db_session, sample_service): return inbound_numbers +@pytest.fixture +def sample_inbound_sms(notify_db, notify_db_session, sample_service): + return create_inbound_sms(sample_service) + + @pytest.fixture def restore_provider_details(notify_db, notify_db_session): """ diff --git a/tests/app/db.py b/tests/app/db.py index c7c950544..cb7729244 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,6 +1,8 @@ from datetime import datetime +import pytz import uuid +from app import DATETIME_FORMAT from app import db from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api @@ -247,10 +249,10 @@ def create_inbound_sms( ): inbound = InboundSms( service=service, - created_at=created_at or datetime.utcnow(), + created_at=created_at or datetime.utcnow().strftime(DATETIME_FORMAT), notify_number=notify_number or service.sms_sender, user_number=user_number, - provider_date=provider_date or datetime.utcnow(), + provider_date=provider_date or datetime.utcnow().strftime(DATETIME_FORMAT), provider_reference=provider_reference or 'foo', content=content, provider=provider diff --git a/tests/app/v2/inbound_sms/__init__.py b/tests/app/v2/inbound_sms/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/v2/inbound_sms/test_get_inbound_sms.py b/tests/app/v2/inbound_sms/test_get_inbound_sms.py new file mode 100644 index 000000000..028692414 --- /dev/null +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -0,0 +1,131 @@ +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 + + +def test_get_all_inbound_sms_returns_200( + client, sample_service +): + all_inbound_sms = [ + create_inbound_sms(service=sample_service, user_number='447700900111', content='Hi'), + create_inbound_sms(service=sample_service, user_number='447700900112'), + create_inbound_sms(service=sample_service, user_number='447700900111', content='Bye'), + create_inbound_sms(service=sample_service, user_number='447700900113') + ] + + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.get( + path='/v2/inbound_sms', + 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))['inbound_sms_list'] + + reversed_all_inbound_sms = sorted(all_inbound_sms, key=lambda sms: sms.created_at, reverse=True) + + expected_response = [i.serialize() for i in reversed_all_inbound_sms] + + assert json_response == expected_response + + +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/inbound_sms', + 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))['inbound_sms_list'] + + expected_response = [] + + assert json_response == expected_response + + +def test_get_inbound_sms_by_number_returns_200( + client, sample_service +): + sample_inbound_sms = create_inbound_sms(service=sample_service) + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.get( + path='/v2/inbound_sms/{}'.format(sample_inbound_sms.user_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))['inbound_sms_list'] + + expected_response = [sample_inbound_sms.serialize()] + + assert json_response == expected_response + + +def test_get_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/inbound_sms', + 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))['inbound_sms_list'] + + expected_response = [] + + assert json_response == expected_response + + +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/inbound_sms/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))['inbound_sms_list'] + 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/inbound_sms/{}'.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 + } diff --git a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py new file mode 100644 index 000000000..04e61a321 --- /dev/null +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -0,0 +1,75 @@ +import uuid + +import pytest +from flask import json +from jsonschema.exceptions import ValidationError + +from app.dao.api_key_dao import save_model_api_key +from app.models import ApiKey, KEY_TYPE_NORMAL, EMAIL_TYPE, SMS_TYPE, TEMPLATE_TYPES +from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_response, get_inbound_sms_single_response +from app.schema_validation import validate + +from tests import create_authorization_header + + +valid_inbound_sms = { + "provider_date": "2017-11-02T15:07:57.199541Z", + "provider_reference": "foo", + "user_number": "447700900111", + "created_at": "2017-11-02T15:07:57.197546Z", + "service_id": "a5149c32-f03b-4711-af49-ad6993797d45", + "id": "342786aa-23ce-4695-9aad-7f79e68ee29a", + "notify_number": "testing", + "content": "Hello" +} + +valid_inbound_sms_list = { + "inbound_sms_list": [valid_inbound_sms] +} + +invalid_inbound_sms = { + "provider_date": "2017-11-02T15:07:57.199541", + "provider_reference": "foo", + "user_number": "447700900111", + "created_at": "2017-11-02T15:07:57.197546", + "service_id": "a5149c32-f03b-4711-af49-ad6993797d45", + "id": "342786aa-23ce-4695-9aad-7f79e68ee29a", + "notify_number": "testing" +} + +invalid_inbound_sms_list = { + "inbound_sms_list": [invalid_inbound_sms] +} + + +def _get_inbound_sms(client, inbound_sms, url): + auth_header = create_authorization_header(service_id=inbound_sms.service_id) + response = client.get(url, headers=[auth_header]) + return json.loads(response.get_data(as_text=True)) + + +def test_get_inbound_sms_contract(client, sample_inbound_sms): + response_json = _get_inbound_sms( + client, + sample_inbound_sms, + '/v2/inbound_sms/{}'.format(sample_inbound_sms.user_number) + ) + res = validate(response_json, get_inbound_sms_response) + + +def test_valid_inbound_sms_json(): + validate(valid_inbound_sms, get_inbound_sms_single_response) + + +def test_valid_inbound_sms_list_json(): + validate(valid_inbound_sms_list, get_inbound_sms_response) + + +def test_invalid_inbound_sms_json(): + with pytest.raises(expected_exception=ValidationError): + validate(invalid_inbound_sms, get_inbound_sms_single_response) + + +def test_invalid_inbound_sms_list_json(): + with pytest.raises(expected_exception=ValidationError): + validate(invalid_inbound_sms_list, get_inbound_sms_response) From 1c673a4afe2a1aac8fb9679455ec84c0ee582619 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 3 Nov 2017 17:26:53 +0000 Subject: [PATCH 02/15] Refactor tests --- tests/app/db.py | 6 ++--- .../inbound_sms/test_inbound_sms_schemas.py | 24 ++++++------------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/tests/app/db.py b/tests/app/db.py index cb7729244..c7c950544 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,8 +1,6 @@ from datetime import datetime -import pytz import uuid -from app import DATETIME_FORMAT from app import db from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api @@ -249,10 +247,10 @@ def create_inbound_sms( ): inbound = InboundSms( service=service, - created_at=created_at or datetime.utcnow().strftime(DATETIME_FORMAT), + created_at=created_at or datetime.utcnow(), notify_number=notify_number or service.sms_sender, user_number=user_number, - provider_date=provider_date or datetime.utcnow().strftime(DATETIME_FORMAT), + provider_date=provider_date or datetime.utcnow(), provider_reference=provider_reference or 'foo', content=content, provider=provider 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 04e61a321..9a7f654ba 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -1,11 +1,7 @@ -import uuid - import pytest from flask import json from jsonschema.exceptions import ValidationError -from app.dao.api_key_dao import save_model_api_key -from app.models import ApiKey, KEY_TYPE_NORMAL, EMAIL_TYPE, SMS_TYPE, TEMPLATE_TYPES from app.v2.inbound_sms.inbound_sms_schemas import get_inbound_sms_response, get_inbound_sms_single_response from app.schema_validation import validate @@ -42,23 +38,17 @@ invalid_inbound_sms_list = { } -def _get_inbound_sms(client, inbound_sms, url): - auth_header = create_authorization_header(service_id=inbound_sms.service_id) - response = client.get(url, headers=[auth_header]) - return json.loads(response.get_data(as_text=True)) - - def test_get_inbound_sms_contract(client, sample_inbound_sms): - response_json = _get_inbound_sms( - client, - sample_inbound_sms, - '/v2/inbound_sms/{}'.format(sample_inbound_sms.user_number) - ) - res = validate(response_json, get_inbound_sms_response) + auth_header = create_authorization_header(service_id=sample_inbound_sms.service_id) + response = client.get('/v2/inbound_sms/{}'.format(sample_inbound_sms.user_number), headers=[auth_header]) + response_json = json.loads(response.get_data(as_text=True)) + + assert validate(response_json, get_inbound_sms_response)['inbound_sms_list'][0] \ + == sample_inbound_sms.serialize() def test_valid_inbound_sms_json(): - validate(valid_inbound_sms, get_inbound_sms_single_response) + assert validate(valid_inbound_sms, get_inbound_sms_single_response) == valid_inbound_sms def test_valid_inbound_sms_list_json(): From 02ee072251a3b1ac24d468c84cb730655e4f77b5 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 6 Nov 2017 18:19:02 +0000 Subject: [PATCH 03/15] Refactor inbound sms dao Added function for paginated results --- app/dao/inbound_sms_dao.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 18060cced..5bbc9a3a9 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -2,7 +2,8 @@ from datetime import ( timedelta, datetime ) - +from flask import current_app +from sqlalchemy import desc from app import db from app.dao.dao_utils import transactional @@ -31,6 +32,34 @@ def dao_get_inbound_sms_for_service(service_id, limit=None, user_number=None): return q.all() +def dao_get_paginated_inbound_sms_for_service( + service_id, + user_number=None, + older_than=None, + page=1, + page_size=None +): + if page_size is None: + page_size = current_app.config['PAGE_SIZE'] + + filters = [InboundSms.service_id == service_id] + + if older_than is not None: + older_than_created_at = db.session.query( + InboundSms.created_at).filter(InboundSms.id == older_than).as_scalar() + filters.append(InboundSms.created_at < older_than_created_at) + + if user_number: + filters.append(InboundSms.user_number == user_number) + + query = InboundSms.query.filter(*filters) + + return query.order_by(desc(InboundSms.created_at)).paginate( + page=page, + per_page=page_size + ).items + + def dao_count_inbound_sms_for_service(service_id): return InboundSms.query.filter( InboundSms.service_id == service_id From 0350c99a3e99e35b0de10cf29616b139e3b57b8f Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 6 Nov 2017 18:22:18 +0000 Subject: [PATCH 04/15] Updated inbound sms API endpoints - from `/v2/inbound_sms` to `/v2/received-text-messages` - use paginated results --- app/v2/inbound_sms/__init__.py | 2 +- app/v2/inbound_sms/get_inbound_sms.py | 61 ++++++++++++++++--- app/v2/inbound_sms/inbound_sms_schemas.py | 15 ++++- .../v2/inbound_sms/test_get_inbound_sms.py | 56 ++++++++++++++--- .../inbound_sms/test_inbound_sms_schemas.py | 8 ++- 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/app/v2/inbound_sms/__init__.py b/app/v2/inbound_sms/__init__.py index d48a37f94..5c713b2ef 100644 --- a/app/v2/inbound_sms/__init__.py +++ b/app/v2/inbound_sms/__init__.py @@ -1,6 +1,6 @@ from flask import Blueprint from app.v2.errors import register_errors -v2_inbound_sms_blueprint = Blueprint("v2_inbound_sms", __name__, url_prefix='/v2/inbound_sms') +v2_inbound_sms_blueprint = Blueprint("v2_inbound_sms", __name__, url_prefix='/v2/received-text-messages') register_errors(v2_inbound_sms_blueprint) diff --git a/app/v2/inbound_sms/get_inbound_sms.py b/app/v2/inbound_sms/get_inbound_sms.py index 10ae26885..1ebae90c5 100644 --- a/app/v2/inbound_sms/get_inbound_sms.py +++ b/app/v2/inbound_sms/get_inbound_sms.py @@ -15,22 +15,69 @@ from app.v2.inbound_sms import v2_inbound_sms_blueprint @v2_inbound_sms_blueprint.route("/", methods=['GET']) def get_inbound_sms_by_number(user_number): + _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] + try: - validate_and_format_phone_number(user_number) + user_number = validate_and_format_phone_number(user_number) except InvalidPhoneError as e: raise BadRequestError(message=str(e)) - inbound_sms = inbound_sms_dao.dao_get_inbound_sms_for_service( - authenticated_service.id, user_number=user_number + 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'), + page_size=current_app.config.get('API_PAGE_SIZE') ) - return jsonify(inbound_sms_list=[i.serialize() for i in inbound_sms]), 200 + return jsonify( + inbound_sms_list=[i.serialize() for i in paginated_inbound_sms], + links=_build_links( + paginated_inbound_sms, + endpoint='get_inbound_sms_by_number', + user_number=user_number + ) + ), 200 @v2_inbound_sms_blueprint.route("", methods=['GET']) def get_all_inbound_sms(): - all_inbound_sms = inbound_sms_dao.dao_get_inbound_sms_for_service( - authenticated_service.id + _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(inbound_sms_list=[i.serialize() for i in all_inbound_sms]), 200 + return jsonify( + inbound_sms_list=[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): + _links = { + 'current': url_for( + "v2_inbound_sms.{}".format(endpoint), + user_number=user_number, + _external=True, + ), + } + + if inbound_sms_list: + _links['next'] = url_for( + "v2_inbound_sms.{}".format(endpoint), + user_number=user_number, + older_than=inbound_sms_list[-1].id, + _external=True, + ) + + return _links diff --git a/app/v2/inbound_sms/inbound_sms_schemas.py b/app/v2/inbound_sms/inbound_sms_schemas.py index 8dedda195..0990c1e2e 100644 --- a/app/v2/inbound_sms/inbound_sms_schemas.py +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -43,8 +43,21 @@ get_inbound_sms_response = { "$ref": "#/definitions/inbound_sms" } }, + "links": { + "type": "object", + "properties": { + "current": { + "type": "string" + }, + "next": { + "type": "string" + } + }, + "additionalProperties": False, + "required": ["current"] + } }, - "required": ["inbound_sms_list"], + "required": ["inbound_sms_list", "links"], "definitions": { "inbound_sms": get_inbound_sms_single_response } 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 028692414..34eb0420a 100644 --- a/tests/app/v2/inbound_sms/test_get_inbound_sms.py +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -14,12 +14,12 @@ def test_get_all_inbound_sms_returns_200( create_inbound_sms(service=sample_service, user_number='447700900111', content='Hi'), create_inbound_sms(service=sample_service, user_number='447700900112'), create_inbound_sms(service=sample_service, user_number='447700900111', content='Bye'), - create_inbound_sms(service=sample_service, user_number='447700900113') + create_inbound_sms(service=sample_service, user_number='07700900113') ] auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/inbound_sms', + path='/v2/received-text-messages', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -34,12 +34,47 @@ 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') +]) +def test_get_all_inbound_sms_generate_page_links( + client, sample_service, mocker, inbound_sms_path, user_number +): + mocker.patch.dict("app.v2.inbound_sms.get_inbound_sms.current_app.config", {"API_PAGE_SIZE": 1}) + all_inbound_sms = [ + create_inbound_sms(service=sample_service, user_number='447700900111', content='Hi'), + create_inbound_sms(service=sample_service, user_number='447700900111'), + ] + + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.get( + path=url_for(inbound_sms_path, user_number=user_number), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + + json_response = json.loads(response.get_data(as_text=True)) + expected_inbound_sms_list = [all_inbound_sms[-1].serialize()] + + assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert url_for( + inbound_sms_path, + user_number=user_number, + _external=True) == json_response['links']['current'] + assert url_for( + inbound_sms_path, + user_number=user_number, + older_than=all_inbound_sms[-1].id, + _external=True) == json_response['links']['next'] + + 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/inbound_sms', + path='/v2/received-text-messages', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -55,10 +90,13 @@ def test_get_all_inbound_sms_for_no_inbound_sms_returns_200( def test_get_inbound_sms_by_number_returns_200( client, sample_service ): - sample_inbound_sms = create_inbound_sms(service=sample_service) + 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') + auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/inbound_sms/{}'.format(sample_inbound_sms.user_number), + path='/v2/received-text-messages/{}'.format('07700900111'), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -66,7 +104,7 @@ def test_get_inbound_sms_by_number_returns_200( json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] - expected_response = [sample_inbound_sms.serialize()] + expected_response = [sample_inbound_sms2.serialize(), sample_inbound_sms1.serialize()] assert json_response == expected_response @@ -76,7 +114,7 @@ def test_get_inbound_sms_for_no_inbound_sms_returns_200( ): auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( - path='/v2/inbound_sms', + path='/v2/received-text-messages', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -92,7 +130,7 @@ def test_get_inbound_sms_for_no_inbound_sms_returns_200( 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/inbound_sms/447700900000', + path='/v2/received-text-messages/447700900000', headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -113,7 +151,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/inbound_sms/{}'.format(invalid_number), + path='/v2/received-text-messages/{}'.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 9a7f654ba..6365d90a5 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -20,7 +20,11 @@ valid_inbound_sms = { } valid_inbound_sms_list = { - "inbound_sms_list": [valid_inbound_sms] + "inbound_sms_list": [valid_inbound_sms], + "links": { + "current": valid_inbound_sms["id"] + } + } invalid_inbound_sms = { @@ -40,7 +44,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/inbound_sms/{}'.format(sample_inbound_sms.user_number), headers=[auth_header]) + response = client.get('/v2/received-text-messages/{}'.format(sample_inbound_sms.user_number), headers=[auth_header]) response_json = json.loads(response.get_data(as_text=True)) assert validate(response_json, get_inbound_sms_response)['inbound_sms_list'][0] \ From 2e66fe5e38c50002859a6ea57346eedb2855bb4b Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 7 Nov 2017 11:55:51 +0000 Subject: [PATCH 05/15] Removed `page` from get paginated args as not used --- app/dao/inbound_sms_dao.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 5bbc9a3a9..4813fc695 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -36,7 +36,6 @@ def dao_get_paginated_inbound_sms_for_service( service_id, user_number=None, older_than=None, - page=1, page_size=None ): if page_size is None: @@ -44,7 +43,7 @@ def dao_get_paginated_inbound_sms_for_service( filters = [InboundSms.service_id == service_id] - if older_than is not None: + if older_than: older_than_created_at = db.session.query( InboundSms.created_at).filter(InboundSms.id == older_than).as_scalar() filters.append(InboundSms.created_at < older_than_created_at) @@ -55,7 +54,6 @@ def dao_get_paginated_inbound_sms_for_service( query = InboundSms.query.filter(*filters) return query.order_by(desc(InboundSms.created_at)).paginate( - page=page, per_page=page_size ).items From 425d66bf292b3496cf40f7b7088029a030e60e1b Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 7 Nov 2017 11:56:27 +0000 Subject: [PATCH 06/15] Added more inbound sms tests --- .../v2/inbound_sms/test_get_inbound_sms.py | 94 +++++++++++++++++-- 1 file changed, 88 insertions(+), 6 deletions(-) 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 34eb0420a..1f1f1c771 100644 --- a/tests/app/v2/inbound_sms/test_get_inbound_sms.py +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -38,15 +38,21 @@ def test_get_all_inbound_sms_returns_200( ('v2_inbound_sms.get_all_inbound_sms', None), ('v2_inbound_sms.get_inbound_sms_by_number', '447700900111') ]) -def test_get_all_inbound_sms_generate_page_links( +def test_get_inbound_sms_generate_page_links( client, sample_service, mocker, inbound_sms_path, user_number ): - mocker.patch.dict("app.v2.inbound_sms.get_inbound_sms.current_app.config", {"API_PAGE_SIZE": 1}) + mocker.patch.dict( + "app.v2.inbound_sms.get_inbound_sms.current_app.config", + {"API_PAGE_SIZE": 2} + ) all_inbound_sms = [ create_inbound_sms(service=sample_service, user_number='447700900111', content='Hi'), create_inbound_sms(service=sample_service, user_number='447700900111'), + create_inbound_sms(service=sample_service, user_number='447700900111', content='End'), ] + reversed_inbound_sms = sorted(all_inbound_sms, key=lambda sms: sms.created_at, reverse=True) + auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( path=url_for(inbound_sms_path, user_number=user_number), @@ -55,7 +61,7 @@ def test_get_all_inbound_sms_generate_page_links( assert response.status_code == 200 json_response = json.loads(response.get_data(as_text=True)) - expected_inbound_sms_list = [all_inbound_sms[-1].serialize()] + expected_inbound_sms_list = [i.serialize() for i in reversed_inbound_sms[:2]] assert json_response['inbound_sms_list'] == expected_inbound_sms_list assert url_for( @@ -65,10 +71,80 @@ def test_get_all_inbound_sms_generate_page_links( assert url_for( inbound_sms_path, user_number=user_number, - older_than=all_inbound_sms[-1].id, + 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') +]) +def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( + client, sample_service, mocker, inbound_sms_path, user_number +): + mocker.patch.dict( + "app.v2.inbound_sms.get_inbound_sms.current_app.config", + {"API_PAGE_SIZE": 2} + ) + all_inbound_sms = [ + create_inbound_sms(service=sample_service, user_number='447700900111', content='1'), + create_inbound_sms(service=sample_service, user_number='447700900111', content='2'), + create_inbound_sms(service=sample_service, user_number='447700900111', content='3'), + create_inbound_sms(service=sample_service, user_number='447700900111', content='4'), + ] + reversed_inbound_sms = sorted(all_inbound_sms, key=lambda sms: sms.created_at, reverse=True) + + 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), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + + json_response = json.loads(response.get_data(as_text=True)) + expected_inbound_sms_list = [i.serialize() for i in reversed_inbound_sms[2:]] + + assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert url_for( + inbound_sms_path, + user_number=user_number, + _external=True) == json_response['links']['current'] + assert url_for( + inbound_sms_path, + 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') +]) +def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( + client, sample_inbound_sms, mocker, inbound_sms_path, user_number +): + mocker.patch.dict( + "app.v2.inbound_sms.get_inbound_sms.current_app.config", + {"API_PAGE_SIZE": 1} + ) + + 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), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + + json_response = json.loads(response.get_data(as_text=True)) + expected_inbound_sms_list = [] + assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert url_for( + inbound_sms_path, + 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 ): @@ -87,16 +163,22 @@ def test_get_all_inbound_sms_for_no_inbound_sms_returns_200( 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 + 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/{}'.format('07700900111'), + path='/v2/received-text-messages/{}'.format(requested_number), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 From ac15e7fd8bcc74c2181b75e7b67782d905fed40e Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 7 Nov 2017 12:19:44 +0000 Subject: [PATCH 07/15] Renamed `inbound_sms_list` key name to `received_text_messages` --- app/v2/inbound_sms/get_inbound_sms.py | 4 ++-- app/v2/inbound_sms/inbound_sms_schemas.py | 4 ++-- tests/app/v2/inbound_sms/test_get_inbound_sms.py | 16 ++++++++-------- .../v2/inbound_sms/test_inbound_sms_schemas.py | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/v2/inbound_sms/get_inbound_sms.py b/app/v2/inbound_sms/get_inbound_sms.py index 1ebae90c5..563491a55 100644 --- a/app/v2/inbound_sms/get_inbound_sms.py +++ b/app/v2/inbound_sms/get_inbound_sms.py @@ -34,7 +34,7 @@ def get_inbound_sms_by_number(user_number): ) return jsonify( - inbound_sms_list=[i.serialize() for i in paginated_inbound_sms], + received_text_messages=[i.serialize() for i in paginated_inbound_sms], links=_build_links( paginated_inbound_sms, endpoint='get_inbound_sms_by_number', @@ -58,7 +58,7 @@ def get_all_inbound_sms(): ) return jsonify( - inbound_sms_list=[i.serialize() for i in paginated_inbound_sms], + received_text_messages=[i.serialize() for i in paginated_inbound_sms], links=_build_links(paginated_inbound_sms, endpoint='get_all_inbound_sms') ), 200 diff --git a/app/v2/inbound_sms/inbound_sms_schemas.py b/app/v2/inbound_sms/inbound_sms_schemas.py index 0990c1e2e..d21c2b27b 100644 --- a/app/v2/inbound_sms/inbound_sms_schemas.py +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -36,7 +36,7 @@ get_inbound_sms_response = { "description": "GET list of inbound sms response schema", "type": "object", "properties": { - "inbound_sms_list": { + "received_text_messages": { "type": "array", "items": { "type": "object", @@ -57,7 +57,7 @@ get_inbound_sms_response = { "required": ["current"] } }, - "required": ["inbound_sms_list", "links"], + "required": ["received_text_messages", "links"], "definitions": { "inbound_sms": get_inbound_sms_single_response } 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 1f1f1c771..a45a01928 100644 --- a/tests/app/v2/inbound_sms/test_get_inbound_sms.py +++ b/tests/app/v2/inbound_sms/test_get_inbound_sms.py @@ -25,7 +25,7 @@ def test_get_all_inbound_sms_returns_200( assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' - json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] + json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] reversed_all_inbound_sms = sorted(all_inbound_sms, key=lambda sms: sms.created_at, reverse=True) @@ -63,7 +63,7 @@ def test_get_inbound_sms_generate_page_links( json_response = json.loads(response.get_data(as_text=True)) expected_inbound_sms_list = [i.serialize() for i in reversed_inbound_sms[:2]] - assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( inbound_sms_path, user_number=user_number, @@ -104,7 +104,7 @@ def test_get_next_inbound_sms_will_get_correct_inbound_sms_list( json_response = json.loads(response.get_data(as_text=True)) expected_inbound_sms_list = [i.serialize() for i in reversed_inbound_sms[2:]] - assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( inbound_sms_path, user_number=user_number, @@ -137,7 +137,7 @@ def test_get_next_inbound_sms_at_end_will_return_empty_inbound_sms_list( json_response = json.loads(response.get_data(as_text=True)) expected_inbound_sms_list = [] - assert json_response['inbound_sms_list'] == expected_inbound_sms_list + assert json_response['received_text_messages'] == expected_inbound_sms_list assert url_for( inbound_sms_path, user_number=user_number, @@ -156,7 +156,7 @@ def test_get_all_inbound_sms_for_no_inbound_sms_returns_200( assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' - json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] + json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] expected_response = [] @@ -184,7 +184,7 @@ def test_get_inbound_sms_by_number_returns_200( assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' - json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] + json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] expected_response = [sample_inbound_sms2.serialize(), sample_inbound_sms1.serialize()] @@ -202,7 +202,7 @@ def test_get_inbound_sms_for_no_inbound_sms_returns_200( assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' - json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] + json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] expected_response = [] @@ -218,7 +218,7 @@ def test_get_inbound_sms_by_nonexistent_number(client, sample_service): assert response.status_code == 200 assert response.headers['Content-type'] == 'application/json' - json_response = json.loads(response.get_data(as_text=True))['inbound_sms_list'] + json_response = json.loads(response.get_data(as_text=True))['received_text_messages'] expected_response = [] assert json_response == expected_response 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 6365d90a5..47db068ea 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -20,7 +20,7 @@ valid_inbound_sms = { } valid_inbound_sms_list = { - "inbound_sms_list": [valid_inbound_sms], + "received_text_messages": [valid_inbound_sms], "links": { "current": valid_inbound_sms["id"] } @@ -38,7 +38,7 @@ invalid_inbound_sms = { } invalid_inbound_sms_list = { - "inbound_sms_list": [invalid_inbound_sms] + "received_text_messages": [invalid_inbound_sms] } @@ -47,7 +47,7 @@ def test_get_inbound_sms_contract(client, sample_inbound_sms): response = client.get('/v2/received-text-messages/{}'.format(sample_inbound_sms.user_number), headers=[auth_header]) response_json = json.loads(response.get_data(as_text=True)) - assert validate(response_json, get_inbound_sms_response)['inbound_sms_list'][0] \ + assert validate(response_json, get_inbound_sms_response)['received_text_messages'][0] \ == sample_inbound_sms.serialize() From dca331e3ea27bbeaf0fca57f33a1a26e8f5359ee Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 7 Nov 2017 16:39:11 +0000 Subject: [PATCH 08/15] Refactor to use query string rather than path params --- app/v2/inbound_sms/get_inbound_sms.py | 58 +++++------------ app/v2/inbound_sms/inbound_sms_schemas.py | 16 ++++- .../v2/inbound_sms/test_get_inbound_sms.py | 62 ++++++++++--------- .../inbound_sms/test_inbound_sms_schemas.py | 3 +- 4 files changed, 68 insertions(+), 71 deletions(-) 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] \ From 874ca5a8affdaf17eaef9f93f5a4799decc090fe Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 8 Nov 2017 10:57:16 +0000 Subject: [PATCH 09/15] 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) From cd4637a1d7e378d06d9e10060e3e9b9c40764ae4 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:22:27 +0000 Subject: [PATCH 10/15] Removed provider_reference and provider_date from serialize --- app/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models.py b/app/models.py index ea7b93a4e..81cc4d727 100644 --- a/app/models.py +++ b/app/models.py @@ -1410,8 +1410,6 @@ class InboundSms(db.Model): 'notify_number': self.notify_number, 'user_number': self.user_number, 'content': self.content, - 'provider_date': self.provider_date and self.provider_date.strftime(DATETIME_FORMAT), - 'provider_reference': self.provider_reference } From 61a9f6e9507757cc29496479ab5480f0ee86e7df Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:22:53 +0000 Subject: [PATCH 11/15] Removed provider_date and provider_reference from schema --- app/v2/inbound_sms/inbound_sms_schemas.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/app/v2/inbound_sms/inbound_sms_schemas.py b/app/v2/inbound_sms/inbound_sms_schemas.py index e6ef8cc98..e68952a23 100644 --- a/app/v2/inbound_sms/inbound_sms_schemas.py +++ b/app/v2/inbound_sms/inbound_sms_schemas.py @@ -18,12 +18,6 @@ get_inbound_sms_single_response = { "type": "object", "title": "GET response v2/inbound_sms", "properties": { - "provider_date": { - "format": "date-time", - "type": "string", - "description": "Date+time sent by provider" - }, - "provider_reference": {"type": ["string", "null"]}, "user_number": {"type": "string"}, "created_at": { "format": "date-time", @@ -36,8 +30,7 @@ get_inbound_sms_single_response = { "content": {"type": "string"}, }, "required": [ - "id", "provider_date", "provider_reference", - "user_number", "created_at", "service_id", + "id", "user_number", "created_at", "service_id", "notify_number", "content" ], "additionalProperties": False, From 36fb2fb28cbe78ff0ad9008cf1ffb1e017376852 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:23:16 +0000 Subject: [PATCH 12/15] Remove user_number from inbound sms dao --- app/dao/inbound_sms_dao.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 4813fc695..f7d54281e 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -34,7 +34,6 @@ def dao_get_inbound_sms_for_service(service_id, limit=None, user_number=None): def dao_get_paginated_inbound_sms_for_service( service_id, - user_number=None, older_than=None, page_size=None ): @@ -48,9 +47,6 @@ def dao_get_paginated_inbound_sms_for_service( InboundSms.created_at).filter(InboundSms.id == older_than).as_scalar() filters.append(InboundSms.created_at < older_than_created_at) - if user_number: - filters.append(InboundSms.user_number == user_number) - query = InboundSms.query.filter(*filters) return query.order_by(desc(InboundSms.created_at)).paginate( From 362072129fa83df38e7b3cc9e57a78d2032c6d16 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:23:45 +0000 Subject: [PATCH 13/15] Add tests for dao_get_paginated_inbound_sms_for_service --- tests/app/dao/test_inbound_sms_dao.py | 85 +++++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index b26dc913e..7967d8522 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -6,7 +6,8 @@ from app.dao.inbound_sms_dao import ( dao_get_inbound_sms_for_service, dao_count_inbound_sms_for_service, delete_inbound_sms_created_more_than_a_week_ago, - dao_get_inbound_sms_by_id + dao_get_inbound_sms_by_id, + dao_get_paginated_inbound_sms_for_service ) from tests.app.db import create_inbound_sms, create_service @@ -89,9 +90,83 @@ def test_should_not_delete_inbound_sms_before_seven_days(sample_service): assert len(InboundSms.query.all()) == 2 -def test_get_inbound_sms_by_id_returns(sample_service): - inbound = create_inbound_sms(sample_service) +def test_get_inbound_sms_by_id_returns(sample_inbound_sms): + inbound_from_db = dao_get_inbound_sms_by_id(sample_inbound_sms.service.id, sample_inbound_sms.id) - inbound_from_db = dao_get_inbound_sms_by_id(sample_service.id, inbound.id) + assert sample_inbound_sms == inbound_from_db - assert inbound == inbound_from_db + +def test_dao_get_paginated_inbound_sms_for_service(sample_inbound_sms): + inbound_from_db = dao_get_paginated_inbound_sms_for_service(sample_inbound_sms.service.id) + + assert sample_inbound_sms == inbound_from_db[0] + + +def test_dao_get_paginated_inbound_sms_for_service_return_only_for_service(sample_inbound_sms): + another_service = create_service(service_name='another service') + another_inbound_sms = create_inbound_sms(another_service) + + inbound_from_db = dao_get_paginated_inbound_sms_for_service(sample_inbound_sms.service.id) + + assert sample_inbound_sms in inbound_from_db + assert another_inbound_sms not in inbound_from_db + + +def test_dao_get_paginated_inbound_sms_for_service_no_inbound_sms_returns_empty_list(sample_service): + inbound_from_db = dao_get_paginated_inbound_sms_for_service(sample_service.id) + + assert inbound_from_db == [] + + +def test_dao_get_paginated_inbound_sms_for_service_page_size_returns_correct_size(sample_service): + inbound_sms_list = [ + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + ] + reversed_inbound_sms = sorted(inbound_sms_list, key=lambda sms: sms.created_at, reverse=True) + + inbound_from_db = dao_get_paginated_inbound_sms_for_service( + sample_service.id, + older_than=reversed_inbound_sms[1].id, + page_size=2 + ) + + assert len(inbound_from_db) == 2 + + +def test_dao_get_paginated_inbound_sms_for_service_older_than_returns_correct_list(sample_service): + inbound_sms_list = [ + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + ] + reversed_inbound_sms = sorted(inbound_sms_list, key=lambda sms: sms.created_at, reverse=True) + + inbound_from_db = dao_get_paginated_inbound_sms_for_service( + sample_service.id, + older_than=reversed_inbound_sms[1].id, + page_size=2 + ) + + expected_inbound_sms = reversed_inbound_sms[2:] + + assert expected_inbound_sms == inbound_from_db + + +def test_dao_get_paginated_inbound_sms_for_service_older_than_end_returns_empty_list(sample_service): + inbound_sms_list = [ + create_inbound_sms(sample_service), + create_inbound_sms(sample_service), + ] + reversed_inbound_sms = sorted(inbound_sms_list, key=lambda sms: sms.created_at, reverse=True) + + inbound_from_db = dao_get_paginated_inbound_sms_for_service( + sample_service.id, + older_than=reversed_inbound_sms[1].id, + page_size=2 + ) + + assert inbound_from_db == [] From aac7551fc82336fc2e02f0a2af079c143dc9db4a Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:24:08 +0000 Subject: [PATCH 14/15] Removed provider_date and provider_reference --- tests/app/inbound_sms/test_rest.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index 9d2c891f2..a693307ef 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -33,9 +33,7 @@ def test_get_inbound_sms_with_no_params(client, sample_service): 'service_id', 'notify_number', 'user_number', - 'content', - 'provider_date', - 'provider_reference' + 'content' } @@ -178,9 +176,7 @@ def test_get_inbound_sms(admin_request, sample_service): 'service_id', 'notify_number', 'user_number', - 'content', - 'provider_date', - 'provider_reference' + 'content' } From 99c2ccb496f8ac77e86013c780af4f1cee33022a Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 9 Nov 2017 15:24:30 +0000 Subject: [PATCH 15/15] Refactor schema --- tests/app/v2/inbound_sms/test_inbound_sms_schemas.py | 4 ---- 1 file changed, 4 deletions(-) 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 fe9613056..c2e32f93a 100644 --- a/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py +++ b/tests/app/v2/inbound_sms/test_inbound_sms_schemas.py @@ -13,8 +13,6 @@ 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", "user_number": "447700900111", "created_at": "2017-11-02T15:07:57.197546Z", "service_id": "a5149c32-f03b-4711-af49-ad6993797d45", @@ -32,8 +30,6 @@ valid_inbound_sms_list = { } invalid_inbound_sms = { - "provider_date": "2017-11-02T15:07:57.199541", - "provider_reference": "foo", "user_number": "447700900111", "created_at": "2017-11-02T15:07:57.197546", "service_id": "a5149c32-f03b-4711-af49-ad6993797d45",