diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5ae25c0e6..31b9e3300 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -35,11 +35,19 @@ from app.models import ( JobStatistics, SMS_TYPE, EMAIL_TYPE, - ServiceSmsSender) + INTERNATIONAL_SMS_TYPE, + ServiceSmsSender, +) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc +DEFAULT_SERVICE_PERMISSIONS = [ + SMS_TYPE, + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, +] + def dao_fetch_all_services(only_active=False): query = Service.query.order_by( @@ -146,13 +154,16 @@ def dao_fetch_service_by_id_and_user(service_id, user_id): @transactional @version_class(Service) -def dao_create_service(service, user, service_id=None, service_permissions=[SMS_TYPE, EMAIL_TYPE]): +def dao_create_service(service, user, service_id=None, service_permissions=None): # the default property does not appear to work when there is a difference between the sqlalchemy schema and the # db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender # migration is completed, this code should be able to be removed. if not service.sms_sender: service.sms_sender = current_app.config['FROM_NUMBER'] + if service_permissions is None: + service_permissions = DEFAULT_SERVICE_PERMISSIONS + from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d904c8ecf..7f3bd3008 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -137,7 +137,7 @@ def sample_service( restricted=False, limit=1000, email_from=None, - permissions=[SMS_TYPE, EMAIL_TYPE], + permissions=None, research_mode=None, free_sms_fragment_limit=250000 ): @@ -166,7 +166,7 @@ def sample_service( if user not in service.users: dao_add_user_to_service(service, user) - if INBOUND_SMS_TYPE in permissions: + if permissions and INBOUND_SMS_TYPE in permissions: create_inbound_number('12345', service_id=service.id) return service diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1d01c4a3e..3396e52ca 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -57,6 +57,7 @@ from app.models import ( KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, + INTERNATIONAL_SMS_TYPE, LETTER_TYPE, SERVICE_PERMISSION_TYPES ) @@ -260,25 +261,38 @@ def test_create_service_returns_service_with_default_permissions(service_factory service = service_factory.get('testing', email_from='testing') service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 2 - assert set([SMS_TYPE, EMAIL_TYPE]) == set(p.permission for p in service.permissions) + assert len(service.permissions) == 3 + assert set([ + SMS_TYPE, + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, + ]) == set( + p.permission for p in service.permissions + ) -@pytest.mark.parametrize("permission_to_remove, permission_remaining", - [(SMS_TYPE, EMAIL_TYPE), - (EMAIL_TYPE, SMS_TYPE)]) +@pytest.mark.parametrize("permission_to_remove, permission_remaining", [ + (SMS_TYPE, EMAIL_TYPE), + (EMAIL_TYPE, SMS_TYPE), +]) def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions( - sample_service, permission_to_remove, permission_remaining): + sample_service, permission_to_remove, permission_remaining +): dao_remove_service_permission(service_id=sample_service.id, permission=permission_to_remove) service = dao_fetch_service_by_id(sample_service.id) - assert len(service.permissions) == 1 - assert service.permissions[0].permission == permission_remaining + assert len(service.permissions) == 2 + assert set( + p.permission for p in service.permissions + ) == set(( + permission_remaining, INTERNATIONAL_SMS_TYPE + )) def test_removing_all_permission_returns_service_with_no_permissions(sample_service): dao_remove_service_permission(service_id=sample_service.id, permission=SMS_TYPE) dao_remove_service_permission(service_id=sample_service.id, permission=EMAIL_TYPE) + dao_remove_service_permission(service_id=sample_service.id, permission=INTERNATIONAL_SMS_TYPE) service = dao_fetch_service_by_id(sample_service.id) assert len(service.permissions) == 0 @@ -298,14 +312,22 @@ def test_create_service_by_id_adding_and_removing_letter_returns_service_without dao_add_service_permission(service_id=service.id, permission=LETTER_TYPE) service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 3 - assert set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]) == set([p.permission for p in service.permissions]) + assert len(service.permissions) == 4 + assert set([ + SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, + ]) == set([ + p.permission for p in service.permissions + ]) dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE) service = dao_fetch_service_by_id(service.id) - assert len(service.permissions) == 2 - assert set([SMS_TYPE, EMAIL_TYPE]) == set([p.permission for p in service.permissions]) + assert len(service.permissions) == 3 + assert set([ + SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, + ]) == set([ + p.permission for p in service.permissions + ]) def test_create_service_creates_a_history_record_with_current_data(sample_user): @@ -431,7 +453,7 @@ def test_delete_service_and_associated_objects(notify_db, sample_permission, sample_provider_statistics): # Default service permissions of Email and SMS - assert ServicePermission.query.count() == 2 + assert ServicePermission.query.count() == 3 delete_service_and_all_associated_db_objects(sample_service) assert NotificationStatistics.query.count() == 0 diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 387da140e..fecfc4da5 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -310,9 +310,14 @@ 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): +def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms( + key_type, + notify_db, + notify_db_session, +): + service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE]) with pytest.raises(BadRequestError) as e: - validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, SMS_TYPE) + validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE) assert e.value.status_code == 400 assert e.value.message == 'Cannot send to international mobile numbers' assert e.value.fields == [] diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 21046702d..63381561c 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -203,7 +203,14 @@ def test_get_service_list_has_default_permissions(client, service_factory): assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) assert len(json_resp['data']) == 3 - assert all([set(json['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) for json in json_resp['data']]) + assert all( + set( + json['permissions'] + ) == set([ + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, + ]) + for json in json_resp['data'] + ) def test_get_service_by_id_has_default_service_permissions(client, sample_service): @@ -214,7 +221,11 @@ def test_get_service_by_id_has_default_service_permissions(client, sample_servic ) json_resp = json.loads(resp.get_data(as_text=True)) - assert set(json_resp['data']['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) + assert set( + json_resp['data']['permissions'] + ) == set([ + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, + ]) def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 390ccac08..94fea27e5 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -328,17 +328,26 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( assert not deliver_mock.called -def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client, sample_service, sample_template): +def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms( + client, + notify_db, + notify_db_session, +): + + service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE]) + template = create_sample_template(notify_db, notify_db_session, service=service) + data = { 'phone_number': '20-12-1234-1234', - 'template_id': sample_template.id + 'template_id': template.id } - auth_header = create_authorization_header(service_id=sample_service.id) + auth_header = create_authorization_header(service_id=service.id) response = client.post( path='/v2/notifications/sms', data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) + headers=[('Content-Type', 'application/json'), auth_header] + ) assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' @@ -378,18 +387,20 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification( ] -def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client, mocker): - - service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE]) - template = create_sample_template(notify_db, notify_db_session, service=service) +def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms( + sample_service, + sample_template, + client, + mocker, +): mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'phone_number': '20-12-1234-1234', - 'template_id': template.id + 'template_id': sample_template.id } - auth_header = create_authorization_header(service_id=service.id) + auth_header = create_authorization_header(service_id=sample_service.id) response = client.post( path='/v2/notifications/sms',