From e9fed12a1ec94699451d836e675897a62dc8f311 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 14 Jul 2020 16:26:32 +0100 Subject: [PATCH 01/10] Rename API URLs for guest list to guest list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a small part of removing the term ‘white list’ from the API. Once the admin app is pointed at these new URLs, we can remove the old ones. --- app/service/rest.py | 2 ++ tests/app/service/test_service_whitelist.py | 25 ++++++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 08560da73..edfe4d63c 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -561,6 +561,7 @@ def get_detailed_services(start_date, end_date, only_active=False, include_from_ @service_blueprint.route('//whitelist', methods=['GET']) +@service_blueprint.route('//guest-list', methods=['GET']) def get_whitelist(service_id): from app.models import (EMAIL_TYPE, MOBILE_TYPE) service = dao_fetch_service_by_id(service_id) @@ -578,6 +579,7 @@ def get_whitelist(service_id): @service_blueprint.route('//whitelist', methods=['PUT']) +@service_blueprint.route('//guest-list', methods=['PUT']) def update_whitelist(service_id): # doesn't commit so if there are any errors, we preserve old values in db dao_remove_service_whitelist(service_id) diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index 18c0595f8..db2f790eb 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -1,3 +1,4 @@ +import pytest import uuid import json @@ -10,10 +11,14 @@ from app.models import ( from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts -def test_get_whitelist_returns_data(client, sample_service_whitelist): +@pytest.mark.parametrize('url_path', ( + 'service/{}/whitelist', + 'service/{}/guest-list', +)) +def test_get_whitelist_returns_data(client, sample_service_whitelist, url_path): service_id = sample_service_whitelist.service_id - response = client.get('service/{}/whitelist'.format(service_id), headers=[create_authorization_header()]) + response = client.get(url_path.format(service_id), headers=[create_authorization_header()]) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == { 'email_addresses': [sample_service_whitelist.recipient], @@ -28,7 +33,7 @@ def test_get_whitelist_separates_emails_and_phones(client, sample_service): ServiceWhitelist.from_string(sample_service.id, MOBILE_TYPE, '+1800-555-555'), ]) - response = client.get('service/{}/whitelist'.format(sample_service.id), headers=[create_authorization_header()]) + response = client.get('service/{}/guest-list'.format(sample_service.id), headers=[create_authorization_header()]) assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) assert json_resp['email_addresses'] == ['service@example.com'] @@ -36,7 +41,7 @@ def test_get_whitelist_separates_emails_and_phones(client, sample_service): def test_get_whitelist_404s_with_unknown_service_id(client): - path = 'service/{}/whitelist'.format(uuid.uuid4()) + path = 'service/{}/guest-list'.format(uuid.uuid4()) response = client.get(path, headers=[create_authorization_header()]) assert response.status_code == 404 @@ -46,7 +51,7 @@ def test_get_whitelist_404s_with_unknown_service_id(client): def test_get_whitelist_returns_no_data(client, sample_service): - path = 'service/{}/whitelist'.format(sample_service.id) + path = 'service/{}/guest-list'.format(sample_service.id) response = client.get(path, headers=[create_authorization_header()]) @@ -54,14 +59,18 @@ def test_get_whitelist_returns_no_data(client, sample_service): assert json.loads(response.get_data(as_text=True)) == {'email_addresses': [], 'phone_numbers': []} -def test_update_whitelist_replaces_old_whitelist(client, sample_service_whitelist): +@pytest.mark.parametrize('url_path', ( + 'service/{}/whitelist', + 'service/{}/guest-list', +)) +def test_update_whitelist_replaces_old_whitelist(client, sample_service_whitelist, url_path): data = { 'email_addresses': ['foo@bar.com'], 'phone_numbers': ['07123456789'] } response = client.put( - 'service/{}/whitelist'.format(sample_service_whitelist.service_id), + url_path.format(sample_service_whitelist.service_id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) @@ -81,7 +90,7 @@ def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_se } response = client.put( - 'service/{}/whitelist'.format(sample_service_whitelist.service_id), + 'service/{}/guest-list'.format(sample_service_whitelist.service_id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) From 7d09599bc51fc0d826d1e0308bc89e855be5aa68 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 10:16:48 +0100 Subject: [PATCH 02/10] Rename methods and variables in rest.py To reflect the new name of this feature. --- app/service/rest.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index edfe4d63c..0abd31bd3 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -562,36 +562,36 @@ def get_detailed_services(start_date, end_date, only_active=False, include_from_ @service_blueprint.route('//whitelist', methods=['GET']) @service_blueprint.route('//guest-list', methods=['GET']) -def get_whitelist(service_id): +def get_guest_list(service_id): from app.models import (EMAIL_TYPE, MOBILE_TYPE) service = dao_fetch_service_by_id(service_id) if not service: raise InvalidRequest("Service does not exist", status_code=404) - whitelist = dao_fetch_service_whitelist(service.id) + guest_list = dao_fetch_service_whitelist(service.id) return jsonify( - email_addresses=[item.recipient for item in whitelist + email_addresses=[item.recipient for item in guest_list if item.recipient_type == EMAIL_TYPE], - phone_numbers=[item.recipient for item in whitelist + phone_numbers=[item.recipient for item in guest_list if item.recipient_type == MOBILE_TYPE] ) @service_blueprint.route('//whitelist', methods=['PUT']) @service_blueprint.route('//guest-list', methods=['PUT']) -def update_whitelist(service_id): +def update_guest_list(service_id): # doesn't commit so if there are any errors, we preserve old values in db dao_remove_service_whitelist(service_id) try: - whitelist_objs = get_whitelist_objects(service_id, request.get_json()) + guest_list_objects = get_whitelist_objects(service_id, request.get_json()) except ValueError as e: current_app.logger.exception(e) dao_rollback() msg = '{} is not a valid email address or phone number'.format(str(e)) raise InvalidRequest(msg, 400) else: - dao_add_and_commit_whitelisted_contacts(whitelist_objs) + dao_add_and_commit_whitelisted_contacts(guest_list_objects) return '', 204 From 6384b9ef4fdab8dfd1d2da64c60a856a148e29e6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 10:18:47 +0100 Subject: [PATCH 03/10] Rename whitelist DAO functions To reflect the new name of the feature. --- app/dao/service_whitelist_dao.py | 6 +++--- app/service/rest.py | 12 +++++------ tests/app/dao/test_service_whitelist_dao.py | 20 +++++++++---------- .../service/test_send_one_off_notification.py | 4 ++-- tests/app/service/test_service_whitelist.py | 4 ++-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/dao/service_whitelist_dao.py b/app/dao/service_whitelist_dao.py index 04bbef347..0ced2d149 100644 --- a/app/dao/service_whitelist_dao.py +++ b/app/dao/service_whitelist_dao.py @@ -2,16 +2,16 @@ from app import db from app.models import ServiceWhitelist -def dao_fetch_service_whitelist(service_id): +def dao_fetch_service_guest_list(service_id): return ServiceWhitelist.query.filter( ServiceWhitelist.service_id == service_id).all() -def dao_add_and_commit_whitelisted_contacts(objs): +def dao_add_and_commit_guest_list_contacts(objs): db.session.add_all(objs) db.session.commit() -def dao_remove_service_whitelist(service_id): +def dao_remove_service_guest_list(service_id): return ServiceWhitelist.query.filter( ServiceWhitelist.service_id == service_id).delete() diff --git a/app/service/rest.py b/app/service/rest.py index 0abd31bd3..3db7f858f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -75,9 +75,9 @@ from app.dao.services_dao import ( get_services_by_partial_name, ) from app.dao.service_whitelist_dao import ( - dao_fetch_service_whitelist, - dao_add_and_commit_whitelisted_contacts, - dao_remove_service_whitelist + dao_fetch_service_guest_list, + dao_add_and_commit_guest_list_contacts, + dao_remove_service_guest_list ) from app.dao.service_email_reply_to_dao import ( add_reply_to_email_address_for_service, @@ -569,7 +569,7 @@ def get_guest_list(service_id): if not service: raise InvalidRequest("Service does not exist", status_code=404) - guest_list = dao_fetch_service_whitelist(service.id) + guest_list = dao_fetch_service_guest_list(service.id) return jsonify( email_addresses=[item.recipient for item in guest_list if item.recipient_type == EMAIL_TYPE], @@ -582,7 +582,7 @@ def get_guest_list(service_id): @service_blueprint.route('//guest-list', methods=['PUT']) def update_guest_list(service_id): # doesn't commit so if there are any errors, we preserve old values in db - dao_remove_service_whitelist(service_id) + dao_remove_service_guest_list(service_id) try: guest_list_objects = get_whitelist_objects(service_id, request.get_json()) except ValueError as e: @@ -591,7 +591,7 @@ def update_guest_list(service_id): msg = '{} is not a valid email address or phone number'.format(str(e)) raise InvalidRequest(msg, 400) else: - dao_add_and_commit_whitelisted_contacts(guest_list_objects) + dao_add_and_commit_guest_list_contacts(guest_list_objects) return '', 204 diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py index c18183831..4ff46a91a 100644 --- a/tests/app/dao/test_service_whitelist_dao.py +++ b/tests/app/dao/test_service_whitelist_dao.py @@ -6,27 +6,27 @@ from app.models import ( ) from app.dao.service_whitelist_dao import ( - dao_fetch_service_whitelist, - dao_add_and_commit_whitelisted_contacts, - dao_remove_service_whitelist + dao_fetch_service_guest_list, + dao_add_and_commit_guest_list_contacts, + dao_remove_service_guest_list ) from tests.app.db import create_service def test_fetch_service_whitelist_gets_whitelists(sample_service_whitelist): - whitelist = dao_fetch_service_whitelist(sample_service_whitelist.service_id) + whitelist = dao_fetch_service_guest_list(sample_service_whitelist.service_id) assert len(whitelist) == 1 assert whitelist[0].id == sample_service_whitelist.id def test_fetch_service_whitelist_ignores_other_service(sample_service_whitelist): - assert len(dao_fetch_service_whitelist(uuid.uuid4())) == 0 + assert len(dao_fetch_service_guest_list(uuid.uuid4())) == 0 def test_add_and_commit_whitelisted_contacts_saves_data(sample_service): whitelist = ServiceWhitelist.from_string(sample_service.id, EMAIL_TYPE, 'foo@example.com') - dao_add_and_commit_whitelisted_contacts([whitelist]) + dao_add_and_commit_guest_list_contacts([whitelist]) db_contents = ServiceWhitelist.query.all() assert len(db_contents) == 1 @@ -36,21 +36,21 @@ def test_add_and_commit_whitelisted_contacts_saves_data(sample_service): def test_remove_service_whitelist_only_removes_for_my_service(notify_db, notify_db_session): service_1 = create_service(service_name="service 1") service_2 = create_service(service_name="service 2") - dao_add_and_commit_whitelisted_contacts([ + dao_add_and_commit_guest_list_contacts([ ServiceWhitelist.from_string(service_1.id, EMAIL_TYPE, 'service1@example.com'), ServiceWhitelist.from_string(service_2.id, EMAIL_TYPE, 'service2@example.com') ]) - dao_remove_service_whitelist(service_1.id) + dao_remove_service_guest_list(service_1.id) assert service_1.whitelist == [] assert len(service_2.whitelist) == 1 def test_remove_service_whitelist_does_not_commit(notify_db, sample_service_whitelist): - dao_remove_service_whitelist(sample_service_whitelist.service_id) + dao_remove_service_guest_list(sample_service_whitelist.service_id) - # since dao_remove_service_whitelist doesn't commit, we can still rollback its changes + # since dao_remove_service_guest_list doesn't commit, we can still rollback its changes notify_db.session.rollback() assert ServiceWhitelist.query.count() == 1 diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 0b8e74cd4..d4f9fdad5 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -7,7 +7,7 @@ from notifications_utils.recipients import InvalidPhoneError from app.v2.errors import BadRequestError, TooManyRequestsError from app.config import QueueNames -from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts +from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts from app.service.send_notification import send_one_off_notification from app.models import ( EMAIL_TYPE, @@ -269,7 +269,7 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient( ): service = create_service(restricted=True) template = create_template(service=service) - dao_add_and_commit_whitelisted_contacts([ + dao_add_and_commit_guest_list_contacts([ ServiceWhitelist.from_string(service.id, MOBILE_TYPE, '07700900123'), ]) diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index db2f790eb..a159148f2 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -8,7 +8,7 @@ from app.models import ( ServiceWhitelist, MOBILE_TYPE, EMAIL_TYPE) -from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts +from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts @pytest.mark.parametrize('url_path', ( @@ -27,7 +27,7 @@ def test_get_whitelist_returns_data(client, sample_service_whitelist, url_path): def test_get_whitelist_separates_emails_and_phones(client, sample_service): - dao_add_and_commit_whitelisted_contacts([ + dao_add_and_commit_guest_list_contacts([ ServiceWhitelist.from_string(sample_service.id, EMAIL_TYPE, 'service@example.com'), ServiceWhitelist.from_string(sample_service.id, MOBILE_TYPE, '07123456789'), ServiceWhitelist.from_string(sample_service.id, MOBILE_TYPE, '+1800-555-555'), From 4d896aa642089155571d1c42d53964994e9cf84a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 10:19:46 +0100 Subject: [PATCH 04/10] Rename function in service utils To reflect the new name of the feature. squash! Rename function in service utils Rename function, variable and argument names in service utils --- app/notifications/validators.py | 8 ++++---- app/service/rest.py | 4 ++-- app/service/send_notification.py | 4 ++-- app/service/utils.py | 10 +++++----- tests/app/notifications/test_validators.py | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 93992c580..d26068b92 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -78,8 +78,8 @@ def check_template_is_active(template): message="Template has been deleted") -def service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_recipients=True): - if not service_allowed_to_send_to(send_to, service, key_type, allow_whitelisted_recipients): +def service_can_send_to_recipient(send_to, key_type, service, allow_guest_list_recipients=True): + if not service_allowed_to_send_to(send_to, service, key_type, allow_guest_list_recipients): if key_type == KEY_TYPE_TEAM: message = 'Can’t send to this recipient using a team-only API key' else: @@ -109,11 +109,11 @@ def check_if_service_can_send_files_by_email(service_contact_link, service_id): ) -def validate_and_format_recipient(send_to, key_type, service, notification_type, allow_whitelisted_recipients=True): +def validate_and_format_recipient(send_to, key_type, service, notification_type, allow_guest_list_recipients=True): if send_to is None: raise BadRequestError(message="Recipient can't be empty") - service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_recipients) + service_can_send_to_recipient(send_to, key_type, service, allow_guest_list_recipients) if notification_type == SMS_TYPE: international_phone_info = check_if_service_can_send_to_number(service, send_to) diff --git a/app/service/rest.py b/app/service/rest.py index 3db7f858f..1f794eb0b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -119,7 +119,7 @@ from app.service.service_senders_schema import ( add_service_letter_contact_block_request, add_service_sms_sender_request ) -from app.service.utils import get_whitelist_objects +from app.service.utils import get_guest_list_objects from app.service.sender import send_notification_to_service_users from app.service.send_notification import send_one_off_notification, send_pdf_letter_notification from app.schemas import ( @@ -584,7 +584,7 @@ def update_guest_list(service_id): # doesn't commit so if there are any errors, we preserve old values in db dao_remove_service_guest_list(service_id) try: - guest_list_objects = get_whitelist_objects(service_id, request.get_json()) + guest_list_objects = get_guest_list_objects(service_id, request.get_json()) except ValueError as e: current_app.logger.exception(e) dao_rollback() diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 10ed42ab7..2987a5362 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -73,7 +73,7 @@ def send_one_off_notification(service_id, post_data): key_type=KEY_TYPE_NORMAL, service=service, notification_type=template.template_type, - allow_whitelisted_recipients=False, + allow_guest_list_recipients=False, ) validate_created_by(service, post_data['created_by']) @@ -147,7 +147,7 @@ def send_pdf_letter_notification(service_id, post_data): key_type=KEY_TYPE_NORMAL, service=service, notification_type=LETTER_TYPE, - allow_whitelisted_recipients=False, + allow_guest_list_recipients=False, ) template = get_precompiled_letter_template(service.id) diff --git a/app/service/utils.py b/app/service/utils.py index c6e0e0476..3ab1ed97c 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -14,7 +14,7 @@ def get_recipients_from_request(request_json, key, type): return [(type, recipient) for recipient in request_json.get(key)] -def get_whitelist_objects(service_id, request_json): +def get_guest_list_objects(service_id, request_json): return [ ServiceWhitelist.from_string(service_id, type, recipient) for type, recipient in ( @@ -28,7 +28,7 @@ def get_whitelist_objects(service_id, request_json): ] -def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_recipients=True): +def service_allowed_to_send_to(recipient, service, key_type, allow_guest_list_recipients=True): if key_type == KEY_TYPE_TEST: return True @@ -42,9 +42,9 @@ def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_r team_members = itertools.chain.from_iterable( [user.mobile_number, user.email_address] for user in service.users ) - whitelist_members = [ + guest_list_members = [ member.recipient for member in service.whitelist - if allow_whitelisted_recipients + if allow_guest_list_recipients ] if ( @@ -55,6 +55,6 @@ def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_r recipient, itertools.chain( team_members, - whitelist_members + guest_list_members ) ) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 4e6748917..f69060bf1 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -272,7 +272,7 @@ def test_service_can_send_to_recipient_fails_when_ignoring_whitelist( next(iter(recipient.values())), 'team', sample_service, - allow_whitelisted_recipients=False, + allow_guest_list_recipients=False, ) assert exec_info.value.status_code == 400 assert exec_info.value.message == 'Can’t send to this recipient using a team-only API key' From 083573e4dcd90cefe410f38658eeae5b19cbff23 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 10:22:13 +0100 Subject: [PATCH 05/10] Rename model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reflects the new name of the feature. Note that the name of the underlying table hasn’t changed because it’s explicitly set to `service_whitelist`. Changing this will be a more involved process. --- app/dao/service_whitelist_dao.py | 10 +++++----- app/models.py | 8 ++++---- app/service/utils.py | 4 ++-- tests/app/conftest.py | 4 ++-- tests/app/dao/test_service_whitelist_dao.py | 12 ++++++------ tests/app/db.py | 8 ++++---- tests/app/service/test_send_one_off_notification.py | 4 ++-- tests/app/service/test_service_whitelist.py | 12 ++++++------ tests/app/test_model.py | 8 ++++---- 9 files changed, 35 insertions(+), 35 deletions(-) diff --git a/app/dao/service_whitelist_dao.py b/app/dao/service_whitelist_dao.py index 0ced2d149..24ee2a7f2 100644 --- a/app/dao/service_whitelist_dao.py +++ b/app/dao/service_whitelist_dao.py @@ -1,10 +1,10 @@ from app import db -from app.models import ServiceWhitelist +from app.models import ServiceGuestList def dao_fetch_service_guest_list(service_id): - return ServiceWhitelist.query.filter( - ServiceWhitelist.service_id == service_id).all() + return ServiceGuestList.query.filter( + ServiceGuestList.service_id == service_id).all() def dao_add_and_commit_guest_list_contacts(objs): @@ -13,5 +13,5 @@ def dao_add_and_commit_guest_list_contacts(objs): def dao_remove_service_guest_list(service_id): - return ServiceWhitelist.query.filter( - ServiceWhitelist.service_id == service_id).delete() + return ServiceGuestList.query.filter( + ServiceGuestList.service_id == service_id).delete() diff --git a/app/models.py b/app/models.py index d447505b2..baff90554 100644 --- a/app/models.py +++ b/app/models.py @@ -655,17 +655,17 @@ class ServicePermission(db.Model): MOBILE_TYPE = 'mobile' EMAIL_TYPE = 'email' -WHITELIST_RECIPIENT_TYPE = [MOBILE_TYPE, EMAIL_TYPE] -whitelist_recipient_types = db.Enum(*WHITELIST_RECIPIENT_TYPE, name='recipient_type') +GUEST_LIST_RECIPIENT_TYPE = [MOBILE_TYPE, EMAIL_TYPE] +guest_list_recipient_types = db.Enum(*GUEST_LIST_RECIPIENT_TYPE, name='recipient_type') -class ServiceWhitelist(db.Model): +class ServiceGuestList(db.Model): __tablename__ = 'service_whitelist' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) service = db.relationship('Service', backref='whitelist') - recipient_type = db.Column(whitelist_recipient_types, nullable=False) + recipient_type = db.Column(guest_list_recipient_types, nullable=False) recipient = db.Column(db.String(255), nullable=False) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow) diff --git a/app/service/utils.py b/app/service/utils.py index 3ab1ed97c..346d75fcf 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -3,7 +3,7 @@ import itertools from notifications_utils.recipients import allowed_to_send_to from app.models import ( - ServiceWhitelist, + ServiceGuestList, MOBILE_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, KEY_TYPE_NORMAL) @@ -16,7 +16,7 @@ def get_recipients_from_request(request_json, key, type): def get_guest_list_objects(service_id, request_json): return [ - ServiceWhitelist.from_string(service_id, type, recipient) + ServiceGuestList.from_string(service_id, type, recipient) for type, recipient in ( get_recipients_from_request(request_json, 'phone_numbers', diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 64ca8ab61..1698d594b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -33,7 +33,7 @@ from app.models import ( ProviderDetails, ProviderDetailsHistory, ProviderRates, - ServiceWhitelist, + ServiceGuestList, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, @@ -811,7 +811,7 @@ def notify_service(notify_db, notify_db_session): @pytest.fixture(scope='function') def sample_service_whitelist(notify_db, notify_db_session): service = create_service(check_if_service_exists=True) - whitelisted_user = ServiceWhitelist.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') + whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') notify_db.session.add(whitelisted_user) notify_db.session.commit() diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py index 4ff46a91a..c0a22be5d 100644 --- a/tests/app/dao/test_service_whitelist_dao.py +++ b/tests/app/dao/test_service_whitelist_dao.py @@ -1,7 +1,7 @@ import uuid from app.models import ( - ServiceWhitelist, + ServiceGuestList, EMAIL_TYPE, ) @@ -24,11 +24,11 @@ def test_fetch_service_whitelist_ignores_other_service(sample_service_whitelist) def test_add_and_commit_whitelisted_contacts_saves_data(sample_service): - whitelist = ServiceWhitelist.from_string(sample_service.id, EMAIL_TYPE, 'foo@example.com') + whitelist = ServiceGuestList.from_string(sample_service.id, EMAIL_TYPE, 'foo@example.com') dao_add_and_commit_guest_list_contacts([whitelist]) - db_contents = ServiceWhitelist.query.all() + db_contents = ServiceGuestList.query.all() assert len(db_contents) == 1 assert db_contents[0].id == whitelist.id @@ -37,8 +37,8 @@ def test_remove_service_whitelist_only_removes_for_my_service(notify_db, notify_ service_1 = create_service(service_name="service 1") service_2 = create_service(service_name="service 2") dao_add_and_commit_guest_list_contacts([ - ServiceWhitelist.from_string(service_1.id, EMAIL_TYPE, 'service1@example.com'), - ServiceWhitelist.from_string(service_2.id, EMAIL_TYPE, 'service2@example.com') + ServiceGuestList.from_string(service_1.id, EMAIL_TYPE, 'service1@example.com'), + ServiceGuestList.from_string(service_2.id, EMAIL_TYPE, 'service2@example.com') ]) dao_remove_service_guest_list(service_1.id) @@ -53,4 +53,4 @@ def test_remove_service_whitelist_does_not_commit(notify_db, sample_service_whit # since dao_remove_service_guest_list doesn't commit, we can still rollback its changes notify_db.session.rollback() - assert ServiceWhitelist.query.count() == 1 + assert ServiceGuestList.query.count() == 1 diff --git a/tests/app/db.py b/tests/app/db.py index 190ef0c0c..012d55e18 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -40,7 +40,7 @@ from app.models import ( ServiceLetterContact, ServicePermission, ServiceSmsSender, - ServiceWhitelist, + ServiceGuestList, Template, User, EMAIL_TYPE, @@ -740,11 +740,11 @@ def create_ft_notification_status( def create_service_whitelist(service, email_address=None, mobile_number=None): if email_address: - whitelisted_user = ServiceWhitelist.from_string(service.id, EMAIL_TYPE, email_address) + whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, email_address) elif mobile_number: - whitelisted_user = ServiceWhitelist.from_string(service.id, MOBILE_TYPE, mobile_number) + whitelisted_user = ServiceGuestList.from_string(service.id, MOBILE_TYPE, mobile_number) else: - whitelisted_user = ServiceWhitelist.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') + whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') db.session.add(whitelisted_user) db.session.commit() diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index d4f9fdad5..af1bc353a 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -17,7 +17,7 @@ from app.models import ( PRIORITY, SMS_TYPE, Notification, - ServiceWhitelist, + ServiceGuestList, ) from tests.app.db import ( @@ -270,7 +270,7 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient( service = create_service(restricted=True) template = create_template(service=service) dao_add_and_commit_guest_list_contacts([ - ServiceWhitelist.from_string(service.id, MOBILE_TYPE, '07700900123'), + ServiceGuestList.from_string(service.id, MOBILE_TYPE, '07700900123'), ]) post_data = { diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index a159148f2..5c4e7bbd8 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -5,7 +5,7 @@ import json from tests import create_authorization_header from app.models import ( - ServiceWhitelist, + ServiceGuestList, MOBILE_TYPE, EMAIL_TYPE) from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts @@ -28,9 +28,9 @@ def test_get_whitelist_returns_data(client, sample_service_whitelist, url_path): def test_get_whitelist_separates_emails_and_phones(client, sample_service): dao_add_and_commit_guest_list_contacts([ - ServiceWhitelist.from_string(sample_service.id, EMAIL_TYPE, 'service@example.com'), - ServiceWhitelist.from_string(sample_service.id, MOBILE_TYPE, '07123456789'), - ServiceWhitelist.from_string(sample_service.id, MOBILE_TYPE, '+1800-555-555'), + ServiceGuestList.from_string(sample_service.id, EMAIL_TYPE, 'service@example.com'), + ServiceGuestList.from_string(sample_service.id, MOBILE_TYPE, '07123456789'), + ServiceGuestList.from_string(sample_service.id, MOBILE_TYPE, '+1800-555-555'), ]) response = client.get('service/{}/guest-list'.format(sample_service.id), headers=[create_authorization_header()]) @@ -76,7 +76,7 @@ def test_update_whitelist_replaces_old_whitelist(client, sample_service_whitelis ) assert response.status_code == 204 - whitelist = ServiceWhitelist.query.order_by(ServiceWhitelist.recipient).all() + whitelist = ServiceGuestList.query.order_by(ServiceGuestList.recipient).all() assert len(whitelist) == 2 assert whitelist[0].recipient == '07123456789' assert whitelist[1].recipient == 'foo@bar.com' @@ -100,5 +100,5 @@ def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_se 'result': 'error', 'message': 'Invalid whitelist: "" is not a valid email address or phone number' } - whitelist = ServiceWhitelist.query.one() + whitelist = ServiceGuestList.query.one() assert whitelist.id == sample_service_whitelist.id diff --git a/tests/app/test_model.py b/tests/app/test_model.py index c6df9d884..e44d50221 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -5,7 +5,7 @@ from sqlalchemy.exc import IntegrityError from app import encryption from app.models import ( - ServiceWhitelist, + ServiceGuestList, Notification, SMS_TYPE, MOBILE_TYPE, @@ -38,7 +38,7 @@ from tests.app.db import ( '+44 7700 900678' ]) def test_should_build_service_whitelist_from_mobile_number(mobile_number): - service_whitelist = ServiceWhitelist.from_string('service_id', MOBILE_TYPE, mobile_number) + service_whitelist = ServiceGuestList.from_string('service_id', MOBILE_TYPE, mobile_number) assert service_whitelist.recipient == mobile_number @@ -47,7 +47,7 @@ def test_should_build_service_whitelist_from_mobile_number(mobile_number): 'test@example.com' ]) def test_should_build_service_whitelist_from_email_address(email_address): - service_whitelist = ServiceWhitelist.from_string('service_id', EMAIL_TYPE, email_address) + service_whitelist = ServiceGuestList.from_string('service_id', EMAIL_TYPE, email_address) assert service_whitelist.recipient == email_address @@ -59,7 +59,7 @@ def test_should_build_service_whitelist_from_email_address(email_address): ]) def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, contact): with pytest.raises(ValueError): - ServiceWhitelist.from_string('service_id', recipient_type, contact) + ServiceGuestList.from_string('service_id', recipient_type, contact) @pytest.mark.parametrize('initial_statuses, expected_statuses', [ From e41022214f7b9e1e396d33994669c29bef220e58 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 11:23:30 +0100 Subject: [PATCH 06/10] Rename backref to service model To reflect the new name. Appears this is only used by the tests. --- app/models.py | 2 +- app/schemas.py | 6 +++--- app/service/utils.py | 2 +- tests/app/dao/test_service_whitelist_dao.py | 4 ++-- tests/app/notifications/rest/test_send_notification.py | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models.py b/app/models.py index baff90554..b05ce5233 100644 --- a/app/models.py +++ b/app/models.py @@ -664,7 +664,7 @@ class ServiceGuestList(db.Model): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref='whitelist') + service = db.relationship('Service', backref='guest_list') recipient_type = db.Column(guest_list_recipient_types, nullable=False) recipient = db.Column(db.String(255), nullable=False) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow) diff --git a/app/schemas.py b/app/schemas.py index 7788e3c79..f9127d394 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -259,7 +259,7 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin): 'returned_letters', 'users', 'version', - 'whitelist', + 'guest_list', 'broadcast_messages', ) strict = True @@ -310,7 +310,7 @@ class DetailedServiceSchema(BaseSchema): 'message_limit', 'email_from', 'inbound_api', - 'whitelist', + 'guest_list', 'reply_to_email_address', 'sms_sender', 'permissions', @@ -326,7 +326,7 @@ class DetailedServiceSchema(BaseSchema): 'returned_letters', 'users', 'version', - 'whitelist', + 'guest_list', 'broadcast_messages', ) diff --git a/app/service/utils.py b/app/service/utils.py index 346d75fcf..04c879777 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -43,7 +43,7 @@ def service_allowed_to_send_to(recipient, service, key_type, allow_guest_list_re [user.mobile_number, user.email_address] for user in service.users ) guest_list_members = [ - member.recipient for member in service.whitelist + member.recipient for member in service.guest_list if allow_guest_list_recipients ] diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py index c0a22be5d..10b7c3d93 100644 --- a/tests/app/dao/test_service_whitelist_dao.py +++ b/tests/app/dao/test_service_whitelist_dao.py @@ -43,8 +43,8 @@ def test_remove_service_whitelist_only_removes_for_my_service(notify_db, notify_ dao_remove_service_guest_list(service_1.id) - assert service_1.whitelist == [] - assert len(service_2.whitelist) == 1 + assert service_1.guest_list == [] + assert len(service_2.guest_list) == 1 def test_remove_service_whitelist_does_not_commit(notify_db, sample_service_whitelist): diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 92c31f555..63856a47e 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -793,7 +793,7 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) template = create_template(service, template_type=notification_type) assert sample_service_whitelist.service_id == service.id - assert to not in [member.recipient for member in service.whitelist] + assert to not in [member.recipient for member in service.guest_list] create_notification(template=template) @@ -852,7 +852,7 @@ def test_should_send_notification_to_whitelist_recipient( service_whitelist = create_service_whitelist(sample_service, email_address=to) assert service_whitelist.service_id == sample_service.id - assert to in [member.recipient for member in sample_service.whitelist] + assert to in [member.recipient for member in sample_service.guest_list] create_notification(template=template) From 716eb67bfd3e43096bc1ca5add69bbe387731526 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 10:23:22 +0100 Subject: [PATCH 07/10] Re-label error messages To reflect the new name of the feature. --- app/models.py | 4 ++-- tests/app/service/test_service_whitelist.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models.py b/app/models.py index b05ce5233..d9695d40e 100644 --- a/app/models.py +++ b/app/models.py @@ -683,9 +683,9 @@ class ServiceGuestList(db.Model): else: raise ValueError('Invalid recipient type') except InvalidPhoneError: - raise ValueError('Invalid whitelist: "{}"'.format(recipient)) + raise ValueError('Invalid guest list: "{}"'.format(recipient)) except InvalidEmailError: - raise ValueError('Invalid whitelist: "{}"'.format(recipient)) + raise ValueError('Invalid guest list: "{}"'.format(recipient)) else: return instance diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index 5c4e7bbd8..365d0feab 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -98,7 +98,7 @@ def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_se assert response.status_code == 400 assert json.loads(response.get_data(as_text=True)) == { 'result': 'error', - 'message': 'Invalid whitelist: "" is not a valid email address or phone number' + 'message': 'Invalid guest list: "" is not a valid email address or phone number' } whitelist = ServiceGuestList.query.one() assert whitelist.id == sample_service_whitelist.id From 65346852edad514fb6013c16505d3c7f0f3e5573 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 11:22:19 +0100 Subject: [PATCH 08/10] Rename variables and functions in tests To reflect the new name of the feature. --- tests/app/conftest.py | 8 ++--- tests/app/dao/test_service_whitelist_dao.py | 24 +++++++------- tests/app/db.py | 12 +++---- .../rest/test_send_notification.py | 22 ++++++------- tests/app/notifications/test_validators.py | 12 +++---- .../service/test_send_one_off_notification.py | 6 ++-- tests/app/service/test_service_whitelist.py | 32 +++++++++---------- tests/app/test_model.py | 14 ++++---- .../notifications/test_post_notifications.py | 2 +- 9 files changed, 66 insertions(+), 66 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 1698d594b..5fd67f4b6 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -809,13 +809,13 @@ def notify_service(notify_db, notify_db_session): @pytest.fixture(scope='function') -def sample_service_whitelist(notify_db, notify_db_session): +def sample_service_guest_list(notify_db, notify_db_session): service = create_service(check_if_service_exists=True) - whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') + guest_list_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'guest_list_user@digital.gov.uk') - notify_db.session.add(whitelisted_user) + notify_db.session.add(guest_list_user) notify_db.session.commit() - return whitelisted_user + return guest_list_user @pytest.fixture diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py index 10b7c3d93..695f98ed7 100644 --- a/tests/app/dao/test_service_whitelist_dao.py +++ b/tests/app/dao/test_service_whitelist_dao.py @@ -13,27 +13,27 @@ from app.dao.service_whitelist_dao import ( from tests.app.db import create_service -def test_fetch_service_whitelist_gets_whitelists(sample_service_whitelist): - whitelist = dao_fetch_service_guest_list(sample_service_whitelist.service_id) - assert len(whitelist) == 1 - assert whitelist[0].id == sample_service_whitelist.id +def test_fetch_service_guest_list_gets_guest_lists(sample_service_guest_list): + guest_list = dao_fetch_service_guest_list(sample_service_guest_list.service_id) + assert len(guest_list) == 1 + assert guest_list[0].id == sample_service_guest_list.id -def test_fetch_service_whitelist_ignores_other_service(sample_service_whitelist): +def test_fetch_service_guest_list_ignores_other_service(sample_service_guest_list): assert len(dao_fetch_service_guest_list(uuid.uuid4())) == 0 -def test_add_and_commit_whitelisted_contacts_saves_data(sample_service): - whitelist = ServiceGuestList.from_string(sample_service.id, EMAIL_TYPE, 'foo@example.com') +def test_add_and_commit_guest_list_contacts_saves_data(sample_service): + guest_list = ServiceGuestList.from_string(sample_service.id, EMAIL_TYPE, 'foo@example.com') - dao_add_and_commit_guest_list_contacts([whitelist]) + dao_add_and_commit_guest_list_contacts([guest_list]) db_contents = ServiceGuestList.query.all() assert len(db_contents) == 1 - assert db_contents[0].id == whitelist.id + assert db_contents[0].id == guest_list.id -def test_remove_service_whitelist_only_removes_for_my_service(notify_db, notify_db_session): +def test_remove_service_guest_list_only_removes_for_my_service(notify_db, notify_db_session): service_1 = create_service(service_name="service 1") service_2 = create_service(service_name="service 2") dao_add_and_commit_guest_list_contacts([ @@ -47,8 +47,8 @@ def test_remove_service_whitelist_only_removes_for_my_service(notify_db, notify_ assert len(service_2.guest_list) == 1 -def test_remove_service_whitelist_does_not_commit(notify_db, sample_service_whitelist): - dao_remove_service_guest_list(sample_service_whitelist.service_id) +def test_remove_service_guest_list_does_not_commit(notify_db, sample_service_guest_list): + dao_remove_service_guest_list(sample_service_guest_list.service_id) # since dao_remove_service_guest_list doesn't commit, we can still rollback its changes notify_db.session.rollback() diff --git a/tests/app/db.py b/tests/app/db.py index 012d55e18..2dc048dae 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -738,17 +738,17 @@ def create_ft_notification_status( return data -def create_service_whitelist(service, email_address=None, mobile_number=None): +def create_service_guest_list(service, email_address=None, mobile_number=None): if email_address: - whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, email_address) + guest_list_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, email_address) elif mobile_number: - whitelisted_user = ServiceGuestList.from_string(service.id, MOBILE_TYPE, mobile_number) + guest_list_user = ServiceGuestList.from_string(service.id, MOBILE_TYPE, mobile_number) else: - whitelisted_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'whitelisted_user@digital.gov.uk') + guest_list_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'guest_list_user@digital.gov.uk') - db.session.add(whitelisted_user) + db.session.add(guest_list_user) db.session.commit() - return whitelisted_user + return guest_list_user def create_complaint(service=None, diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 63856a47e..946f46521 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -25,7 +25,7 @@ from tests.app.db import ( create_api_key, create_notification, create_service, - create_service_whitelist, + create_service_guest_list, create_template, create_reply_to_email, ) @@ -776,23 +776,23 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( ]) @pytest.mark.parametrize('notification_type, to', [ (SMS_TYPE, '07827992635'), - (EMAIL_TYPE, 'non_whitelist_recipient@mail.com')] + (EMAIL_TYPE, 'non_guest_list_recipient@mail.com')] ) -def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( +def test_should_not_send_notification_to_non_guest_list_recipient_in_trial_mode( client, - sample_service_whitelist, + sample_service_guest_list, notification_type, to, key_type, mocker ): - service = sample_service_whitelist.service + service = sample_service_guest_list.service service.restricted = True service.message_limit = 2 apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) template = create_template(service, template_type=notification_type) - assert sample_service_whitelist.service_id == service.id + assert sample_service_guest_list.service_id == service.id assert to not in [member.recipient for member in service.guest_list] create_notification(template=template) @@ -830,9 +830,9 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode( ]) @pytest.mark.parametrize('notification_type, to', [ (SMS_TYPE, '07123123123'), - (EMAIL_TYPE, 'whitelist_recipient@mail.com')] + (EMAIL_TYPE, 'guest_list_recipient@mail.com')] ) -def test_should_send_notification_to_whitelist_recipient( +def test_should_send_notification_to_guest_list_recipient( client, sample_service, notification_type, @@ -847,11 +847,11 @@ def test_should_send_notification_to_whitelist_recipient( apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) template = create_template(sample_service, template_type=notification_type) if notification_type == SMS_TYPE: - service_whitelist = create_service_whitelist(sample_service, mobile_number=to) + service_guest_list = create_service_guest_list(sample_service, mobile_number=to) elif notification_type == EMAIL_TYPE: - service_whitelist = create_service_whitelist(sample_service, email_address=to) + service_guest_list = create_service_guest_list(sample_service, email_address=to) - assert service_whitelist.service_id == sample_service.id + assert service_guest_list.service_id == sample_service.id assert to in [member.recipient for member in sample_service.guest_list] create_notification(template=template) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index f69060bf1..57bd88882 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -39,7 +39,7 @@ from tests.app.db import ( create_reply_to_email, create_service, create_service_sms_sender, - create_service_whitelist, + create_service_guest_list, create_template, ) from unittest.mock import ANY @@ -245,12 +245,12 @@ def test_service_can_send_to_recipient_passes_for_live_service_non_team_member(k serialised_service) is None -def test_service_can_send_to_recipient_passes_for_whitelisted_recipient_passes(sample_service): - create_service_whitelist(sample_service, email_address="some_other_email@test.com") +def test_service_can_send_to_recipient_passes_for_guest_list_recipient_passes(sample_service): + create_service_guest_list(sample_service, email_address="some_other_email@test.com") assert service_can_send_to_recipient("some_other_email@test.com", 'team', sample_service) is None - create_service_whitelist(sample_service, mobile_number='07513332413') + create_service_guest_list(sample_service, mobile_number='07513332413') assert service_can_send_to_recipient('07513332413', 'team', sample_service) is None @@ -260,13 +260,13 @@ def test_service_can_send_to_recipient_passes_for_whitelisted_recipient_passes(s {"email_address": "some_other_email@test.com"}, {"mobile_number": "07513332413"}, ]) -def test_service_can_send_to_recipient_fails_when_ignoring_whitelist( +def test_service_can_send_to_recipient_fails_when_ignoring_guest_list( notify_db, notify_db_session, sample_service, recipient, ): - create_service_whitelist(sample_service, **recipient) + create_service_guest_list(sample_service, **recipient) with pytest.raises(BadRequestError) as exec_info: service_can_send_to_recipient( next(iter(recipient.values())), diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index af1bc353a..685d42286 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -259,9 +259,9 @@ def test_send_one_off_notification_raises_if_invalid_recipient(notify_db_session @pytest.mark.parametrize('recipient', [ - '07700 900 001', # not in team or whitelist - '07700900123', # in whitelist - '+447700-900-123', # in whitelist in different format + '07700 900 001', # not in team or guest_list + '07700900123', # in guest_list + '+447700-900-123', # in guest_list in different format ]) def test_send_one_off_notification_raises_if_cant_send_to_recipient( notify_db_session, diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index 365d0feab..9f7c8b6dd 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -15,18 +15,18 @@ from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts 'service/{}/whitelist', 'service/{}/guest-list', )) -def test_get_whitelist_returns_data(client, sample_service_whitelist, url_path): - service_id = sample_service_whitelist.service_id +def test_get_guest_list_returns_data(client, sample_service_guest_list, url_path): + service_id = sample_service_guest_list.service_id response = client.get(url_path.format(service_id), headers=[create_authorization_header()]) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == { - 'email_addresses': [sample_service_whitelist.recipient], + 'email_addresses': [sample_service_guest_list.recipient], 'phone_numbers': [] } -def test_get_whitelist_separates_emails_and_phones(client, sample_service): +def test_get_guest_list_separates_emails_and_phones(client, sample_service): dao_add_and_commit_guest_list_contacts([ ServiceGuestList.from_string(sample_service.id, EMAIL_TYPE, 'service@example.com'), ServiceGuestList.from_string(sample_service.id, MOBILE_TYPE, '07123456789'), @@ -40,7 +40,7 @@ def test_get_whitelist_separates_emails_and_phones(client, sample_service): assert sorted(json_resp['phone_numbers']) == sorted(['+1800-555-555', '07123456789']) -def test_get_whitelist_404s_with_unknown_service_id(client): +def test_get_guest_list_404s_with_unknown_service_id(client): path = 'service/{}/guest-list'.format(uuid.uuid4()) response = client.get(path, headers=[create_authorization_header()]) @@ -50,7 +50,7 @@ def test_get_whitelist_404s_with_unknown_service_id(client): assert json_resp['message'] == 'No result found' -def test_get_whitelist_returns_no_data(client, sample_service): +def test_get_guest_list_returns_no_data(client, sample_service): path = 'service/{}/guest-list'.format(sample_service.id) response = client.get(path, headers=[create_authorization_header()]) @@ -63,26 +63,26 @@ def test_get_whitelist_returns_no_data(client, sample_service): 'service/{}/whitelist', 'service/{}/guest-list', )) -def test_update_whitelist_replaces_old_whitelist(client, sample_service_whitelist, url_path): +def test_update_guest_list_replaces_old_guest_list(client, sample_service_guest_list, url_path): data = { 'email_addresses': ['foo@bar.com'], 'phone_numbers': ['07123456789'] } response = client.put( - url_path.format(sample_service_whitelist.service_id), + url_path.format(sample_service_guest_list.service_id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) assert response.status_code == 204 - whitelist = ServiceGuestList.query.order_by(ServiceGuestList.recipient).all() - assert len(whitelist) == 2 - assert whitelist[0].recipient == '07123456789' - assert whitelist[1].recipient == 'foo@bar.com' + guest_list = ServiceGuestList.query.order_by(ServiceGuestList.recipient).all() + assert len(guest_list) == 2 + assert guest_list[0].recipient == '07123456789' + assert guest_list[1].recipient == 'foo@bar.com' -def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_service_whitelist): +def test_update_guest_list_doesnt_remove_old_guest_list_if_error(client, sample_service_guest_list): data = { 'email_addresses': [''], @@ -90,7 +90,7 @@ def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_se } response = client.put( - 'service/{}/guest-list'.format(sample_service_whitelist.service_id), + 'service/{}/guest-list'.format(sample_service_guest_list.service_id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) @@ -100,5 +100,5 @@ def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_se 'result': 'error', 'message': 'Invalid guest list: "" is not a valid email address or phone number' } - whitelist = ServiceGuestList.query.one() - assert whitelist.id == sample_service_whitelist.id + guest_list = ServiceGuestList.query.one() + assert guest_list.id == sample_service_guest_list.id diff --git a/tests/app/test_model.py b/tests/app/test_model.py index e44d50221..10e0cd5cc 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -37,19 +37,19 @@ from tests.app.db import ( '07700 900678', '+44 7700 900678' ]) -def test_should_build_service_whitelist_from_mobile_number(mobile_number): - service_whitelist = ServiceGuestList.from_string('service_id', MOBILE_TYPE, mobile_number) +def test_should_build_service_guest_list_from_mobile_number(mobile_number): + service_guest_list = ServiceGuestList.from_string('service_id', MOBILE_TYPE, mobile_number) - assert service_whitelist.recipient == mobile_number + assert service_guest_list.recipient == mobile_number @pytest.mark.parametrize('email_address', [ 'test@example.com' ]) -def test_should_build_service_whitelist_from_email_address(email_address): - service_whitelist = ServiceGuestList.from_string('service_id', EMAIL_TYPE, email_address) +def test_should_build_service_guest_list_from_email_address(email_address): + service_guest_list = ServiceGuestList.from_string('service_id', EMAIL_TYPE, email_address) - assert service_whitelist.recipient == email_address + assert service_guest_list.recipient == email_address @pytest.mark.parametrize('contact, recipient_type', [ @@ -57,7 +57,7 @@ def test_should_build_service_whitelist_from_email_address(email_address): ('07700dsadsad', MOBILE_TYPE), ('gmail.com', EMAIL_TYPE) ]) -def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, contact): +def test_should_not_build_service_guest_list_from_invalid_contact(recipient_type, contact): with pytest.raises(ValueError): ServiceGuestList.from_string('service_id', recipient_type, contact) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 6d1ea71c2..bdb33b7bc 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -652,7 +652,7 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification( @pytest.mark.parametrize('restricted', [True, False]) -def test_post_sms_notification_returns_400_if_number_not_whitelisted( +def test_post_sms_notification_returns_400_if_number_not_in_guest_list( notify_db_session, client, restricted ): service = create_service(restricted=restricted, service_permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE]) From b19451c7c690f7d4782a28b07de6574b6337172e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 11:28:19 +0100 Subject: [PATCH 09/10] Rename DAO file To reflect new name of feature --- app/dao/{service_whitelist_dao.py => service_guest_list_dao.py} | 0 app/service/rest.py | 2 +- tests/app/dao/test_service_whitelist_dao.py | 2 +- tests/app/service/test_send_one_off_notification.py | 2 +- tests/app/service/test_service_whitelist.py | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename app/dao/{service_whitelist_dao.py => service_guest_list_dao.py} (100%) diff --git a/app/dao/service_whitelist_dao.py b/app/dao/service_guest_list_dao.py similarity index 100% rename from app/dao/service_whitelist_dao.py rename to app/dao/service_guest_list_dao.py diff --git a/app/service/rest.py b/app/service/rest.py index 1f794eb0b..c4812abfd 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -74,7 +74,7 @@ from app.dao.services_dao import ( dao_update_service, get_services_by_partial_name, ) -from app.dao.service_whitelist_dao import ( +from app.dao.service_guest_list_dao import ( dao_fetch_service_guest_list, dao_add_and_commit_guest_list_contacts, dao_remove_service_guest_list diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py index 695f98ed7..f38e9482b 100644 --- a/tests/app/dao/test_service_whitelist_dao.py +++ b/tests/app/dao/test_service_whitelist_dao.py @@ -5,7 +5,7 @@ from app.models import ( EMAIL_TYPE, ) -from app.dao.service_whitelist_dao import ( +from app.dao.service_guest_list_dao import ( dao_fetch_service_guest_list, dao_add_and_commit_guest_list_contacts, dao_remove_service_guest_list diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 685d42286..04d09c532 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -7,7 +7,7 @@ from notifications_utils.recipients import InvalidPhoneError from app.v2.errors import BadRequestError, TooManyRequestsError from app.config import QueueNames -from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts +from app.dao.service_guest_list_dao import dao_add_and_commit_guest_list_contacts from app.service.send_notification import send_one_off_notification from app.models import ( EMAIL_TYPE, diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index 9f7c8b6dd..ddfda321c 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -8,7 +8,7 @@ from app.models import ( ServiceGuestList, MOBILE_TYPE, EMAIL_TYPE) -from app.dao.service_whitelist_dao import dao_add_and_commit_guest_list_contacts +from app.dao.service_guest_list_dao import dao_add_and_commit_guest_list_contacts @pytest.mark.parametrize('url_path', ( From 5b1b82030ddfd9b06fa2741ab8f9f598ed1b99c0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Jul 2020 11:30:58 +0100 Subject: [PATCH 10/10] Rename test files To reflect new name of feature. --- ...st_service_whitelist_dao.py => test_service_guest_list_dao.py} | 0 .../{test_service_whitelist.py => test_service_guest_list.py} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/app/dao/{test_service_whitelist_dao.py => test_service_guest_list_dao.py} (100%) rename tests/app/service/{test_service_whitelist.py => test_service_guest_list.py} (100%) diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_guest_list_dao.py similarity index 100% rename from tests/app/dao/test_service_whitelist_dao.py rename to tests/app/dao/test_service_guest_list_dao.py diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_guest_list.py similarity index 100% rename from tests/app/service/test_service_whitelist.py rename to tests/app/service/test_service_guest_list.py