Refactor code

This commit is contained in:
Ken Tsang
2017-06-27 12:00:39 +01:00
committed by venusbb
parent 815f4d0a81
commit 542bbb2f34
4 changed files with 78 additions and 76 deletions

View File

@@ -10,7 +10,8 @@ from notifications_python_client.authentication import create_jwt_token
import app
from app.dao import notifications_dao
from app.models import (
ApiKey, INTERNATIONAL_SMS_TYPE, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory)
ApiKey, INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE,
KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory)
from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template
from app.dao.services_dao import dao_update_service
from app.dao.api_key_dao import save_model_api_key
@@ -32,7 +33,7 @@ from app.errors import InvalidRequest
@pytest.mark.parametrize('template_type',
['sms', 'email'])
[SMS_TYPE, EMAIL_TYPE])
def test_create_notification_should_reject_if_missing_required_fields(notify_api,
sample_api_key, mocker, template_type):
with notify_api.test_request_context():
@@ -79,8 +80,8 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
@pytest.mark.parametrize('template_type, to',
[('sms', '+447700900855'),
('email', 'ok@ok.com')])
[(SMS_TYPE, '+447700900855'),
(EMAIL_TYPE, 'ok@ok.com')])
def test_send_notification_invalid_template_id(notify_api, sample_template, mocker, fake_uuid, template_type, to):
with notify_api.test_request_context():
with notify_api.test_client() as client:
@@ -217,8 +218,8 @@ def test_should_not_send_notification_for_archived_template(notify_api, sample_t
@pytest.mark.parametrize('template_type, to',
[('sms', '+447700900855'),
('email', 'not-someone-we-trust@email-address.com')])
[(SMS_TYPE, '+447700900855'),
(EMAIL_TYPE, 'not-someone-we-trust@email-address.com')])
def test_should_not_send_notification_if_restricted_and_not_a_service_user(notify_api,
sample_template,
sample_email_template,
@@ -228,7 +229,7 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
template = sample_template if template_type == 'sms' else sample_email_template
template = sample_template if template_type == SMS_TYPE else sample_email_template
template.service.restricted = True
dao_update_service(template.service)
data = {
@@ -254,7 +255,7 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif
@pytest.mark.parametrize('template_type',
['sms', 'email'])
[SMS_TYPE, EMAIL_TYPE])
def test_should_send_notification_if_restricted_and_a_service_user(notify_api,
sample_template,
sample_email_template,
@@ -264,8 +265,8 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api,
with notify_api.test_client() as client:
mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
template = sample_template if template_type == 'sms' else sample_email_template
to = template.service.created_by.mobile_number if template_type == 'sms' \
template = sample_template if template_type == SMS_TYPE else sample_email_template
to = template.service.created_by.mobile_number if template_type == SMS_TYPE \
else template.service.created_by.email_address
template.service.restricted = True
dao_update_service(template.service)
@@ -286,7 +287,7 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api,
@pytest.mark.parametrize('template_type',
['sms', 'email'])
[SMS_TYPE, EMAIL_TYPE])
def test_should_not_allow_template_from_another_service(notify_api,
service_factory,
sample_user,
@@ -299,7 +300,7 @@ def test_should_not_allow_template_from_another_service(notify_api,
service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2')
service_2_templates = dao_get_all_templates_for_service(service_id=service_2.id)
to = sample_user.mobile_number if template_type == 'sms' else sample_user.email_address
to = sample_user.mobile_number if template_type == SMS_TYPE else sample_user.email_address
data = {
'to': to,
'template': service_2_templates[0].id
@@ -692,7 +693,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t
@pytest.mark.parametrize('template_type',
['sms', 'email'])
[SMS_TYPE, EMAIL_TYPE])
def test_should_persist_notification(notify_api, sample_template,
sample_email_template,
template_type,
@@ -700,8 +701,8 @@ def test_should_persist_notification(notify_api, sample_template,
with notify_api.test_request_context(), notify_api.test_client() as client:
mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
template = sample_template if template_type == 'sms' else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == 'sms' \
template = sample_template if template_type == SMS_TYPE else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \
else sample_email_template.service.created_by.email_address
data = {
'to': to,
@@ -730,7 +731,7 @@ def test_should_persist_notification(notify_api, sample_template,
@pytest.mark.parametrize('template_type',
['sms', 'email'])
[SMS_TYPE, EMAIL_TYPE])
def test_should_delete_notification_and_return_error_if_sqs_fails(
client,
sample_email_template,
@@ -743,8 +744,8 @@ def test_should_delete_notification_and_return_error_if_sqs_fails(
side_effect=Exception("failed to talk to SQS")
)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
template = sample_template if template_type == 'sms' else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == 'sms' \
template = sample_template if template_type == SMS_TYPE else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == SMS_TYPE \
else sample_email_template.service.created_by.email_address
data = {
'to': to,
@@ -833,8 +834,8 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number(
KEY_TYPE_NORMAL, KEY_TYPE_TEAM
])
@pytest.mark.parametrize('notification_type, to, _create_sample_template', [
('sms', '07827992635', create_sample_template),
('email', 'non_whitelist_recipient@mail.com', create_sample_email_template)]
(SMS_TYPE, '07827992635', create_sample_template),
(EMAIL_TYPE, 'non_whitelist_recipient@mail.com', create_sample_email_template)]
)
def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode(
client,
@@ -888,8 +889,8 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode(
KEY_TYPE_NORMAL, KEY_TYPE_TEAM
])
@pytest.mark.parametrize('notification_type, to, _create_sample_template', [
('sms', '07123123123', create_sample_template),
('email', 'whitelist_recipient@mail.com', create_sample_email_template)]
(SMS_TYPE, '07123123123', create_sample_template),
(EMAIL_TYPE, 'whitelist_recipient@mail.com', create_sample_email_template)]
)
def test_should_send_notification_to_whitelist_recipient(
client,
@@ -905,10 +906,10 @@ def test_should_send_notification_to_whitelist_recipient(
service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=service_restricted)
apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
template = _create_sample_template(notify_db, notify_db_session, service=service)
if notification_type == 'sms':
if notification_type == SMS_TYPE:
service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session,
service=service, mobile_number=to)
elif notification_type == 'email':
elif notification_type == EMAIL_TYPE:
service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session,
service=service, email_address=to)
@@ -940,8 +941,8 @@ def test_should_send_notification_to_whitelist_recipient(
@pytest.mark.parametrize(
'notification_type, template_type, to', [
('email', 'sms', 'notify@digital.cabinet-office.gov.uk'),
('sms', 'email', '+447700900986')
(EMAIL_TYPE, SMS_TYPE, 'notify@digital.cabinet-office.gov.uk'),
(SMS_TYPE, EMAIL_TYPE, '+447700900986')
])
def test_should_error_if_notification_type_does_not_match_template_type(
client,
@@ -987,8 +988,8 @@ def test_create_template_doesnt_raise_with_too_much_personalisation(
@pytest.mark.parametrize(
'template_type, should_error', [
('sms', True),
('email', False)
(SMS_TYPE, True),
(EMAIL_TYPE, False)
]
)
def test_create_template_raises_invalid_request_when_content_too_large(
@@ -1150,7 +1151,7 @@ def test_should_not_allow_international_number_on_sms_notification(client, sampl
def test_should_allow_international_number_on_sms_notification(client, notify_db, notify_db_session, mocker):
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE, 'sms'])
service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE, SMS_TYPE])
template = create_sample_template(notify_db, notify_db_session, service=service)
data = {

View File

@@ -2,6 +2,7 @@ import pytest
from freezegun import freeze_time
from flask import current_app
import app
from app.models import INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE
from app.notifications.validators import (
check_service_over_daily_message_limit,
check_template_is_for_notification_type,
@@ -116,16 +117,16 @@ def test_check_service_message_limit_in_cache_over_message_limit_fails(
@pytest.mark.parametrize('template_type, notification_type',
[('email', 'email'),
('sms', 'sms')])
[(EMAIL_TYPE, EMAIL_TYPE),
(SMS_TYPE, SMS_TYPE)])
def test_check_template_is_for_notification_type_pass(template_type, notification_type):
assert check_template_is_for_notification_type(notification_type=notification_type,
template_type=template_type) is None
@pytest.mark.parametrize('template_type, notification_type',
[('sms', 'email'),
('email', 'sms')])
[(SMS_TYPE, EMAIL_TYPE),
(EMAIL_TYPE, SMS_TYPE)])
def test_check_template_is_for_notification_type_fails_when_template_type_does_not_match_notification_type(
template_type, notification_type):
with pytest.raises(BadRequestError) as e:
@@ -309,7 +310,7 @@ def test_should_not_rate_limit_if_limiting_is_disabled(
@pytest.mark.parametrize('key_type', ['test', 'normal'])
def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type):
with pytest.raises(BadRequestError) as e:
validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, 'sms')
validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, SMS_TYPE)
assert e.value.status_code == 400
assert e.value.message == 'Cannot send to international mobile numbers'
assert e.value.fields == []
@@ -318,6 +319,6 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_
@pytest.mark.parametrize('key_type', ['test', 'normal'])
def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms(
key_type, notify_db, notify_db_session):
service = create_service(notify_db, notify_db_session, permissions=['sms', 'international_sms'])
result = validate_and_format_recipient('20-12-1234-1234', key_type, service, 'sms')
service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE])
result = validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE)
assert result == '201212341234'