Merge pull request #1277 from alphagov/imdad-katie-store-multiple-letter-contacts

[2/4] Upsert into ServiceLetterContact table when updating a service

app/dao/service_letter_contact_dao.py

@@ -0,0 +1,107 @@
+from app import db
+from app.dao.dao_utils import transactional
+from app.errors import InvalidRequest
+from app.models import ServiceLetterContact
+
+
+def dao_get_letter_contacts_by_service_id(service_id):
+    letter_contacts = db.session.query(
+        ServiceLetterContact
+    ).filter(
+        ServiceLetterContact.service_id == service_id
+    ).order_by(
+        ServiceLetterContact.created_at
+    ).all()
+
+    return letter_contacts
+
+
+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:
+        letter_contact = ServiceLetterContact(
+            service_id=service_id,
+            contact_block=contact_block
+        )
+        dao_create_letter_contact(letter_contact)
+    elif len(letter_contacts) == 1:
+        letter_contacts[0].contact_block = contact_block
+        dao_update_letter_contact(letter_contacts[0])
+    else:
+        # TODO: Once we move allowing letter contact blocks, this method will be removed
+        raise InvalidRequest(
+            "Multiple letter contacts were found, this method should not be used.",
+            status_code=500
+        )
+
+
+@transactional
+def dao_create_letter_contact(letter_contact):
+    db.session.add(letter_contact)
+
+
+@transactional
+def dao_update_letter_contact(letter_contact):
+    db.session.add(letter_contact)
+
+
+@transactional
+def add_letter_contact_for_service(service_id, contact_block, is_default):
+    old_default = _get_existing_default(service_id)
+    if is_default:
+        _reset_old_default_to_false(old_default)
+    else:
+        _raise_when_no_default(old_default)
+
+    new_letter_contact = ServiceLetterContact(
+        service_id=service_id,
+        contact_block=contact_block,
+        is_default=is_default
+    )
+    db.session.add(new_letter_contact)
+    return new_letter_contact
+
+
+@transactional
+def update_letter_contact(service_id, letter_contact_id, contact_block, is_default):
+    old_default = _get_existing_default(service_id)
+    # if we want to make this the default, ensure there are no other existing defaults
+    if is_default:
+        _reset_old_default_to_false(old_default)
+    else:
+        if old_default.id == letter_contact_id:
+            raise InvalidRequest("You must have at least one letter contact as the default.", 400)
+
+    letter_contact_update = ServiceLetterContact.query.get(letter_contact_id)
+    letter_contact_update.contact_block = contact_block
+    letter_contact_update.is_default = is_default
+    db.session.add(letter_contact_update)
+    return letter_contact_update
+
+
+def _get_existing_default(service_id):
+    letter_contacts = dao_get_letter_contacts_by_service_id(service_id=service_id)
+    if letter_contacts:
+        old_default = [x for x in letter_contacts if x.is_default]
+        if len(old_default) == 1:
+            return old_default[0]
+        else:
+            raise Exception(
+                "There should only be one default letter contact for each service. Service {} has {}".format(
+                    service_id,
+                    len(old_default)
+                )
+            )
+    return None
+
+
+def _reset_old_default_to_false(old_default):
+    if old_default:
+        old_default.is_default = False
+        db.session.add(old_default)
+
+
+def _raise_when_no_default(old_default):
+    # check that the update is not updating the only default to false
+    if not old_default:
+        raise InvalidRequest("You must have at least one letter contact as the default.", 400)

app/service/rest.py

@@ -46,8 +46,14 @@ from app.dao.service_whitelist_dao import (
     dao_add_and_commit_whitelisted_contacts,
     dao_remove_service_whitelist
 )
