diff --git a/app/notifications/rest.py b/app/notifications/rest.py index a885fe2e3..7095d14d5 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -169,7 +169,7 @@ def _service_has_permission(service, notify_type): if notify_type not in [p.permission for p in service.permissions]: notify_type_text = notify_type + 's' action = 'send' - if notify_type == SMS_TYPE or notify_type == INBOUND_SMS_TYPE: + if notify_type in [SMS_TYPE, INBOUND_SMS_TYPE]: notify_type_text = 'text messages' if notify_type == INBOUND_SMS_TYPE: action = 'receive' diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 4a1da98ab..ffc6de428 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -10,7 +10,8 @@ from notifications_python_client.authentication import create_jwt_token import app from app.dao import notifications_dao from app.models import ( - ApiKey, INTERNATIONAL_SMS_TYPE, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory) + ApiKey, INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, + KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory) from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template from app.dao.services_dao import dao_update_service from app.dao.api_key_dao import save_model_api_key @@ -32,7 +33,7 @@ from app.errors import InvalidRequest @pytest.mark.parametrize('template_type', - ['sms', 'email']) + [SMS_TYPE, EMAIL_TYPE]) def test_create_notification_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker, template_type): with notify_api.test_request_context(): @@ -79,8 +80,8 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): @pytest.mark.parametrize('template_type, to', - [('sms', '+447700900855'), - ('email', 'ok@ok.com')]) + [(SMS_TYPE, '+447700900855'), + (EMAIL_TYPE, 'ok@ok.com')]) def test_send_notification_invalid_template_id(notify_api, sample_template, mocker, fake_uuid, template_type, to): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -217,8 +218,8 @@ def test_should_not_send_notification_for_archived_template(notify_api, sample_t @pytest.mark.parametrize('template_type, to', - [('sms', '+447700900855'), - ('email', 'not-someone-we-trust@email-address.com')]) + [(SMS_TYPE, '+447700900855'), + (EMAIL_TYPE, 'not-someone-we-trust@email-address.com')]) def test_should_not_send_notification_if_restricted_and_not_a_service_user(notify_api, sample_template, sample_email_template, @@ -228,7 +229,7 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif with notify_api.test_request_context(): with notify_api.test_client() as client: mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type)) - template = sample_template if template_type == 'sms' else sample_email_template + template = sample_template if template_type == SMS_TYPE else sample_email_template template.service.restricted = True dao_update_service(template.service) data = { @@ -254,7 +255,7 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif @pytest.mark.parametrize('template_type', - ['sms', 'email']) + [SMS_TYPE, EMAIL_TYPE]) def test_should_send_notification_if_restricted_and_a_service_user(notify_api, sample_template, sample_email_template, @@ -264,8 +265,8 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api, with notify_api.test_client() as client: mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type)) - template = sample_template if template_type == 'sms' else sample_email_template - to = template.service.created_by.mobile_number if template_type == 'sms' \ + template = sample_template if template_type == SMS_TYPE else sample_email_template + to = template.service.created_by.mobile_number if template_type == SMS_TYPE \ else template.service.created_by.email_address template.service.restricted = True dao_update_service(template.service) @@ -286,7 +287,7 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api, @pytest.mark.parametrize('template_type', - ['sms', 'email']) + [SMS_TYPE, EMAIL_TYPE]) def test_should_not_allow_template_from_another_service(notify_api, service_factory, sample_user, @@ -299,7 +300,7 @@ def test_should_not_allow_template_from_another_service(notify_api, service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2') service_2_templates = dao_get_all_templates_for_service(service_id=service_2.id) - to = sample_user.mobile_number if template_type == 'sms' else sample_user.email_address + to = sample_user.mobile_number if template_type == SMS_TYPE else sample_user.email_address data = { 'to': to, 'template': service_2_templates[0].id @@ -692,7 +693,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t @pytest.mark.parametrize('template_type', - ['sms', 'email']) + [SMS_TYPE, EMAIL_TYPE]) def test_should_persist_notification(notify_api, sample_template, sample_email_template, template_type, @@ -700,8 +701,8 @@ def test_should_persist_notification(notify_api, sample_template, with notify_api.test_request_context(), notify_api.test_client() as client: mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type)) mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) - template = sample_template if template_type == 'sms' else sample_email_template - to = sample_template.service.created_by.mobile_number if template_type == 'sms' \ + template = sample_template if template_type == SMS_TYPE else sample_email_template + to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \ else sample_email_template.service.created_by.email_address data = { 'to': to, @@ -730,7 +731,7 @@ def test_should_persist_notification(notify_api, sample_template, @pytest.mark.parametrize('template_type', - ['sms', 'email']) + [SMS_TYPE, EMAIL_TYPE]) def test_should_delete_notification_and_return_error_if_sqs_fails( client, sample_email_template, @@ -743,8 +744,8 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( side_effect=Exception("failed to talk to SQS") ) mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid) - template = sample_template if template_type == 'sms' else sample_email_template - to = sample_template.service.created_by.mobile_number if template_type == 'sms' \ + template = sample_template if template_type == SMS_TYPE else sample_email_template + to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \ else sample_email_template.service.created_by.email_address data = { 'to': to, @@ -833,8 +834,8 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( KEY_TYPE_NORMAL, KEY_TYPE_TEAM ]) @pytest.mark.parametrize('notification_type, to, _create_sample_template', [ - ('sms', '07827992635', create_sample_template), - ('email', 'non_whitelist_recipient@mail.com', create_sample_email_template)] + (SMS_TYPE, '07827992635', create_sample_template), + (EMAIL_TYPE, 'non_whitelist_recipient@mail.com', create_sample_email_template)] ) def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( client, @@ -888,8 +889,8 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( KEY_TYPE_NORMAL, KEY_TYPE_TEAM ]) @pytest.mark.parametrize('notification_type, to, _create_sample_template', [ - ('sms', '07123123123', create_sample_template), - ('email', 'whitelist_recipient@mail.com', create_sample_email_template)] + (SMS_TYPE, '07123123123', create_sample_template), + (EMAIL_TYPE, 'whitelist_recipient@mail.com', create_sample_email_template)] ) def test_should_send_notification_to_whitelist_recipient( client, @@ -905,10 +906,10 @@ def test_should_send_notification_to_whitelist_recipient( service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=service_restricted) apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) template = _create_sample_template(notify_db, notify_db_session, service=service) - if notification_type == 'sms': + if notification_type == SMS_TYPE: service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, mobile_number=to) - elif notification_type == 'email': + elif notification_type == EMAIL_TYPE: service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, email_address=to) @@ -940,8 +941,8 @@ def test_should_send_notification_to_whitelist_recipient( @pytest.mark.parametrize( 'notification_type, template_type, to', [ - ('email', 'sms', 'notify@digital.cabinet-office.gov.uk'), - ('sms', 'email', '+447700900986') + (EMAIL_TYPE, SMS_TYPE, 'notify@digital.cabinet-office.gov.uk'), + (SMS_TYPE, EMAIL_TYPE, '+447700900986') ]) def test_should_error_if_notification_type_does_not_match_template_type( client, @@ -987,8 +988,8 @@ def test_create_template_doesnt_raise_with_too_much_personalisation( @pytest.mark.parametrize( 'template_type, should_error', [ - ('sms', True), - ('email', False) + (SMS_TYPE, True), + (EMAIL_TYPE, False) ] ) def test_create_template_raises_invalid_request_when_content_too_large( @@ -1150,7 +1151,7 @@ def test_should_not_allow_international_number_on_sms_notification(client, sampl def test_should_allow_international_number_on_sms_notification(client, notify_db, notify_db_session, mocker): mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE, 'sms']) + service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE, SMS_TYPE]) template = create_sample_template(notify_db, notify_db_session, service=service) data = { diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index db651532f..94e465895 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -2,6 +2,7 @@ import pytest from freezegun import freeze_time from flask import current_app import app +from app.models import INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE from app.notifications.validators import ( check_service_over_daily_message_limit, check_template_is_for_notification_type, @@ -116,16 +117,16 @@ def test_check_service_message_limit_in_cache_over_message_limit_fails( @pytest.mark.parametrize('template_type, notification_type', - [('email', 'email'), - ('sms', 'sms')]) + [(EMAIL_TYPE, EMAIL_TYPE), + (SMS_TYPE, SMS_TYPE)]) def test_check_template_is_for_notification_type_pass(template_type, notification_type): assert check_template_is_for_notification_type(notification_type=notification_type, template_type=template_type) is None @pytest.mark.parametrize('template_type, notification_type', - [('sms', 'email'), - ('email', 'sms')]) + [(SMS_TYPE, EMAIL_TYPE), + (EMAIL_TYPE, SMS_TYPE)]) def test_check_template_is_for_notification_type_fails_when_template_type_does_not_match_notification_type( template_type, notification_type): with pytest.raises(BadRequestError) as e: @@ -309,7 +310,7 @@ def test_should_not_rate_limit_if_limiting_is_disabled( @pytest.mark.parametrize('key_type', ['test', 'normal']) def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type): with pytest.raises(BadRequestError) as e: - validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, 'sms') + validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, SMS_TYPE) assert e.value.status_code == 400 assert e.value.message == 'Cannot send to international mobile numbers' assert e.value.fields == [] @@ -318,6 +319,6 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_ @pytest.mark.parametrize('key_type', ['test', 'normal']) def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms( key_type, notify_db, notify_db_session): - service = create_service(notify_db, notify_db_session, permissions=['sms', 'international_sms']) - result = validate_and_format_recipient('20-12-1234-1234', key_type, service, 'sms') + service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE]) + result = validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE) assert result == '201212341234' diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 0eea25672..7b0478916 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -479,7 +479,7 @@ def test_update_service_flags(client, sample_service): data = { 'research_mode': True, - 'permissions': ['letter', 'international_sms'] + 'permissions': [LETTER_TYPE, INTERNATIONAL_SMS_TYPE] } auth_header = create_authorization_header() @@ -492,7 +492,7 @@ def test_update_service_flags(client, sample_service): result = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 200 assert result['data']['research_mode'] is True - assert set(result['data']['permissions']) == set(['letter', 'international_sms']) + assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) @pytest.fixture(scope='function') @@ -503,7 +503,7 @@ def service_with_no_permissions(notify_db, notify_db_session): def test_update_service_flags_with_service_without_default_service_permissions(client, service_with_no_permissions): auth_header = create_authorization_header() data = { - 'permissions': ['letter', 'international_sms'], + 'permissions': [LETTER_TYPE, INTERNATIONAL_SMS_TYPE], } resp = client.post( @@ -537,7 +537,7 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db, result = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 200 - assert 'international_sms' not in result['data']['permissions'] + assert INTERNATIONAL_SMS_TYPE not in result['data']['permissions'] permissions = ServicePermission.query.filter_by(service_id=service.id).all() assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) @@ -1454,8 +1454,8 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s service = json.loads(resp.get_data(as_text=True))['data'] assert service['id'] == str(sample_service.id) assert 'statistics' in service.keys() - assert set(service['statistics'].keys()) == {'sms', 'email', 'letter'} - assert service['statistics']['sms'] == stats + assert set(service['statistics'].keys()) == {SMS_TYPE, EMAIL_TYPE, LETTER_TYPE} + assert service['statistics'][SMS_TYPE] == stats @pytest.mark.parametrize( @@ -1507,9 +1507,9 @@ def test_get_services_with_detailed_flag(client, notify_db, notify_db_session): assert data[0]['name'] == 'Sample service' assert data[0]['id'] == str(notifications[0].service_id) assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 3}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 3}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1530,9 +1530,9 @@ def test_get_services_with_detailed_flag_excluding_from_test_key(notify_api, not data = json.loads(resp.get_data(as_text=True))['data'] assert len(data) == 1 assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 2}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1586,15 +1586,15 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): assert len(data) == 2 assert data[0]['id'] == str(service_1.id) assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 1, 'failed': 0, 'requested': 3}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 1, 'failed': 0, 'requested': 3}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } assert data[1]['id'] == str(service_2.id) assert data[1]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 1}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 1}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1613,15 +1613,15 @@ def test_get_detailed_services_includes_services_with_no_notifications(notify_db assert len(data) == 2 assert data[0]['id'] == str(service_1.id) assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 1}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 1}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } assert data[1]['id'] == str(service_2.id) assert data[1]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1638,9 +1638,9 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not assert len(data) == 1 assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 2}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1665,9 +1665,9 @@ def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_ assert len(data) == 1 assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 2}, - 'letter': {'delivered': 0, 'failed': 0, 'requested': 0} + EMAIL_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0}, + SMS_TYPE: {'delivered': 0, 'failed': 0, 'requested': 2}, + LETTER_TYPE: {'delivered': 0, 'failed': 0, 'requested': 0} } @@ -1761,7 +1761,7 @@ def test_get_template_stats_by_month_returns_error_for_incorrect_year( def test_get_yearly_billing_usage(client, notify_db, notify_db_session): - rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type='sms') + rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type=SMS_TYPE) notify_db.session.add(rate) notification = create_sample_notification(notify_db, notify_db_session, created_at=datetime(2016, 6, 5), sent_at=datetime(2016, 6, 5), @@ -1775,13 +1775,13 @@ def test_get_yearly_billing_usage(client, notify_db, notify_db_session): assert json.loads(response.get_data(as_text=True)) == [{'credits': 1, 'billing_units': 1, 'rate_multiplier': 1, - 'notification_type': 'sms', + 'notification_type': SMS_TYPE, 'international': False, 'rate': 0.0158}, {'credits': 0, 'billing_units': 0, 'rate_multiplier': 1, - 'notification_type': 'email', + 'notification_type': EMAIL_TYPE, 'international': False, 'rate': 0}] @@ -1798,7 +1798,7 @@ def test_get_yearly_billing_usage_returns_400_if_missing_year(client, sample_ser def test_get_monthly_billing_usage(client, notify_db, notify_db_session, sample_service): - rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type='sms') + rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type=SMS_TYPE) notify_db.session.add(rate) notification = create_sample_notification(notify_db, notify_db_session, created_at=datetime(2016, 6, 5), sent_at=datetime(2016, 6, 5), @@ -1810,7 +1810,7 @@ def test_get_monthly_billing_usage(client, notify_db, notify_db_session, sample_ sent_at=datetime(2016, 7, 5), status='sending') - template = create_template(sample_service, template_type='email') + template = create_template(sample_service, template_type=EMAIL_TYPE) create_sample_notification(notify_db, notify_db_session, created_at=datetime(2016, 6, 5), sent_at=datetime(2016, 6, 5), status='sending', @@ -1825,19 +1825,19 @@ def test_get_monthly_billing_usage(client, notify_db, notify_db_session, sample_ assert actual == [{'month': 'June', 'international': False, 'rate_multiplier': 1, - 'notification_type': 'sms', + 'notification_type': SMS_TYPE, 'rate': 0.0158, 'billing_units': 1}, {'month': 'June', 'international': False, 'rate_multiplier': 2, - 'notification_type': 'sms', + 'notification_type': SMS_TYPE, 'rate': 0.0158, 'billing_units': 1}, {'month': 'July', 'international': False, 'rate_multiplier': 1, - 'notification_type': 'sms', + 'notification_type': SMS_TYPE, 'rate': 0.0158, 'billing_units': 1}] @@ -1854,7 +1854,7 @@ def test_get_monthly_billing_usage_returns_400_if_missing_year(client, sample_se def test_get_monthly_billing_usage_returns_empty_list_if_no_notifications(client, notify_db, sample_service): - rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type='sms') + rate = Rate(id=uuid.uuid4(), valid_from=datetime(2016, 3, 31, 23, 00), rate=0.0158, notification_type=SMS_TYPE) notify_db.session.add(rate) response = client.get( '/service/{}/monthly-usage?year=2016'.format(sample_service.id),