Merge pull request #2928 from alphagov/serve-on-slash-guest-list

Rename API URLs for guest list to guest list
This commit is contained in:
Chris Hill-Scott
2020-08-03 16:44:41 +01:00
committed by GitHub
19 changed files with 265 additions and 254 deletions

View File

@@ -33,7 +33,7 @@ from app.models import (
ProviderDetails,
ProviderDetailsHistory,
ProviderRates,
ServiceWhitelist,
ServiceGuestList,
KEY_TYPE_NORMAL,
KEY_TYPE_TEST,
KEY_TYPE_TEAM,
@@ -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 = ServiceWhitelist.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

View File

@@ -0,0 +1,56 @@
import uuid
from app.models import (
ServiceGuestList,
EMAIL_TYPE,
)
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
)
from tests.app.db import create_service
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_guest_list_ignores_other_service(sample_service_guest_list):
assert len(dao_fetch_service_guest_list(uuid.uuid4())) == 0
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([guest_list])
db_contents = ServiceGuestList.query.all()
assert len(db_contents) == 1
assert db_contents[0].id == guest_list.id
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([
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)
assert service_1.guest_list == []
assert len(service_2.guest_list) == 1
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()
assert ServiceGuestList.query.count() == 1

View File

@@ -1,56 +0,0 @@
import uuid
from app.models import (
ServiceWhitelist,
EMAIL_TYPE,
)
from app.dao.service_whitelist_dao import (
dao_fetch_service_whitelist,
dao_add_and_commit_whitelisted_contacts,
dao_remove_service_whitelist
)
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)
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
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])
db_contents = ServiceWhitelist.query.all()
assert len(db_contents) == 1
assert db_contents[0].id == whitelist.id
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([
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)
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)
# since dao_remove_service_whitelist doesn't commit, we can still rollback its changes
notify_db.session.rollback()
assert ServiceWhitelist.query.count() == 1

View File

@@ -40,7 +40,7 @@ from app.models import (
ServiceLetterContact,
ServicePermission,
ServiceSmsSender,
ServiceWhitelist,
ServiceGuestList,
Template,
User,
EMAIL_TYPE,
@@ -740,17 +740,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 = ServiceWhitelist.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 = ServiceWhitelist.from_string(service.id, MOBILE_TYPE, mobile_number)
guest_list_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')
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,

View File

@@ -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,24 +776,24 @@ 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 to not in [member.recipient for member in service.whitelist]
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,12 +847,12 @@ 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 to in [member.recipient for member in sample_service.whitelist]
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)

View File

@@ -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,19 +260,19 @@ 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())),
'team',
sample_service,
allow_whitelisted_recipients=False,
allow_guest_list_recipients=False,
)
assert exec_info.value.status_code == 400
assert exec_info.value.message == 'Cant send to this recipient using a team-only API key'

View File