-from app.dao.service_email_reply_to_dao import create_or_update_email_reply_to, dao_get_reply_to_by_service_id, \
+from app.dao.service_email_reply_to_dao import (
-    add_reply_to_email_address_for_service, update_reply_to_email_address, dao_get_reply_to_by_id
+    add_reply_to_email_address_for_service,
+    create_or_update_email_reply_to,
+    dao_get_reply_to_by_id,
+    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.provider_statistics_dao import get_fragment_count
 from app.dao.users_dao import get_user_by_id
 from app.errors import (
@@ -144,15 +150,19 @@ def update_service(service_id):
     # Capture the status change here as Marshmallow changes this later
     service_going_live = fetched_service.restricted and not req_json.get('restricted', True)
 
-    if 'reply_to_email_address' in req_json:
-        create_or_update_email_reply_to(fetched_service.id, req_json['reply_to_email_address'])
-
     current_data = dict(service_schema.dump(fetched_service).data.items())
     current_data.update(request.get_json())
     update_dict = service_schema.load(current_data).data
+    dao_update_service(update_dict)
+
+    if 'reply_to_email_address' in req_json:
+        create_or_update_email_reply_to(fetched_service.id, req_json['reply_to_email_address'])
+
     if 'sms_sender' in req_json:
         insert_or_update_service_sms_sender(fetched_service, req_json['sms_sender'])
-    dao_update_service(update_dict)
+
+    if 'letter_contact_block' in req_json:
+        create_or_update_letter_contact(fetched_service.id, req_json['letter_contact_block'])
 
     if service_going_live:
         send_notification_to_service_users(

tests/app/dao/test_service_letter_contact_dao.py

@@ -0,0 +1,202 @@
+import pytest
+
+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,
+    update_letter_contact
+)
+from app.errors import InvalidRequest
+from app.models import ServiceLetterContact
+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')
+
+    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
+
+
+def test_create_or_update_letter_contact_creates_new_entry(notify_db_session):
+    service = create_service()
+
+    create_or_update_letter_contact(service.id, 'Cardiff, CA1 2DB')
+
+    letter_contacts = dao_get_letter_contacts_by_service_id(service.id)
+
+    assert ServiceLetterContact.query.count() == 1
+    assert letter_contacts[0].service.id == service.id
+    assert letter_contacts[0].contact_block == 'Cardiff, CA1 2DB'
+
+
+def test_create_or_update_letter_contact_does_not_create_another_entry(notify_db_session):
+    service = create_service()
+    create_letter_contact(service, 'London, NW1 2DB')
+    create_or_update_letter_contact(service.id, 'Bristol, BR1 2DB')
+
+    letter_contacts = dao_get_letter_contacts_by_service_id(service.id)
+
+    assert len(letter_contacts) == 1
+
+
+def test_create_or_update_letter_contact_updates_existing_entry(notify_db_session):
+    service = create_service()
+    create_letter_contact(service, 'London, NW1 2DB')
+
+    create_or_update_letter_contact(service.id, 'Bristol, BR1 2DB')
+
+    letter_contact = dao_get_letter_contacts_by_service_id(service.id)
+
+    assert len(letter_contact) == 1
+    assert letter_contact[0].service.id == service.id
+    assert letter_contact[0].contact_block == 'Bristol, BR1 2DB'
+
+
+def test_create_or_update_letter_contact_raises_exception_if_multiple_contact_blocks_exist(notify_db_session):
+    service = create_service()
+    create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA')
+    create_letter_contact(service=service, contact_block='Manchester, MA1 2BB', is_default=False)
+
+    with pytest.raises(expected_exception=InvalidRequest) as e:
+        create_or_update_letter_contact(service_id=service.id, contact_block='Swansea, SN1 3CC')
+    assert e.value.message == "Multiple letter contacts were found, this method should not be used."
+
+
+def test_create_or_update_letter_contact_raises_exception_if_multiple_letter_contacts_exist(notify_db_session):
+    service = create_service()
+    create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA')
+    create_letter_contact(service=service, contact_block='Manchester, MA1 2BB', is_default=False)
+
+    with pytest.raises(expected_exception=InvalidRequest) as e:
+        create_or_update_letter_contact(service_id=service.id, contact_block='Swansea, SN1 3CC')
+    assert e.value.message == "Multiple letter contacts were found, this method should not be used."
+
+
+def test_add_letter_contact_for_service_creates_additional_letter_contact_for_service(notify_db_session):
+    service = create_service()
+
+    create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA')
+    add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False)
+
+    results = dao_get_letter_contacts_by_service_id(service_id=service.id)
+
+    assert len(results) == 2
+
+    assert results[0].contact_block == 'Edinburgh, ED1 1AA'
+    assert results[0].is_default
+
+    assert results[1].contact_block == 'Swansea, SN1 3CC'
+    assert not results[1].is_default
+
+
+def test_add_another_letter_contact_as_default_overrides_existing(notify_db_session):
+    service = create_service()
+
+    create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA')
+    add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=True)
+
+    results = dao_get_letter_contacts_by_service_id(service_id=service.id)
+
+    assert len(results) == 2
+
+    assert results[0].contact_block == 'Edinburgh, ED1 1AA'
+    assert not results[0].is_default
+
+    assert results[1].contact_block == 'Swansea, SN1 3CC'
+    assert results[1].is_default
+
+
+def test_add_letter_contact_does_not_override_default(notify_db_session):
+    service = create_service()
+
+    add_letter_contact_for_service(service_id=service.id, contact_block='Edinburgh, ED1 1AA', is_default=True)
+    add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False)
+
+    results = dao_get_letter_contacts_by_service_id(service_id=service.id)
+
+    assert len(results) == 2
+
+    assert results[0].contact_block == 'Edinburgh, ED1 1AA'
+    assert results[0].is_default
+
+    assert results[1].contact_block == 'Swansea, SN1 3CC'
+    assert not results[1].is_default
+
+
+def test_add_letter_contact_with_no_default_raises_exception(notify_db_session):
+    service = create_service()
+    with pytest.raises(expected_exception=InvalidRequest):
+        add_letter_contact_for_service(
+            service_id=service.id,
+            contact_block='Swansea, SN1 3CC',
+            is_default=False
+        )
+
+
+def test_add_letter_contact_when_multiple_defaults_exist_raises_exception(notify_db_session):
+    service = create_service()
+    create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA')
+    create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X')
+
+    with pytest.raises(Exception):
+        add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False)
+
+
+def test_can_update_letter_contact(notify_db_session):
+    service = create_service()
+    letter_contact = create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X')
+
+    update_letter_contact(
+        service_id=service.id,
+        letter_contact_id=letter_contact.id,
+        contact_block='Warwick, W14 TSR',
+        is_default=True
+    )
+
+    updated_letter_contact = ServiceLetterContact.query.get(letter_contact.id)
+
+    assert updated_letter_contact.contact_block == 'Warwick, W14 TSR'
+    assert updated_letter_contact.updated_at
+    assert updated_letter_contact.is_default
+
+
+def test_update_letter_contact_as_default_overides_existing_default(notify_db_session):
+    service = create_service()
+
+    create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X')
+    second_letter_contact = create_letter_contact(service=service, contact_block='Swansea, SN1 3CC', is_default=False)
+
+    update_letter_contact(
+        service_id=service.id,
+        letter_contact_id=second_letter_contact.id,
+        contact_block='Warwick, W14 TSR',
+        is_default=True
+    )
+
+    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[1].contact_block == 'Warwick, W14 TSR'
+    assert results[1].is_default
+
+
+def test_update_letter_contact_unset_default_for_only_letter_contact_raises_exception(notify_db_session):
+    service = create_service()
+    only_letter_contact = create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X')
+
+    with pytest.raises(expected_exception=InvalidRequest):
+        update_letter_contact(
+            service_id=service.id,
+            letter_contact_id=only_letter_contact.id,
+            contact_block='Warwick, W14 TSR',
+            is_default=False
+        )

