From a0c0dec42924ff2478dca275f499adaaf0914ce8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 8 Jul 2019 12:33:53 +0100 Subject: [PATCH] add tests include dynamically loading the template contents from the alembic upgrade file itself --- app/organisation/rest.py | 3 - tests/app/conftest.py | 29 ++++++++++ tests/app/organisation/test_rest.py | 86 +++++++++++++++++++++++++---- 3 files changed, 103 insertions(+), 15 deletions(-) diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 1d3da458b..cbc405025 100644 --- a/app/organisation/rest.py +++ b/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 ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 56ba4e099..2ed53996b 100644 --- a/tests/app/conftest.py +++ b/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: diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index e3bef936c..d016a67f7 100644 --- a/tests/app/organisation/test_rest.py +++ b/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)