From d9fd541ab7149c9fbb3fd0a0dba5206d0b970fc0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 11 Aug 2020 07:51:50 +0100 Subject: [PATCH] Add international letters as a default permission when creating a new service --- app/dao/services_dao.py | 5 +-- tests/app/dao/test_services_dao.py | 14 +++---- tests/app/service/test_rest.py | 62 ++++++++++++++++-------------- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index a7fb6278e..f678969ef 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -51,6 +51,7 @@ from app.models import ( SMS_TYPE, LETTER_TYPE, UPLOAD_LETTERS, + INTERNATIONAL_LETTERS ) from app.utils import ( email_address_is_nhs, @@ -66,6 +67,7 @@ DEFAULT_SERVICE_PERMISSIONS = [ LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + INTERNATIONAL_LETTERS, ] @@ -294,9 +296,6 @@ def dao_create_service( 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 user: raise ValueError("Can't create a service without a user") diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 215f4dc86..b05793f14 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -42,7 +42,7 @@ from app.models import (EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, KEY_TYPE_NORMAL, NotificationHistory, Organisation, Permission, Service, ServicePermission, ServiceUser, Template, TemplateHistory, User, VerifyCode, - user_folder_permissions) + user_folder_permissions, INTERNATIONAL_LETTERS) from tests.app.db import (create_annual_billing, create_api_key, create_email_branding, create_ft_billing, create_inbound_number, create_invited_user, @@ -484,16 +484,16 @@ def test_create_service_returns_service_with_default_permissions(notify_db_sessi service = dao_fetch_service_by_id(service.id) _assert_service_permissions(service.permissions, ( - SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS + SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )) @pytest.mark.parametrize("permission_to_remove, permissions_remaining", [ (SMS_TYPE, ( - EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )), (EMAIL_TYPE, ( - SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )), ]) def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions( @@ -525,14 +525,14 @@ def test_create_service_by_id_adding_and_removing_letter_returns_service_without service = dao_fetch_service_by_id(service.id) _assert_service_permissions(service.permissions, ( - SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )) dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE) service = dao_fetch_service_by_id(service.id) _assert_service_permissions(service.permissions, ( - SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )) @@ -669,7 +669,7 @@ def test_delete_service_and_associated_objects(notify_db_session): user.organisations = [organisation] assert ServicePermission.query.count() == len(( - SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, + SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS )) delete_service_and_all_associated_db_objects(service) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 867f66b29..91e67502d 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -24,10 +24,18 @@ from app.models import ( ServicePermission, ServiceSmsSender, User, - KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, - INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, - NOTIFICATION_RETURNED_LETTER, UPLOAD_LETTERS, + KEY_TYPE_NORMAL, + KEY_TYPE_TEAM, + KEY_TYPE_TEST, + EMAIL_TYPE, + SMS_TYPE, + LETTER_TYPE, + INTERNATIONAL_LETTERS, + INTERNATIONAL_SMS_TYPE, + INBOUND_SMS_TYPE, + NOTIFICATION_RETURNED_LETTER, + UPLOAD_LETTERS, + ) from tests import create_authorization_header from tests.app.db import ( @@ -91,9 +99,9 @@ def test_get_service_list_with_only_active_flag(client, service_factory): def test_get_service_list_with_user_id_and_only_active_flag( - admin_request, - sample_user, - service_factory + admin_request, + sample_user, + service_factory ): other_user = create_user(email='foo@bar.gov.uk') @@ -280,9 +288,9 @@ def test_get_service_list_has_default_permissions(admin_request, service_factory assert all( set( json['permissions'] - ) == set([ - EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, UPLOAD_LETTERS, - ]) + ) == { + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS + } for json in json_resp['data'] ) @@ -292,9 +300,9 @@ def test_get_service_by_id_has_default_service_permissions(admin_request, sample assert set( json_resp['data']['permissions'] - ) == set([ - EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, UPLOAD_LETTERS, - ]) + ) == { + EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE, UPLOAD_LETTERS, INTERNATIONAL_LETTERS + } def test_get_service_by_id_should_404_if_no_service(admin_request, notify_db_session): @@ -407,7 +415,6 @@ def test_create_service_with_domain_sets_organisation( domain, expected_org, ): - red_herring_org = create_organisation(name='Sub example') create_domain('specific.example.gov.uk', red_herring_org.id) create_domain('aaaaaaaa.example.gov.uk', red_herring_org.id) @@ -445,7 +452,6 @@ def test_create_service_inherits_branding_from_organisation( admin_request, sample_user, ): - org = create_organisation() email_branding = create_email_branding() org.email_branding = email_branding @@ -1518,7 +1524,7 @@ def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db def test_remove_user_from_service( - client, sample_user_service_permission + client, sample_user_service_permission ): second_user = create_user(email="new@digital.cabinet-office.gov.uk") service = sample_user_service_permission.service @@ -1542,7 +1548,7 @@ def test_remove_user_from_service( def test_remove_non_existant_user_from_service( - client, sample_user_service_permission + client, sample_user_service_permission ): second_user = create_user(email="new@digital.cabinet-office.gov.uk") endpoint = url_for( @@ -1649,7 +1655,6 @@ def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_ def test_get_notification_for_service(client, notify_db_session): - service_1 = create_service(service_name="1", email_from='1') service_2 = create_service(service_name="2", email_from='2') @@ -1725,13 +1730,13 @@ def test_get_notification_for_service_returns_old_template_version(admin_request ] ) def test_get_all_notifications_for_service_including_ones_made_by_jobs( - client, - sample_service, - include_from_test_key, - expected_count_of_notifications, - sample_notification, - sample_notification_with_job, - sample_template, + client, + sample_service, + include_from_test_key, + expected_count_of_notifications, + sample_notification, + sample_notification_with_job, + sample_template, ): # notification from_test_api_key create_notification(sample_template, key_type=KEY_TYPE_TEST) @@ -2070,7 +2075,6 @@ def test_get_detailed_services_for_date_range(sample_template, start_date_delta, def test_search_for_notification_by_to_field(client, sample_template, sample_email_template): - notification1 = create_notification(template=sample_template, to_field='+447700900855', normalised_to='447700900855') notification2 = create_notification(template=sample_email_template, to_field='jack@gmail.com', @@ -2544,9 +2548,9 @@ def test_is_service_name_unique_returns_200_with_name_capitalized_or_punctuation @pytest.mark.parametrize('name, email_from', [ - ("existing name", "email.from"), - ("name", "existing.name") - ]) + ("existing name", "email.from"), + ("name", "existing.name") +]) def test_is_service_name_unique_returns_200_and_false_if_name_or_email_from_exist_for_a_different_service( admin_request, notify_db,