From 23a501af1691227a9fdd30e22115bfc69468f336 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 6 Jun 2017 17:11:59 +0100 Subject: [PATCH 1/3] Add dao to get inbound sms by id --- app/dao/inbound_sms_dao.py | 4 ++++ tests/app/dao/test_inbound_sms_dao.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 3411aeb22..d87c1b1ed 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -47,3 +47,7 @@ def delete_inbound_sms_created_more_than_a_week_ago(): ).delete(synchronize_session='fetch') return deleted + + +def dao_get_inbound_sms_by_id(inbound_id): + return InboundSms.query.filter_by(id=inbound_id).one() diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index 2731562c4..6162b6d82 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -5,7 +5,8 @@ from freezegun import freeze_time 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 + delete_inbound_sms_created_more_than_a_week_ago, + dao_get_inbound_sms_by_id ) from tests.app.db import create_inbound_sms, create_service @@ -86,3 +87,11 @@ def test_should_not_delete_inbound_sms_before_seven_days(sample_service): delete_inbound_sms_created_more_than_a_week_ago() assert len(InboundSms.query.all()) == 2 + + +def test_get_inbound_sms_by_id_returns(sample_service): + inbound = create_inbound_sms(sample_service) + + inbound_from_db = dao_get_inbound_sms_by_id(inbound.id) + + assert inbound == inbound_from_db From ee488d416a0186ddb28e47764e9860d969498489 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 6 Jun 2017 17:12:21 +0100 Subject: [PATCH 2/3] Add endpoint to get inbound by id --- app/inbound_sms/rest.py | 23 ++++++++++++++++++++++- tests/app/inbound_sms/test_rest.py | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/inbound_sms/rest.py b/app/inbound_sms/rest.py index d4072dea3..755f830d3 100644 --- a/app/inbound_sms/rest.py +++ b/app/inbound_sms/rest.py @@ -1,11 +1,19 @@ +import uuid + from flask import ( Blueprint, jsonify, request ) +from werkzeug.exceptions import abort + from notifications_utils.recipients import validate_and_format_phone_number -from app.dao.inbound_sms_dao import dao_get_inbound_sms_for_service, dao_count_inbound_sms_for_service +from app.dao.inbound_sms_dao import ( + dao_get_inbound_sms_for_service, + dao_count_inbound_sms_for_service, + dao_get_inbound_sms_by_id +) from app.errors import register_errors inbound_sms = Blueprint( @@ -40,3 +48,16 @@ def get_inbound_sms_summary_for_service(service_id): count=count, most_recent=most_recent[0].created_at.isoformat() if most_recent else None ) + + +@inbound_sms.route('/', methods=['GET']) +def get_inbound_by_id(service_id, inbound_sms_id): + # TODO: Add JSON Schema here + try: + validated_uuid = uuid.UUID(inbound_sms_id) + except (ValueError, AttributeError): + abort(400) + + inbound_sms = dao_get_inbound_sms_by_id(validated_uuid) + + return jsonify(inbound_sms.serialize()), 200 diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index da10ecb6b..4f021d03a 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -112,3 +112,29 @@ def test_get_inbound_sms_summary_with_no_inbound(admin_request, sample_service): 'count': 0, 'most_recent': None } + + +def test_get_inbound_sms_by_id_returns_200(admin_request, sample_service): + inbound = create_inbound_sms(sample_service, user_number='447700900001') + + response = admin_request.get( + 'inbound_sms.get_inbound_by_id', + endpoint_kwargs={ + 'service_id': sample_service.id, + 'inbound_sms_id': inbound.id + } + ) + + assert response['user_number'] == '447700900001' + assert response['service_id'] == str(sample_service.id) + + +def test_get_inbound_sms_by_id_invalid_id_returns_400(admin_request, sample_service): + assert admin_request.get( + 'inbound_sms.get_inbound_by_id', + endpoint_kwargs={ + 'service_id': sample_service.id, + 'inbound_sms_id': 'dsadsda' + }, + expected_status=400 + ) From 5b4ceda1c6c49fc57b3c4fc978428354ac3cdfba Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 7 Jun 2017 14:23:31 +0100 Subject: [PATCH 3/3] Refactor: * Filter inbound by service_id * Refactor to return 404 instead of 400 for consistency --- app/dao/inbound_sms_dao.py | 7 +++++-- app/inbound_sms/rest.py | 15 +++------------ tests/app/dao/test_inbound_sms_dao.py | 2 +- tests/app/inbound_sms/test_rest.py | 17 ++++++++++++++--- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index d87c1b1ed..18060cced 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -49,5 +49,8 @@ def delete_inbound_sms_created_more_than_a_week_ago(): return deleted -def dao_get_inbound_sms_by_id(inbound_id): - return InboundSms.query.filter_by(id=inbound_id).one() +def dao_get_inbound_sms_by_id(service_id, inbound_id): + return InboundSms.query.filter_by( + id=inbound_id, + service_id=service_id + ).one() diff --git a/app/inbound_sms/rest.py b/app/inbound_sms/rest.py index 755f830d3..1eca4b089 100644 --- a/app/inbound_sms/rest.py +++ b/app/inbound_sms/rest.py @@ -1,11 +1,8 @@ -import uuid - from flask import ( Blueprint, jsonify, request ) -from werkzeug.exceptions import abort from notifications_utils.recipients import validate_and_format_phone_number @@ -19,7 +16,7 @@ from app.errors import register_errors inbound_sms = Blueprint( 'inbound_sms', __name__, - url_prefix='/service//inbound-sms' + url_prefix='/service//inbound-sms' ) register_errors(inbound_sms) @@ -50,14 +47,8 @@ def get_inbound_sms_summary_for_service(service_id): ) -@inbound_sms.route('/', methods=['GET']) +@inbound_sms.route('/', methods=['GET']) def get_inbound_by_id(service_id, inbound_sms_id): - # TODO: Add JSON Schema here - try: - validated_uuid = uuid.UUID(inbound_sms_id) - except (ValueError, AttributeError): - abort(400) - - inbound_sms = dao_get_inbound_sms_by_id(validated_uuid) + inbound_sms = dao_get_inbound_sms_by_id(service_id, inbound_sms_id) return jsonify(inbound_sms.serialize()), 200 diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index 6162b6d82..b26dc913e 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -92,6 +92,6 @@ def test_should_not_delete_inbound_sms_before_seven_days(sample_service): def test_get_inbound_sms_by_id_returns(sample_service): inbound = create_inbound_sms(sample_service) - inbound_from_db = dao_get_inbound_sms_by_id(inbound.id) + inbound_from_db = dao_get_inbound_sms_by_id(sample_service.id, inbound.id) assert inbound == inbound_from_db diff --git a/tests/app/inbound_sms/test_rest.py b/tests/app/inbound_sms/test_rest.py index 4f021d03a..b4c55cd83 100644 --- a/tests/app/inbound_sms/test_rest.py +++ b/tests/app/inbound_sms/test_rest.py @@ -129,12 +129,23 @@ def test_get_inbound_sms_by_id_returns_200(admin_request, sample_service): assert response['service_id'] == str(sample_service.id) -def test_get_inbound_sms_by_id_invalid_id_returns_400(admin_request, sample_service): +def test_get_inbound_sms_by_id_invalid_id_returns_404(admin_request, sample_service): assert admin_request.get( 'inbound_sms.get_inbound_by_id', endpoint_kwargs={ 'service_id': sample_service.id, - 'inbound_sms_id': 'dsadsda' + 'inbound_sms_id': 'bar' }, - expected_status=400 + expected_status=404 + ) + + +def test_get_inbound_sms_by_id_with_invalid_service_id_returns_404(admin_request, sample_service): + assert admin_request.get( + 'inbound_sms.get_inbound_by_id', + endpoint_kwargs={ + 'service_id': 'foo', + 'inbound_sms_id': '2cfbd6a1-1575-4664-8969-f27be0ea40d9' + }, + expected_status=404 )