add tests

include dynamically loading the template contents from the alembic upgrade file itself

app/organisation/rest.py

@@ -178,7 +178,6 @@ def send_notifications_on_mou_signed(organisation_id):
             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 = {
@@ -198,8 +197,6 @@ def send_notifications_on_mou_signed(organisation_id):
     # let notify team know something's happened
     _send_notification(
         current_app.config['MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID'],
-
-        # TODO: decide should we do this to distinguish emails from different envs?
         'notify-support+{}@digital.cabinet-office.gov.uk'.format(current_app.config['NOTIFY_ENVIRONMENT']),
         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)