From 3de0cc67ffb0d90a0532ef74a4ac6104a780f0e5 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 28 Oct 2019 16:31:47 +0000 Subject: [PATCH] [#299] Fix the fixtures for templates lacking certain permissions These fixtures were both calling other fixtures as functions and being called as functions in the tests. Rewriting the tests to make them Pytest 4 compatible means we are no longer using `sample_template_without_letter_permission`, so this has been deleted. --- tests/app/conftest.py | 15 +++--- .../rest/test_send_notification.py | 53 +++++++++++++------ tests/app/template/test_rest.py | 34 +++++++----- 3 files changed, 64 insertions(+), 38 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index c2ebb23cef..365fc86760 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -431,13 +431,9 @@ def sample_template( @pytest.fixture(scope='function') -def sample_template_without_sms_permission(notify_db, notify_db_session): - return sample_template(notify_db, notify_db_session, permissions=[EMAIL_TYPE]) - - -@pytest.fixture(scope='function') -def sample_template_without_letter_permission(notify_db, notify_db_session): - return sample_template(notify_db, notify_db_session, template_type="letter", permissions=[EMAIL_TYPE]) +def sample_template_without_sms_permission(notify_db_session): + service = create_service(service_permissions=[EMAIL_TYPE], check_if_service_exists=True) + return create_template(service, template_type=SMS_TYPE) @pytest.fixture(scope='function') @@ -485,8 +481,9 @@ def sample_email_template( @pytest.fixture(scope='function') -def sample_template_without_email_permission(notify_db, notify_db_session): - return sample_email_template(notify_db, notify_db_session, permissions=[SMS_TYPE]) +def sample_template_without_email_permission(notify_db_session): + service = create_service(service_permissions=[SMS_TYPE], check_if_service_exists=True) + return create_template(service, template_type=EMAIL_TYPE) @pytest.fixture diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 9f1fea8555..ba1fc3922e 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1159,26 +1159,49 @@ def test_should_allow_international_number_on_sms_notification(client, sample_se assert response.status_code == 201 -@pytest.mark.parametrize( - 'template_factory, to, expected_error', [ - (sample_template_without_sms_permission, '+16502532222', 'Cannot send text messages'), - (sample_template_without_email_permission, 'notify@digital.cabinet-office.gov.uk', 'Cannot send emails') - ]) -def test_should_not_allow_notification_if_service_permission_not_set( - client, notify_db, notify_db_session, mocker, template_factory, to, expected_error): - template_without_permission = template_factory(notify_db, notify_db_session) - mocked = mocker.patch( - 'app.celery.provider_tasks.deliver_{}.apply_async'.format(template_without_permission.template_type)) +def test_should_not_allow_sms_notifications_if_service_permission_not_set( + client, + mocker, + sample_template_without_sms_permission, +): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { - 'to': to, - 'template': str(template_without_permission.id) + 'to': '+447700900986', + 'template': str(sample_template_without_sms_permission.id) } - auth_header = create_authorization_header(service_id=template_without_permission.service_id) + auth_header = create_authorization_header(service_id=sample_template_without_sms_permission.service_id) response = client.post( - path='/notifications/{}'.format(template_without_permission.template_type), + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert not mocked.called + assert response.status_code == 400 + error_json = json.loads(response.get_data(as_text=True)) + + assert error_json['result'] == 'error' + assert error_json['message']['service'][0] == 'Cannot send text messages' + + +def test_should_not_allow_email_notifications_if_service_permission_not_set( + client, + mocker, + sample_template_without_email_permission, +): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + + data = { + 'to': 'notify@digital.cabinet-office.gov.uk', + 'template': str(sample_template_without_email_permission.id) + } + + auth_header = create_authorization_header(service_id=sample_template_without_email_permission.service_id) + + response = client.post( + path='/notifications/email', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) @@ -1187,7 +1210,7 @@ def test_should_not_allow_notification_if_service_permission_not_set( error_json = json.loads(response.get_data(as_text=True)) assert error_json['result'] == 'error' - assert error_json['message']['service'][0] == expected_error + assert error_json['message']['service'][0] == 'Cannot send emails' @pytest.mark.parametrize( diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 4af44b9b8b..bc5dff089b 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -24,11 +24,7 @@ from app.dao.service_permissions_dao import dao_add_service_permission from tests import create_authorization_header -from tests.app.conftest import ( - sample_template as create_sample_template, - sample_template_without_email_permission, - sample_template_without_letter_permission, - sample_template_without_sms_permission) +from tests.app.conftest import sample_template as create_sample_template from tests.app.db import ( create_service, create_letter_contact, create_template, create_notification, create_template_folder, @@ -248,14 +244,21 @@ def test_should_raise_error_on_create_if_no_permission( assert json_resp['message'] == expected_error -@pytest.mark.parametrize('template_factory, expected_error', [ - (sample_template_without_sms_permission, {'template_type': ['Updating text message templates is not allowed']}), - (sample_template_without_email_permission, {'template_type': ['Updating email templates is not allowed']}), - (sample_template_without_letter_permission, {'template_type': ['Updating letter templates is not allowed']}) +@pytest.mark.parametrize('template_type, permissions, expected_error', [ + (SMS_TYPE, [EMAIL_TYPE], {'template_type': ['Updating text message templates is not allowed']}), + (EMAIL_TYPE, [LETTER_TYPE], {'template_type': ['Updating email templates is not allowed']}), + (LETTER_TYPE, [SMS_TYPE], {'template_type': ['Updating letter templates is not allowed']}) ]) def test_should_be_error_on_update_if_no_permission( - client, sample_user, template_factory, expected_error, notify_db, notify_db_session): - template_without_permission = template_factory(notify_db, notify_db_session) + client, + sample_user, + notify_db_session, + template_type, + permissions, + expected_error, +): + service = create_service(service_permissions=permissions) + template_without_permission = create_template(service, template_type=template_type) data = { 'content': 'new template content', 'created_by': str(sample_user.id) @@ -644,7 +647,9 @@ def test_should_return_404_if_no_templates_for_service_with_id(client, sample_se def test_create_400_for_over_limit_content(client, notify_api, sample_user, sample_service, fake_uuid): - content = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(SMS_CHAR_COUNT_LIMIT + 1)) + content = ''.join( + random.choice(string.ascii_uppercase + string.digits) for _ in range(SMS_CHAR_COUNT_LIMIT + 1) # nosec + ) data = { 'name': 'too big template', 'template_type': SMS_TYPE, @@ -669,8 +674,9 @@ def test_create_400_for_over_limit_content(client, notify_api, sample_user, samp def test_update_400_for_over_limit_content(client, notify_api, sample_user, sample_template): json_data = json.dumps({ - 'content': ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range( - SMS_CHAR_COUNT_LIMIT + 1)), + 'content': ''.join( + random.choice(string.ascii_uppercase + string.digits) for _ in range(SMS_CHAR_COUNT_LIMIT + 1) # nosec + ), 'created_by': str(sample_user.id) }) auth_header = create_authorization_header()