From 1fa1ebb96f5c0030340fa2a35c5ccbf6937ff898 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 4 Oct 2017 11:26:27 +0100 Subject: [PATCH 1/4] Add endpoint to return all letter contact blocks Added the GET '//letter-contact' endpoint for returning all letter contact blocks for a service. Updated the DAO for service letter contacts to return the default letter contact first. --- app/dao/service_letter_contact_dao.py | 5 +- app/service/rest.py | 8 ++- .../dao/test_service_letter_contact_dao.py | 26 ++++---- tests/app/service/test_rest.py | 59 ++++++++++++++++++- 4 files changed, 83 insertions(+), 15 deletions(-) diff --git a/app/dao/service_letter_contact_dao.py b/app/dao/service_letter_contact_dao.py index 2d0430299..fba378afd 100644 --- a/app/dao/service_letter_contact_dao.py +++ b/app/dao/service_letter_contact_dao.py @@ -1,3 +1,5 @@ +from sqlalchemy import desc + from app import db from app.dao.dao_utils import transactional from app.errors import InvalidRequest @@ -10,7 +12,8 @@ def dao_get_letter_contacts_by_service_id(service_id): ).filter( ServiceLetterContact.service_id == service_id ).order_by( - ServiceLetterContact.created_at + desc(ServiceLetterContact.is_default), + desc(ServiceLetterContact.created_at) ).all() return letter_contacts diff --git a/app/service/rest.py b/app/service/rest.py index c4a2d9e59..05952dd9d 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -53,7 +53,7 @@ from app.dao.service_email_reply_to_dao import ( dao_get_reply_to_by_service_id, update_reply_to_email_address ) -from app.dao.service_letter_contact_dao import create_or_update_letter_contact +from app.dao.service_letter_contact_dao import dao_get_letter_contacts_by_service_id, create_or_update_letter_contact from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( @@ -571,6 +571,12 @@ def update_service_reply_to_email_address(service_id, reply_to_email_id): return jsonify(data=new_reply_to.serialize()), 200 +@service_blueprint.route('//letter-contact', methods=["GET"]) +def get_letter_contacts(service_id): + result = dao_get_letter_contacts_by_service_id(service_id) + return jsonify([i.serialize() for i in result]), 200 + + @service_blueprint.route('/unique', methods=["GET"]) def is_service_name_unique(): name, email_from = check_request_args(request) diff --git a/tests/app/dao/test_service_letter_contact_dao.py b/tests/app/dao/test_service_letter_contact_dao.py index 5af184547..cad3ace84 100644 --- a/tests/app/dao/test_service_letter_contact_dao.py +++ b/tests/app/dao/test_service_letter_contact_dao.py @@ -14,13 +14,15 @@ from tests.app.db import create_letter_contact, create_service def test_dao_get_letter_contacts_by_service_id(notify_db_session): service = create_service() default_letter_contact = create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - another_letter_contact = create_letter_contact(service=service, contact_block='Cardiff, CA1 2DB') + second_letter_contact = create_letter_contact(service=service, contact_block='Cardiff, CA1 2DB', is_default=False) + third_letter_contact = create_letter_contact(service=service, contact_block='London, E1 8QS', is_default=False) results = dao_get_letter_contacts_by_service_id(service_id=service.id) - assert len(results) == 2 - assert default_letter_contact in results - assert another_letter_contact in results + assert len(results) == 3 + assert default_letter_contact == results[0] + assert third_letter_contact == results[1] + assert second_letter_contact == results[2] def test_create_or_update_letter_contact_creates_new_entry(notify_db_session): @@ -105,11 +107,11 @@ def test_add_another_letter_contact_as_default_overrides_existing(notify_db_sess assert len(results) == 2 - assert results[0].contact_block == 'Edinburgh, ED1 1AA' - assert not results[0].is_default + assert results[0].contact_block == 'Swansea, SN1 3CC' + assert results[0].is_default - assert results[1].contact_block == 'Swansea, SN1 3CC' - assert results[1].is_default + assert results[1].contact_block == 'Edinburgh, ED1 1AA' + assert not results[1].is_default def test_add_letter_contact_does_not_override_default(notify_db_session): @@ -182,11 +184,11 @@ def test_update_letter_contact_as_default_overides_existing_default(notify_db_se results = dao_get_letter_contacts_by_service_id(service_id=service.id) assert len(results) == 2 - assert results[0].contact_block == 'Aberdeen, AB12 23X' - assert not results[0].is_default + assert results[0].contact_block == 'Warwick, W14 TSR' + assert results[0].is_default - assert results[1].contact_block == 'Warwick, W14 TSR' - assert results[1].is_default + assert results[1].contact_block == 'Aberdeen, AB12 23X' + assert not results[1].is_default def test_update_letter_contact_unset_default_for_only_letter_contact_raises_exception(notify_db_session): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c41b4ff13..a773be57a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -25,7 +25,13 @@ from tests.app.conftest import ( sample_notification_history as create_notification_history, sample_notification_with_job ) -from tests.app.db import create_template, create_service_inbound_api, create_notification, create_reply_to_email +from tests.app.db import ( + create_template, + create_service_inbound_api, + create_notification, + create_reply_to_email, + create_letter_contact, +) from tests.app.db import create_user @@ -2320,3 +2326,54 @@ def test_update_service_letter_contact_upserts_letter_contact(admin_request, sam assert letter_contacts[0].contact_block == 'Aberdeen, AB23 1XH' assert letter_contacts[0].is_default assert response['data']['letter_contact_block'] == 'Aberdeen, AB23 1XH' + + +def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_service): + response = client.get('/service/{}/letter-contact'.format(sample_service.id), + headers=[create_authorization_header()]) + + assert json.loads(response.get_data(as_text=True)) == [] + assert response.status_code == 200 + + +def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_db_session): + service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + letter_contact = create_letter_contact(service, 'Aberdeen, AB23 1XH') + + response = client.get('/service/{}/letter-contact'.format(service.id), + headers=[create_authorization_header()]) + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response) == 1 + assert json_response[0]['contact_block'] == 'Aberdeen, AB23 1XH' + assert json_response[0]['is_default'] + assert json_response[0]['created_at'] + assert not json_response[0]['updated_at'] + assert response.status_code == 200 + + +def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, notify_db_session): + service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + letter_contact_a = create_letter_contact(service, 'Aberdeen, AB23 1XH') + letter_contact_b = create_letter_contact(service, 'London, E1 8QS', False) + + response = client.get('/service/{}/letter-contact'.format(service.id), + headers=[create_authorization_header()]) + json_response = json.loads(response.get_data(as_text=True)) + + assert len(json_response) == 2 + assert response.status_code == 200 + + assert json_response[0]['id'] == str(letter_contact_a.id) + assert json_response[0]['service_id'] == str(letter_contact_a.service_id) + assert json_response[0]['contact_block'] == 'Aberdeen, AB23 1XH' + assert json_response[0]['is_default'] + assert json_response[0]['created_at'] + assert not json_response[0]['updated_at'] + + assert json_response[1]['id'] == str(letter_contact_b.id) + assert json_response[1]['service_id'] == str(letter_contact_b.service_id) + assert json_response[1]['contact_block'] == 'London, E1 8QS' + assert not json_response[1]['is_default'] + assert json_response[1]['created_at'] + assert not json_response[1]['updated_at'] From 212ce302c8cd8eb16cf104335f0cdda1eaeb93d2 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 4 Oct 2017 12:26:38 +0100 Subject: [PATCH 2/4] Add get letter contact by id endpoint Added the GET //letter-contact/ endpoint for returning a letter contact for a service with the letter contact id Added the DAO for returning a letter contact by id --- app/dao/service_letter_contact_dao.py | 10 ++++++++++ app/service/rest.py | 12 ++++++++++- .../dao/test_service_letter_contact_dao.py | 20 +++++++++++++++++++ tests/app/service/test_rest.py | 20 +++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/app/dao/service_letter_contact_dao.py b/app/dao/service_letter_contact_dao.py index fba378afd..a9c64efc0 100644 --- a/app/dao/service_letter_contact_dao.py +++ b/app/dao/service_letter_contact_dao.py @@ -19,6 +19,16 @@ def dao_get_letter_contacts_by_service_id(service_id): return letter_contacts +def dao_get_letter_contact_by_id(service_id, letter_contact_id): + letter_contact = db.session.query( + ServiceLetterContact + ).filter( + ServiceLetterContact.service_id == service_id, + ServiceLetterContact.id == letter_contact_id + ).one() + return letter_contact + + def create_or_update_letter_contact(service_id, contact_block): letter_contacts = dao_get_letter_contacts_by_service_id(service_id) if len(letter_contacts) == 0: diff --git a/app/service/rest.py b/app/service/rest.py index 05952dd9d..e3f6ac578 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -53,7 +53,11 @@ from app.dao.service_email_reply_to_dao import ( dao_get_reply_to_by_service_id, update_reply_to_email_address ) -from app.dao.service_letter_contact_dao import dao_get_letter_contacts_by_service_id, create_or_update_letter_contact +from app.dao.service_letter_contact_dao import ( + dao_get_letter_contacts_by_service_id, + create_or_update_letter_contact, + dao_get_letter_contact_by_id +) from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( @@ -577,6 +581,12 @@ def get_letter_contacts(service_id): return jsonify([i.serialize() for i in result]), 200 +@service_blueprint.route('//letter-contact/', methods=["GET"]) +def get_letter_contact_by_id(service_id, letter_contact_id): + result = dao_get_letter_contact_by_id(service_id=service_id, letter_contact_id=letter_contact_id) + return jsonify(result.serialize()), 200 + + @service_blueprint.route('/unique', methods=["GET"]) def is_service_name_unique(): name, email_from = check_request_args(request) diff --git a/tests/app/dao/test_service_letter_contact_dao.py b/tests/app/dao/test_service_letter_contact_dao.py index cad3ace84..1429a566e 100644 --- a/tests/app/dao/test_service_letter_contact_dao.py +++ b/tests/app/dao/test_service_letter_contact_dao.py @@ -1,9 +1,12 @@ +import uuid import pytest +from sqlalchemy.exc import SQLAlchemyError from app.dao.service_letter_contact_dao import ( add_letter_contact_for_service, create_or_update_letter_contact, dao_get_letter_contacts_by_service_id, + dao_get_letter_contact_by_id, update_letter_contact ) from app.errors import InvalidRequest @@ -202,3 +205,20 @@ def test_update_letter_contact_unset_default_for_only_letter_contact_raises_exce contact_block='Warwick, W14 TSR', is_default=False ) + + +def test_dao_get_letter_contact_by_id(sample_service): + letter_contact = create_letter_contact(service=sample_service, contact_block='Aberdeen, AB12 23X') + result = dao_get_letter_contact_by_id(service_id=sample_service.id, letter_contact_id=letter_contact.id) + assert result == letter_contact + + +def test_dao_get_letter_contact_by_id_raises_sqlalchemy_error_when_letter_contact_does_not_exist(sample_service): + with pytest.raises(SQLAlchemyError): + dao_get_letter_contact_by_id(service_id=sample_service.id, letter_contact_id=uuid.uuid4()) + + +def test_dao_get_letter_contact_by_id_raises_sqlalchemy_error_when_service_does_not_exist(sample_service): + letter_contact = create_letter_contact(service=sample_service, contact_block='Some address') + with pytest.raises(SQLAlchemyError): + dao_get_letter_contact_by_id(service_id=uuid.uuid4(), letter_contact_id=letter_contact.id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a773be57a..63149e107 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2377,3 +2377,23 @@ def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, no assert not json_response[1]['is_default'] assert json_response[1]['created_at'] assert not json_response[1]['updated_at'] + + +def test_get_letter_contact_by_id(client, notify_db, notify_db_session): + service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + letter_contact = create_letter_contact(service, 'London, E1 8QS') + + response = client.get('/service/{}/letter-contact/{}'.format(service.id, letter_contact.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == letter_contact.serialize() + + +def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db, notify_db_session): + service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + + response = client.get('/service/{}/letter-contact/{}'.format(service.id, '93d59f88-4aa1-453c-9900-f61e2fc8a2de'), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 404 From 158e2ea1818624159d46f6a57f3f601b9bddc12d Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 4 Oct 2017 14:41:36 +0100 Subject: [PATCH 3/4] Add endpoint to add new letter contact block Added the '//letter-contact' service endpoint. --- app/service/rest.py | 19 +++++++++- app/service/service_senders_schema.py | 13 +++++++ tests/app/service/test_rest.py | 54 +++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index e3f6ac578..0b7ea5591 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -56,7 +56,8 @@ from app.dao.service_email_reply_to_dao import ( from app.dao.service_letter_contact_dao import ( dao_get_letter_contacts_by_service_id, create_or_update_letter_contact, - dao_get_letter_contact_by_id + dao_get_letter_contact_by_id, + add_letter_contact_for_service, ) from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id @@ -68,7 +69,10 @@ from app.models import Service, ServiceInboundApi from app.schema_validation import validate from app.service import statistics from app.service.service_inbound_api_schema import service_inbound_api, update_service_inbound_api_schema -from app.service.service_senders_schema import add_service_email_reply_to_request +from app.service.service_senders_schema import ( + add_service_email_reply_to_request, + add_service_letter_contact_block_request, +) from app.service.utils import get_whitelist_objects from app.service.sender import send_notification_to_service_users from app.service.send_notification import send_one_off_notification @@ -587,6 +591,17 @@ def get_letter_contact_by_id(service_id, letter_contact_id): return jsonify(result.serialize()), 200 +@service_blueprint.route('//letter-contact', methods=['POST']) +def add_service_letter_contact(service_id): + # validate the service exists, throws ResultNotFound exception. + dao_fetch_service_by_id(service_id) + form = validate(request.get_json(), add_service_letter_contact_block_request) + new_letter_contact = add_letter_contact_for_service(service_id=service_id, + contact_block=form['contact_block'], + is_default=form.get('is_default', True)) + return jsonify(data=new_letter_contact.serialize()), 201 + + @service_blueprint.route('/unique', methods=["GET"]) def is_service_name_unique(): name, email_from = check_request_args(request) diff --git a/app/service/service_senders_schema.py b/app/service/service_senders_schema.py index 57b689099..e0a07b0a4 100644 --- a/app/service/service_senders_schema.py +++ b/app/service/service_senders_schema.py @@ -9,3 +9,16 @@ add_service_email_reply_to_request = { }, "required": ["email_address", "is_default"] } + + +add_service_letter_contact_block_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST service letter contact block", + "type": "object", + "title": "Add new letter contact block for service", + "properties": { + "contact_block": {"type": "string"}, + "is_default": {"type": "boolean"} + }, + "required": ["contact_block", "is_default"] +} diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 63149e107..b972d68c7 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2397,3 +2397,57 @@ def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db headers=[('Content-Type', 'application/json'), create_authorization_header()]) assert response.status_code == 404 + + +def test_add_service_contact_block(client, sample_service): + data = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) + response = client.post('/service/{}/letter-contact'.format(sample_service.id), + data=data, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 201 + json_resp = json.loads(response.get_data(as_text=True)) + results = ServiceLetterContact.query.all() + assert len(results) == 1 + assert json_resp['data'] == results[0].serialize() + + +def test_add_service_letter_contact_can_add_multiple_addresses(client, sample_service): + first = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) + client.post('/service/{}/letter-contact'.format(sample_service.id), + data=first, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + second = json.dumps({"contact_block": "Aberdeen, AB23 1XH", "is_default": True}) + response = client.post('/service/{}/letter-contact'.format(sample_service.id), + data=second, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + assert response.status_code == 201 + json_resp = json.loads(response.get_data(as_text=True)) + results = ServiceLetterContact.query.all() + assert len(results) == 2 + default = [x for x in results if x.is_default] + assert json_resp['data'] == default[0].serialize() + first_letter_contact_not_default = [x for x in results if not x.is_default] + assert first_letter_contact_not_default[0].contact_block == 'London, E1 8QS' + + +def test_add_service_letter_contact_block_raise_exception_if_no_default(client, sample_service): + data = json.dumps({"contact_block": "London, E1 8QS", "is_default": False}) + response = client.post('/service/{}/letter-contact'.format(sample_service.id), + data=data, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + assert response.status_code == 400 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['message'] == 'You must have at least one letter contact as the default.' + + +def test_add_service_letter_contact_block_404s_when_invalid_service_id(client, notify_db, notify_db_session): + response = client.post('/service/{}/letter-contact'.format(uuid.uuid4()), + data={}, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 404 + result = json.loads(response.get_data(as_text=True)) + assert result['result'] == 'error' + assert result['message'] == 'No result found' From 5e2bf4cd32c520eb6978f3f367767a7907e539d0 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 4 Oct 2017 15:35:41 +0100 Subject: [PATCH 4/4] Add new POST //letter-contact/ end point --- app/service/rest.py | 13 ++++++++++++ tests/app/service/test_rest.py | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/app/service/rest.py b/app/service/rest.py index 0b7ea5591..e06d66dc9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -58,6 +58,7 @@ from app.dao.service_letter_contact_dao import ( create_or_update_letter_contact, dao_get_letter_contact_by_id, add_letter_contact_for_service, + update_letter_contact ) from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id @@ -602,6 +603,18 @@ def add_service_letter_contact(service_id): return jsonify(data=new_letter_contact.serialize()), 201 +@service_blueprint.route('//letter-contact/', methods=['POST']) +def update_service_letter_contact(service_id, letter_contact_id): + # validate the service exists, throws ResultNotFound exception. + dao_fetch_service_by_id(service_id) + form = validate(request.get_json(), add_service_letter_contact_block_request) + new_reply_to = update_letter_contact(service_id=service_id, + letter_contact_id=letter_contact_id, + contact_block=form['contact_block'], + is_default=form.get('is_default', True)) + return jsonify(data=new_reply_to.serialize()), 200 + + @service_blueprint.route('/unique', methods=["GET"]) def is_service_name_unique(): name, email_from = check_request_args(request) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b972d68c7..ba0465958 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2451,3 +2451,40 @@ def test_add_service_letter_contact_block_404s_when_invalid_service_id(client, n result = json.loads(response.get_data(as_text=True)) assert result['result'] == 'error' assert result['message'] == 'No result found' + + +def test_update_service_letter_contact(client, sample_service): + original_letter_contact = create_letter_contact(service=sample_service, contact_block="Aberdeen, AB23 1XH") + data = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) + response = client.post('/service/{}/letter-contact/{}'.format(sample_service.id, original_letter_contact.id), + data=data, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + results = ServiceLetterContact.query.all() + assert len(results) == 1 + assert json_resp['data'] == results[0].serialize() + + +def test_update_service_letter_contact_returns_400_when_no_default(client, sample_service): + original_reply_to = create_letter_contact(service=sample_service, contact_block="Aberdeen, AB23 1XH") + data = json.dumps({"contact_block": "London, E1 8QS", "is_default": False}) + response = client.post('/service/{}/letter-contact/{}'.format(sample_service.id, original_reply_to.id), + data=data, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 400 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['message'] == 'You must have at least one letter contact as the default.' + + +def test_update_service_letter_contact_returns_404_when_invalid_service_id(client, notify_db, notify_db_session): + response = client.post('/service/{}/letter-contact/{}'.format(uuid.uuid4(), uuid.uuid4()), + data={}, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 404 + result = json.loads(response.get_data(as_text=True)) + assert result['result'] == 'error' + assert result['message'] == 'No result found'