Merge pull request #2548 from alphagov/mou-recipts

tell people when they've signed an MOU

app/config.py

@@ -151,6 +151,10 @@ class Config(object):
     TEAM_MEMBER_EDIT_EMAIL_TEMPLATE_ID = 'c73f1d71-4049-46d5-a647-d013bdeca3f0'
     TEAM_MEMBER_EDIT_MOBILE_TEMPLATE_ID = '8a31520f-4751-4789-8ea1-fe54496725eb'
     REPLY_TO_EMAIL_ADDRESS_VERIFICATION_TEMPLATE_ID = 'a42f1d17-9404-46d5-a647-d013bdfca3e1'
+    MOU_SIGNER_RECEIPT_TEMPLATE_ID = '4fd2e43c-309b-4e50-8fb8-1955852d9d71'
+    MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID = 'c20206d5-bf03-4002-9a90-37d5032d9e84'
+    MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID = '522b6657-5ca5-4368-a294-6b527703bd0b'
+    MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID = 'd0e66c4c-0c50-43f0-94f5-f85b613202d4'
 
     BROKER_URL = 'sqs://'
     BROKER_TRANSPORT_OPTIONS = {

app/dao/organisation_dao.py

@@ -64,7 +64,7 @@ def dao_update_organisation(organisation_id, **kwargs):
 
     domains = kwargs.pop('domains', None)
 
-    organisation = Organisation.query.filter_by(id=organisation_id).update(
+    num_updated = Organisation.query.filter_by(id=organisation_id).update(
         kwargs
     )
 
@@ -79,7 +79,7 @@ def dao_update_organisation(organisation_id, **kwargs):
 
         db.session.commit()
 
-    return organisation
+    return num_updated
 
 
 @transactional

app/models.py

@@ -346,6 +346,7 @@ class Organisation(db.Model):
         db.ForeignKey('users.id'),
         nullable=True,
     )
+    agreement_signed_by = db.relationship('User')
     agreement_signed_on_behalf_of_name = db.Column(db.String(255), nullable=True)
     agreement_signed_on_behalf_of_email_address = db.Column(db.String(255), nullable=True)
     agreement_signed_version = db.Column(db.Float, nullable=True)

app/organisation/rest.py

@@ -1,6 +1,7 @@
 from flask import abort, Blueprint, jsonify, request, current_app
 from sqlalchemy.exc import IntegrityError
 
+from app.config import QueueNames
 from app.dao.organisation_dao import (
     dao_create_organisation,
     dao_get_organisations,
@@ -12,9 +13,11 @@ from app.dao.organisation_dao import (
     dao_get_users_for_organisation,
     dao_add_user_to_organisation
 )
+from app.dao.templates_dao import dao_get_template_by_id
 from app.dao.services_dao import dao_fetch_service_by_id
 from app.errors import register_errors, InvalidRequest
-from app.models import Organisation
+from app.models import Organisation, KEY_TYPE_NORMAL
+from app.notifications.process_notifications import persist_notification, send_notification_to_queue
 from app.organisation.organisation_schema import (
     post_create_organisation_schema,
     post_update_organisation_schema,
@@ -91,6 +94,12 @@ def update_organisation(organisation_id):
     data = request.get_json()
     validate(data, post_update_organisation_schema)
     result = dao_update_organisation(organisation_id, **data)
+
+    if data.get('agreement_signed') is True:
+        # if a platform admin has manually adjusted the organisation, don't tell people
+        if data.get('agreement_signed_by_id'):
+            send_notifications_on_mou_signed(organisation_id)
+
     if result:
         return '', 204
     else:
@@ -149,3 +158,64 @@ def check_request_args(request):
     if errors:
         raise InvalidRequest(errors, status_code=400)
     return org_id, name
+
+
+def send_notifications_on_mou_signed(organisation_id):
+    organisation = dao_get_organisation_by_id(organisation_id)
+    notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID'])
+
+    def _send_notification(template_id, recipient, personalisation):
+        template = dao_get_template_by_id(template_id)
+
+        saved_notification = persist_notification(
+            template_id=template.id,
+            template_version=template.version,
+            recipient=recipient,
+            service=notify_service,
+            personalisation=personalisation,
+            notification_type=template.template_type,
+            api_key_id=None,
+            key_type=KEY_TYPE_NORMAL,
+            reply_to_text=notify_service.get_default_reply_to_email_address()
+        )
+        send_notification_to_queue(saved_notification, research_mode=False, queue=QueueNames.NOTIFY)
+
+    personalisation = {
+        'mou_link': '{}/agreement/{}.pdf'.format(
+            current_app.config['ADMIN_BASE_URL'],
+            'crown' if organisation.crown else 'non-crown'
+        ),
+        'org_name': organisation.name,
+        'org_dashboard_link': '{}/organisations/{}'.format(
+            current_app.config['ADMIN_BASE_URL'],
+            organisation.id
+        ),
+        'signed_by_name': organisation.agreement_signed_by.name,
+        'on_behalf_of_name': organisation.agreement_signed_on_behalf_of_name
+    }
+
+    # let notify team know something's happened
+    _send_notification(
+        current_app.config['MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID'],
+        'notify-support+{}@digital.cabinet-office.gov.uk'.format(current_app.config['NOTIFY_ENVIRONMENT']),
+        personalisation
+    )
+
+    if not organisation.agreement_signed_on_behalf_of_email_address:
+        signer_template_id = 'MOU_SIGNER_RECEIPT_TEMPLATE_ID'
+    else:
+        signer_template_id = 'MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID'
+
+        # let the person who has been signed on behalf of know.
+        _send_notification(
+            current_app.config['MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID'],
+            organisation.agreement_signed_on_behalf_of_email_address,
+            personalisation
+        )
+
+    # let the person who signed know - the template is different depending on if they signed on behalf of someone
+    _send_notification(
+        current_app.config[signer_template_id],
+        organisation.agreement_signed_by.email_address,
+        personalisation
+    )

tests/app/conftest.py

@@ -1001,6 +1001,35 @@ def change_email_confirmation_template(notify_db,
     return template
 
 
+@pytest.fixture(scope='function')
+def mou_signed_templates(notify_db, notify_db_session):
+    service, user = notify_service(notify_db, notify_db_session)
+    import importlib
+    alembic_script = importlib.import_module('migrations.versions.0298_add_mou_signed_receipt')
+
+    return {
+        config_name: create_custom_template(
+            service,
+            user,
+            config_name,
+            'email',
+            content='\n'.join(
+                next(
+                    x
+                    for x in alembic_script.templates
+                    if x['id'] == current_app.config[config_name]
+                )['content_lines']
+            ),
+        )
+        for config_name in [
+            'MOU_SIGNER_RECEIPT_TEMPLATE_ID',
+            'MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID',
+            'MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID',
+            'MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID',
+        ]
+    }
+
+
 def create_custom_template(service, user, template_config_name, template_type, content='', subject=None):
     template = Template.query.get(current_app.config[template_config_name])
     if not template:

tests/app/organisation/test_rest.py

@@ -222,29 +222,21 @@ def test_post_create_organisation_with_missing_name_gives_validation_error(
     assert response['errors'][0]['message'] == expected_error
 
 
-@pytest.mark.parametrize('agreement_signed', (
-    None, True, False
-))
 @pytest.mark.parametrize('crown', (
     None, True, False
 ))
 def test_post_update_organisation_updates_fields(
     admin_request,
     notify_db_session,
-    agreement_signed,
     crown,
 ):
     org = create_organisation()
     data = {
         'name': 'new organisation name',
         'active': False,
-        'agreement_signed': agreement_signed,
         'crown': crown,
-        'agreement_signed_on_behalf_of_name': 'Firstname Lastname',
-        'agreement_signed_on_behalf_of_email_address': 'test@example.com',
         'organisation_type': 'central',
     }
-    assert org.agreement_signed is None
     assert org.crown is None
 
     admin_request.post(
@@ -260,11 +252,8 @@ def test_post_update_organisation_updates_fields(
     assert organisation[0].id == org.id
     assert organisation[0].name == data['name']
     assert organisation[0].active == data['active']
-    assert organisation[0].agreement_signed == agreement_signed
     assert organisation[0].crown == crown
     assert organisation[0].domains == []
-    assert organisation[0].agreement_signed_on_behalf_of_name == 'Firstname Lastname'
-    assert organisation[0].agreement_signed_on_behalf_of_email_address == 'test@example.com'
     assert organisation[0].organisation_type == 'central'
 
 
@@ -308,7 +297,7 @@ def test_update_other_organisation_attributes_doesnt_clear_domains(
     admin_request.post(
         'organisation.update_organisation',
         _data={
-            'agreement_signed': True,
+            'crown': True,
         },
         organisation_id=org.id,
         _expected_status=204
@@ -398,6 +387,79 @@ def test_post_update_organisation_returns_400_if_domain_is_duplicate(admin_reque
     assert response['message'] == 'Domain already exists'
 
 
+def test_post_update_organisation_set_mou_doesnt_email_if_no_signed_by(
+    sample_organisation,
+    admin_request,
+    mocker
+):
+    queue_mock = mocker.patch('app.organisation.rest.send_notification_to_queue')
+
+    data = {'agreement_signed': True}
+
+    admin_request.post(
+        'organisation.update_organisation',
+        _data=data,
+        organisation_id=sample_organisation.id,
+        _expected_status=204
+    )
+
+    assert queue_mock.called is False
+
+
+@pytest.mark.parametrize('on_behalf_of_name, on_behalf_of_email_address, templates_and_recipients', [
+    (
+        None,
+        None,
+        {
+            'MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID': 'notify-support+test@digital.cabinet-office.gov.uk',
+            'MOU_SIGNER_RECEIPT_TEMPLATE_ID': 'notify@digital.cabinet-office.gov.uk',
+        }
+    ),
+    (
+        'Important Person',
+        'important@person.com',
+        {
+            'MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID': 'notify-support+test@digital.cabinet-office.gov.uk',
+            'MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID': 'important@person.com',
+            'MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID': 'notify@digital.cabinet-office.gov.uk',
+        }
+    ),
+])
+def test_post_update_organisation_set_mou_emails_signed_by(
+    sample_organisation,
+    admin_request,
+    mou_signed_templates,
+    mocker,
+    sample_user,
+    on_behalf_of_name,
+    on_behalf_of_email_address,
+    templates_and_recipients
+):
+    queue_mock = mocker.patch('app.organisation.rest.send_notification_to_queue')
+    sample_organisation.agreement_signed_on_behalf_of_name = on_behalf_of_name
+    sample_organisation.agreement_signed_on_behalf_of_email_address = on_behalf_of_email_address
+
+    admin_request.post(
+        'organisation.update_organisation',
+        _data={'agreement_signed': True, 'agreement_signed_by_id': str(sample_user.id)},
+        organisation_id=sample_organisation.id,
+        _expected_status=204
+    )
+
+    notifications = [x[0][0] for x in queue_mock.call_args_list]
+    assert {n.template.name: n.to for n in notifications} == templates_and_recipients
+
+    for n in notifications:
+        # we pass in the same personalisation for all templates (though some templates don't use all fields)
+        assert n.personalisation == {
+            'mou_link': 'http://localhost:6012/agreement/non-crown.pdf',
+            'org_name': 'sample organisation',
+            'org_dashboard_link': 'http://localhost:6012/organisations/{}'.format(sample_organisation.id),
+            'signed_by_name': 'Test User',
+            'on_behalf_of_name': on_behalf_of_name
+        }
+
+
 def test_post_link_service_to_organisation(admin_request, sample_service, sample_organisation):
     data = {
         'service_id': str(sample_service.id)