tests/app/service/test_rest.py

@@ -12,7 +12,7 @@ from app.dao.services_dao import dao_remove_user_from_service
 from app.dao.templates_dao import dao_redact_template
 from app.dao.users_dao import save_model_user
 from app.models import (
-    User, Organisation, Service, ServicePermission, Notification, ServiceEmailReplyTo,
+    User, Organisation, Service, ServicePermission, Notification, ServiceEmailReplyTo, ServiceLetterContact,
     DVLA_ORG_LAND_REGISTRY,
     KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST,
     EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE,
@@ -2304,3 +2304,21 @@ def test_get_email_reply_to_address(client, notify_db, notify_db_session):
 
     assert response.status_code == 200
     assert json.loads(response.get_data(as_text=True)) == reply_to.serialize()
+
+
+def test_update_service_letter_contact_upserts_letter_contact(admin_request, sample_service):
+    response = admin_request.post(
+        'service.update_service',
+        service_id=sample_service.id,
+        _data={
+            'letter_contact_block': 'Aberdeen, AB23 1XH'
+        },
+        _expected_status=200
+    )
+
+    letter_contacts = ServiceLetterContact.query.all()
+
+    assert len(letter_contacts) == 1
+    assert letter_contacts[0].contact_block == 'Aberdeen, AB23 1XH'
+    assert letter_contacts[0].is_default
+    assert response['data']['letter_contact_block'] == 'Aberdeen, AB23 1XH'