@@ -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_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,
@@ -17,7 +17,7 @@ from app.models import (
PRIORITY,
SMS_TYPE,
Notification,
ServiceWhitelist,
ServiceGuestList,
)
from tests.app.db import (
@@ -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,
@@ -269,8 +269,8 @@ 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([
ServiceWhitelist.from_string(service.id, MOBILE_TYPE, '07700900123'),
dao_add_and_commit_guest_list_contacts([
ServiceGuestList.from_string(service.id, MOBILE_TYPE, '07700900123'),
])
post_data = {

View File

@@ -0,0 +1,104 @@
import pytest
import uuid
import json
from tests import create_authorization_header
from app.models import (
ServiceGuestList,
MOBILE_TYPE, EMAIL_TYPE)
from app.dao.service_guest_list_dao import dao_add_and_commit_guest_list_contacts
@pytest.mark.parametrize('url_path', (
'service/{}/whitelist',
'service/{}/guest-list',
))
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_guest_list.recipient],
'phone_numbers': []
}
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'),
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()])
assert response.status_code == 200
json_resp = json.loads(response.get_data(as_text=True))
assert json_resp['email_addresses'] == ['service@example.com']
assert sorted(json_resp['phone_numbers']) == sorted(['+1800-555-555', '07123456789'])
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()])
assert response.status_code == 404
json_resp = json.loads(response.get_data(as_text=True))
assert json_resp['result'] == 'error'
assert json_resp['message'] == 'No result found'
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()])
assert response.status_code == 200
assert json.loads(response.get_data(as_text=True)) == {'email_addresses': [], 'phone_numbers': []}
@pytest.mark.parametrize('url_path', (
'service/{}/whitelist',
'service/{}/guest-list',
))
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_guest_list.service_id),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
assert response.status_code == 204
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_guest_list_doesnt_remove_old_guest_list_if_error(client, sample_service_guest_list):
data = {
'email_addresses': [''],
'phone_numbers': ['07123456789']
}
response = client.put(
'service/{}/guest-list'.format(sample_service_guest_list.service_id),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
assert response.status_code == 400
assert json.loads(response.get_data(as_text=True)) == {
'result': 'error',
'message': 'Invalid guest list: "" is not a valid email address or phone number'
}
guest_list = ServiceGuestList.query.one()
assert guest_list.id == sample_service_guest_list.id

View File

@@ -1,95 +0,0 @@
import uuid
import json
from tests import create_authorization_header
from app.models import (
ServiceWhitelist,
MOBILE_TYPE, EMAIL_TYPE)
from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts
def test_get_whitelist_returns_data(client, sample_service_whitelist):
service_id = sample_service_whitelist.service_id
response = client.get('service/{}/whitelist'.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],
'phone_numbers': []
}
def test_get_whitelist_separates_emails_and_phones(client, sample_service):
dao_add_and_commit_whitelisted_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'),
])
response = client.get('service/{}/whitelist'.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']
assert sorted(json_resp['phone_numbers']) == sorted(['+1800-555-555', '07123456789'])
def test_get_whitelist_404s_with_unknown_service_id(client):
path = 'service/{}/whitelist'.format(uuid.uuid4())
response = client.get(path, headers=[create_authorization_header()])
assert response.status_code == 404
json_resp = json.loads(response.get_data(as_text=True))
assert json_resp['result'] == 'error'
assert json_resp['message'] == 'No result found'
def test_get_whitelist_returns_no_data(client, sample_service):
path = 'service/{}/whitelist'.format(sample_service.id)
response = client.get(path, headers=[create_authorization_header()])
assert response.status_code == 200
assert json.loads(response.get_data(as_text=True)) == {'email_addresses': [], 'phone_numbers': []}
def test_update_whitelist_replaces_old_whitelist(client, sample_service_whitelist):
data = {
'email_addresses': ['foo@bar.com'],
'phone_numbers': ['07123456789']
}
response = client.put(
'service/{}/whitelist'.format(sample_service_whitelist.service_id),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
assert response.status_code == 204
whitelist = ServiceWhitelist.query.order_by(ServiceWhitelist.recipient).all()
assert len(whitelist) == 2
assert whitelist[0].recipient == '07123456789'
assert whitelist[1].recipient == 'foo@bar.com'
def test_update_whitelist_doesnt_remove_old_whitelist_if_error(client, sample_service_whitelist):
data = {
'email_addresses': [''],
'phone_numbers': ['07123456789']
}
response = client.put(
'service/{}/whitelist'.format(sample_service_whitelist.service_id),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
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'
}
whitelist = ServiceWhitelist.query.one()
assert whitelist.id == sample_service_whitelist.id

View File

@@ -5,7 +5,7 @@ from sqlalchemy.exc import IntegrityError
from app import encryption
from app.models import (
ServiceWhitelist,
ServiceGuestList,
Notification,
SMS_TYPE,
MOBILE_TYPE,
@@ -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 = ServiceWhitelist.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 = ServiceWhitelist.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,9 +57,9 @@ 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):
ServiceWhitelist.from_string('service_id', recipient_type, contact)
ServiceGuestList.from_string('service_id', recipient_type, contact)
@pytest.mark.parametrize('initial_statuses, expected_statuses', [

View File

@@ -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])