More cleanup happening.

Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
This commit is contained in:
Cliff Hill
2024-02-16 15:49:46 -05:00
parent cf1db4dbf4
commit 287dcd6a46
6 changed files with 261 additions and 183 deletions

View File

@@ -11,7 +11,7 @@ from notifications_utils.recipients import (
)
from sqlalchemy.exc import SQLAlchemyError
from app.enums import ServicePermissionType, TemplateType
from app.enums import KeyType, NotificationType, ServicePermissionType, TemplateType
from app.models import Notification, NotificationHistory
from app.notifications.process_notifications import (
create_content_for_notification,
@@ -79,7 +79,7 @@ def test_persist_notification_creates_and_save_to_db(
recipient="+447111111111",
service=sample_template.service,
personalisation={},
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
job_id=sample_job.id,
@@ -123,7 +123,7 @@ def test_persist_notification_throws_exception_when_missing_template(sample_api_
recipient="+447111111111",
service=sample_api_key.service,
personalisation=None,
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
)
@@ -143,7 +143,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key):
recipient="+12028675309",
service=sample_job.service,
personalisation=None,
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
created_at=created_at,
@@ -181,7 +181,7 @@ def test_persist_notification_cache_is_not_incremented_on_failure_to_create_noti
recipient="+447111111111",
service=sample_api_key.service,
personalisation=None,
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
)
@@ -191,20 +191,38 @@ def test_persist_notification_cache_is_not_incremented_on_failure_to_create_noti
@pytest.mark.parametrize(
("requested_queue, notification_type, key_type, expected_queue, expected_task"),
[
(None, "sms", "normal", "send-sms-tasks", "provider_tasks.deliver_sms"),
(None, "email", "normal", "send-email-tasks", "provider_tasks.deliver_email"),
(None, "sms", "team", "send-sms-tasks", "provider_tasks.deliver_sms"),
(
None,
NotificationType.SMS,
KeyType.NORMAL,
"send-sms-tasks",
"provider_tasks.deliver_sms",
),
(
None,
NotificationType.EMAIL,
KeyType.NORMAL,
"send-email-tasks",
"provider_tasks.deliver_email",
),
(
None,
NotificationType.SMS,
KeyType.TEAM,
"send-sms-tasks",
"provider_tasks.deliver_sms",
),
(
"notify-internal-tasks",
"sms",
"normal",
NotificationType.SMS,
KeyType.NORMAL,
"notify-internal-tasks",
"provider_tasks.deliver_sms",
),
(
"notify-internal-tasks",
"email",
"normal",
NotificationType.EMAIL,
KeyType.NORMAL,
"notify-internal-tasks",
"provider_tasks.deliver_email",
),
@@ -245,7 +263,8 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(
with pytest.raises(Boto3Error):
send_notification_to_queue(sample_notification, False)
mocked.assert_called_once_with(
[(str(sample_notification.id))], queue="send-sms-tasks"
[(str(sample_notification.id))],
queue="send-sms-tasks",
)
assert Notification.query.count() == 0
@@ -255,13 +274,13 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(
@pytest.mark.parametrize(
"to_address, notification_type, expected",
[
("+14254147755", "sms", True),
("+14254147167", "sms", True),
("simulate-delivered@notifications.service.gov.uk", "email", True),
("simulate-delivered-2@notifications.service.gov.uk", "email", True),
("simulate-delivered-3@notifications.service.gov.uk", "email", True),
("2028675309", "sms", False),
("valid_email@test.com", "email", False),
("+14254147755", NotificationType.SMS, True),
("+14254147167", NotificationType.SMS, True),
("simulate-delivered@notifications.service.gov.uk", NotificationType.EMAIL, True),
("simulate-delivered-2@notifications.service.gov.uk", NotificationType.EMAIL, True),
("simulate-delivered-3@notifications.service.gov.uk", NotificationType.EMAIL, True),
("2028675309", NotificationType.SMS, False),
("valid_email@test.com", NotificationType.EMAIL, False),
],
)
def test_simulated_recipient(notify_api, to_address, notification_type, expected):
@@ -277,7 +296,7 @@ def test_simulated_recipient(notify_api, to_address, notification_type, expected
"""
formatted_address = None
if notification_type == "email":
if notification_type == NotificationType.EMAIL:
formatted_address = validate_and_format_email_address(to_address)
else:
formatted_address = validate_and_format_phone_number(to_address)
@@ -311,7 +330,7 @@ def test_persist_notification_with_international_info_stores_correct_info(
recipient=recipient,
service=sample_job.service,
personalisation=None,
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
job_id=sample_job.id,
@@ -334,7 +353,7 @@ def test_persist_notification_with_international_info_does_not_store_for_email(
recipient="foo@bar.com",
service=sample_job.service,
personalisation=None,
notification_type="email",
notification_type=NotificationType.EMAIL,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
job_id=sample_job.id,
@@ -368,7 +387,7 @@ def test_persist_sms_notification_stores_normalised_number(
recipient=recipient,
service=sample_job.service,
personalisation=None,
notification_type="sms",
notification_type=NotificationType.SMS,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
job_id=sample_job.id,
@@ -392,7 +411,7 @@ def test_persist_email_notification_stores_normalised_email(
recipient=recipient,
service=sample_job.service,
personalisation=None,
notification_type="email",
notification_type=NotificationType.EMAIL,
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,
job_id=sample_job.id,
@@ -415,7 +434,7 @@ def test_persist_notification_with_billable_units_stores_correct_info(mocker):
personalisation=None,
notification_type=template.template_type,
api_key_id=None,
key_type="normal",
key_type=KeyType.NORMAL,
billable_units=3,
)
persisted_notification = Notification.query.all()[0]

View File

@@ -8,19 +8,19 @@ from notifications_python_client.authentication import create_jwt_token
from app.dao.api_key_dao import save_model_api_key
from app.dao.notifications_dao import dao_update_notification
from app.dao.templates_dao import dao_update_template
from app.enums import KeyType
from app.enums import KeyType, NotificationStatus, NotificationType, TemplateType
from app.models import ApiKey
from tests import create_service_authorization_header
from tests.app.db import create_api_key, create_notification
@pytest.mark.parametrize("type", ("email", "sms"))
@pytest.mark.parametrize("type", (NotificationType.EMAIL, NotificationType.SMS))
def test_get_notification_by_id(
client, sample_notification, sample_email_notification, type
):
if type == "email":
if type == NotificationType.EMAIL:
notification_to_get = sample_email_notification
if type == "sms":
elif type == NotificationType.SMS:
notification_to_get = sample_notification
auth_header = create_service_authorization_header(
@@ -46,13 +46,13 @@ def test_get_notification_by_id(
@pytest.mark.parametrize("id", ["1234-badly-formatted-id-7890", "0"])
@pytest.mark.parametrize("type", ("email", "sms"))
@pytest.mark.parametrize("type", (NotificationType.EMAIL, NotificationType.SMS))
def test_get_notification_by_invalid_id(
client, sample_notification, sample_email_notification, id, type
):
if type == "email":
if type == NotificationType.EMAIL:
notification_to_get = sample_email_notification
if type == "sms":
elif type == NotificationType.SMS:
notification_to_get = sample_notification
auth_header = create_service_authorization_header(
service_id=notification_to_get.service_id
@@ -420,7 +420,10 @@ def test_filter_by_template_type(client, sample_template, sample_email_template)
notifications = json.loads(response.get_data(as_text=True))
assert len(notifications["notifications"]) == 1
assert notifications["notifications"][0]["template"]["template_type"] == "sms"
assert (
notifications["notifications"][0]["template"]["template_type"]
== TemplateType.SMS
)
assert response.status_code == 200
@@ -441,13 +444,13 @@ def test_filter_by_multiple_template_types(
assert response.status_code == 200
notifications = json.loads(response.get_data(as_text=True))
assert len(notifications["notifications"]) == 2
assert {"sms", "email"} == set(
assert {TemplateType.SMS, TemplateType.EMAIL} == {
x["template"]["template_type"] for x in notifications["notifications"]
)
}
def test_filter_by_status(client, sample_email_template):
create_notification(sample_email_template, status="delivered")
create_notification(sample_email_template, status=NotificationStatus.DELIVERED)
create_notification(sample_email_template)
auth_header = create_service_authorization_header(
@@ -458,13 +461,13 @@ def test_filter_by_status(client, sample_email_template):
notifications = json.loads(response.get_data(as_text=True))
assert len(notifications["notifications"]) == 1
assert notifications["notifications"][0]["status"] == "delivered"
assert notifications["notifications"][0]["status"] == NotificationStatus.DELIVERED
assert response.status_code == 200
def test_filter_by_multiple_statuses(client, sample_email_template):
create_notification(sample_email_template, status="delivered")
create_notification(sample_email_template, status="sending")
create_notification(sample_email_template, status=NotificationStatus.DELIVERED)
create_notification(sample_email_template, status=NotificationStatus.SENDING)
auth_header = create_service_authorization_header(
service_id=sample_email_template.service_id
@@ -477,9 +480,9 @@ def test_filter_by_multiple_statuses(client, sample_email_template):
assert response.status_code == 200
notifications = json.loads(response.get_data(as_text=True))
assert len(notifications["notifications"]) == 2
assert {"delivered", "sending"} == set(
assert {NotificationStatus.DELIVERED, NotificationStatus.SENDING} == {
x["status"] for x in notifications["notifications"]
)
}
def test_filter_by_status_and_template_type(
@@ -487,7 +490,7 @@ def test_filter_by_status_and_template_type(
):
create_notification(sample_template)
create_notification(sample_email_template)
create_notification(sample_email_template, status="delivered")
create_notification(sample_email_template, status=NotificationStatus.DELIVERED)
auth_header = create_service_authorization_header(
service_id=sample_email_template.service_id
@@ -500,8 +503,11 @@ def test_filter_by_status_and_template_type(
notifications = json.loads(response.get_data(as_text=True))
assert response.status_code == 200
assert len(notifications["notifications"]) == 1
assert notifications["notifications"][0]["template"]["template_type"] == "email"
assert notifications["notifications"][0]["status"] == "delivered"
assert (
notifications["notifications"][0]["template"]["template_type"]
== TemplateType.EMAIL
)
assert notifications["notifications"][0]["status"] == NotificationStatus.DELIVERED
def test_get_notification_by_id_returns_merged_template_content(

View File

@@ -54,7 +54,7 @@ def enable_redis(notify_api):
yield
@pytest.mark.parametrize("key_type", ["team", "normal"])
@pytest.mark.parametrize("key_type", [KeyType.TEAM, KeyType.NORMAL])
def test_check_service_over_total_message_limit_fails(
key_type, mocker, notify_db_session
):
@@ -71,7 +71,7 @@ def test_check_service_over_total_message_limit_fails(
assert e.value.fields == []
@pytest.mark.parametrize("key_type", ["team", "normal"])
@pytest.mark.parametrize("key_type", [KeyType.TEAM, KeyType.NORMAL])
def test_check_application_over_retention_limit_fails(
key_type, mocker, notify_db_session
):
@@ -142,7 +142,7 @@ def test_check_template_is_active_fails(sample_template):
assert e.value.fields == [{"template": "Template has been deleted"}]
@pytest.mark.parametrize("key_type", ["test", "normal"])
@pytest.mark.parametrize("key_type", [KeyType.TEST, KeyType.NORMAL])
def test_service_can_send_to_recipient_passes(key_type, notify_db_session):
trial_mode_service = create_service(service_name="trial mode", restricted=True)
serialised_service = SerialisedService.from_id(trial_mode_service.id)
@@ -175,7 +175,11 @@ def test_service_can_send_to_recipient_passes_with_non_normalized_number(
serialised_service = SerialisedService.from_id(sample_service.id)
assert (
service_can_send_to_recipient(recipient_number, "team", serialised_service)
service_can_send_to_recipient(
recipient_number,
KeyType.TEAM,
serialised_service,
)
is None
)
@@ -194,12 +198,12 @@ def test_service_can_send_to_recipient_passes_with_non_normalized_email(
serialised_service = SerialisedService.from_id(sample_service.id)
assert (
service_can_send_to_recipient(recipient_email, "team", serialised_service)
service_can_send_to_recipient(recipient_email, KeyType.TEAM, serialised_service)
is None
)
@pytest.mark.parametrize("key_type", ["test", "normal"])
@pytest.mark.parametrize("key_type", [KeyType.TEST, KeyType.NORMAL])
def test_service_can_send_to_recipient_passes_for_live_service_non_team_member(
key_type, sample_service
):
@@ -222,12 +226,19 @@ def test_service_can_send_to_recipient_passes_for_guest_list_recipient_passes(
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
"some_other_email@test.com", KeyType.TEAM, sample_service
)
is None
)
create_service_guest_list(sample_service, mobile_number="2028675309")
assert service_can_send_to_recipient("2028675309", "team", sample_service) is None
assert (
service_can_send_to_recipient(
"2028675309",
KeyType.TEAM,
sample_service,
)
is None
)
@pytest.mark.parametrize(
@@ -246,7 +257,7 @@ def test_service_can_send_to_recipient_fails_when_ignoring_guest_list(
with pytest.raises(BadRequestError) as exec_info:
service_can_send_to_recipient(
next(iter(recipient.values())),
"team",
KeyType.TEAM,
sample_service,
allow_guest_list_recipients=False,
)
@@ -262,9 +273,9 @@ def test_service_can_send_to_recipient_fails_when_ignoring_guest_list(
@pytest.mark.parametrize(
"key_type, error_message",
[
("team", "Cant send to this recipient using a team-only API key"),
(KeyType.TEAM, "Cant send to this recipient using a team-only API key"),
(
"normal",
KeyType.NORMAL,
"Cant send to this recipient when service is in trial mode see https://www.notifications.service.gov.uk/trial-mode", # noqa
),
],
@@ -287,7 +298,7 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(
sample_service,
):
with pytest.raises(BadRequestError) as e:
service_can_send_to_recipient("0758964221", "team", sample_service)
service_can_send_to_recipient("0758964221", KeyType.TEAM, sample_service)
assert e.value.status_code == 400
assert e.value.message == "Cant send to this recipient using a team-only API key"
assert e.value.fields == []
@@ -295,7 +306,7 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(
@pytest.mark.parametrize("char_count", [612, 0, 494, 200, 918])
@pytest.mark.parametrize("show_prefix", [True, False])
@pytest.mark.parametrize("template_type", ["sms", "email"])
@pytest.mark.parametrize("template_type", [TemplateType.SMS, TemplateType.EMAIL])
def test_check_is_message_too_long_passes(
notify_db_session, show_prefix, char_count, template_type
):
@@ -316,7 +327,7 @@ def test_check_is_message_too_long_fails(notify_db_session, show_prefix, char_co
with pytest.raises(BadRequestError) as e:
service = create_service(prefix_sms=show_prefix)
t = create_template(
service=service, content="a" * char_count, template_type="sms"
service=service, content="a" * char_count, template_type=TemplateType.SMS
)
template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=t.id, service_id=service.id
@@ -340,7 +351,7 @@ def test_check_is_message_too_long_passes_for_long_email(sample_service):
t = create_template(
service=sample_service,
content="a" * email_character_count,
template_type="email",
template_type=TemplateType.EMAIL,
)
template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=t.id, service_id=t.service_id
@@ -392,15 +403,15 @@ def test_check_notification_content_is_not_empty_fails(
def test_validate_template(sample_service):
template = create_template(sample_service, template_type="email")
validate_template(template.id, {}, sample_service, "email")
template = create_template(sample_service, template_type=TemplateType.EMAIL)
validate_template(template.id, {}, sample_service, NotificationType.EMAIL)
@pytest.mark.parametrize("check_char_count", [True, False])
def test_validate_template_calls_all_validators(
mocker, fake_uuid, sample_service, check_char_count
):
template = create_template(sample_service, template_type="email")
template = create_template(sample_service, template_type=TemplateType.EMAIL)
mock_check_type = mocker.patch(
"app.notifications.validators.check_template_is_for_notification_type"
)
@@ -418,10 +429,14 @@ def test_validate_template_calls_all_validators(
"app.notifications.validators.check_is_message_too_long"
)
template, template_with_content = validate_template(
template.id, {}, sample_service, "email", check_char_count=check_char_count
template.id,
{},
sample_service,
NotificationType.EMAIL,
check_char_count=check_char_count,
)
mock_check_type.assert_called_once_with("email", "email")
mock_check_type.assert_called_once_with(NotificationType.EMAIL, TemplateType.EMAIL)
mock_check_if_active.assert_called_once_with(template)
mock_create_conent.assert_called_once_with(template, {})
mock_check_not_empty.assert_called_once_with("content")
@@ -434,7 +449,7 @@ def test_validate_template_calls_all_validators(
def test_validate_template_calls_all_validators_exception_message_too_long(
mocker, fake_uuid, sample_service
):
template = create_template(sample_service, template_type="email")
template = create_template(sample_service, template_type=TemplateType.EMAIL)
mock_check_type = mocker.patch(
"app.notifications.validators.check_template_is_for_notification_type"
)
@@ -452,7 +467,11 @@ def test_validate_template_calls_all_validators_exception_message_too_long(
"app.notifications.validators.check_is_message_too_long"
)
template, template_with_content = validate_template(
template.id, {}, sample_service, "email", check_char_count=False
template.id,
{},
sample_service,
NotificationType.EMAIL,
check_char_count=False,
)
mock_check_type.assert_called_once_with("email", "email")
@@ -462,20 +481,15 @@ def test_validate_template_calls_all_validators_exception_message_too_long(
assert not mock_check_message_is_too_long.called
@pytest.mark.parametrize("key_type", ["team", "live", "test"])
@pytest.mark.parametrize("key_type", [KeyType.TEAM, KeyType.NORMAL, KeyType.TEST])
def test_check_service_over_api_rate_limit_when_exceed_rate_limit_request_fails_raises_error(
key_type, sample_service, mocker
):
with freeze_time("2016-01-01 12:00:00.000000"):
if key_type == "live":
api_key_type = "normal"
else:
api_key_type = key_type
mocker.patch("app.redis_store.exceeded_rate_limit", return_value=True)
sample_service.restricted = True
api_key = create_api_key(sample_service, key_type=api_key_type)
api_key = create_api_key(sample_service, key_type=key_type)
serialised_service = SerialisedService.from_id(sample_service.id)
serialised_api_key = SerialisedAPIKeyCollection.from_service_id(
serialised_service.id
@@ -490,17 +504,16 @@ def test_check_service_over_api_rate_limit_when_exceed_rate_limit_request_fails_
60,
)
assert e.value.status_code == 429
assert (
e.value.message
== "Exceeded rate limit for key type {} of {} requests per {} seconds".format(
key_type.upper(), sample_service.rate_limit, 60
)
assert e.value.message == (
f"Exceeded rate limit for key type {key_type.upper()} of "
f"{sample_service.rate_limit} requests per {60} seconds"
)
assert e.value.fields == []
def test_check_service_over_api_rate_limit_when_rate_limit_has_not_exceeded_limit_succeeds(
sample_service, mocker
sample_service,
mocker,
):
with freeze_time("2016-01-01 12:00:00.000000"):
mocker.patch("app.redis_store.exceeded_rate_limit", return_value=False)
@@ -551,7 +564,7 @@ def test_check_rate_limiting_validates_api_rate_limit_and_daily_limit(
mock_rate_limit.assert_called_once_with(service, api_key)
@pytest.mark.parametrize("key_type", ["test", "normal"])
@pytest.mark.parametrize("key_type", [KeyType.TEST, KeyType.NORMAL])
def test_validate_and_format_recipient_fails_when_international_number_and_service_does_not_allow_int_sms(
key_type,
notify_db_session,
@@ -590,7 +603,10 @@ def test_validate_and_format_recipient_fails_when_no_recipient():
assert e.value.message == "Recipient can't be empty"
@pytest.mark.parametrize("notification_type", ["sms", "email"])
@pytest.mark.parametrize(
"notification_type",
[NotificationType.SMS, NotificationType.EMAIL],
)
def test_check_service_email_reply_to_id_where_reply_to_id_is_none(notification_type):
assert check_service_email_reply_to_id(None, None, notification_type) is None
@@ -638,7 +654,10 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(
)
@pytest.mark.parametrize("notification_type", ["sms", "email"])
@pytest.mark.parametrize(
"notification_type",
[NotificationType.SMS, NotificationType.EMAIL],
)
def test_check_service_sms_sender_id_where_sms_sender_id_is_none(notification_type):
assert check_service_sms_sender_id(None, None, notification_type) is None
@@ -684,7 +703,10 @@ def test_check_service_sms_sender_id_where_sms_sender_is_not_found(
)
@pytest.mark.parametrize("notification_type", ["sms", "email"])
@pytest.mark.parametrize(
"notification_type",
[NotificationType.SMS, NotificationType.EMAIL],
)
def test_check_reply_to_with_empty_reply_to(sample_service, notification_type):
assert check_reply_to(sample_service.id, None, notification_type) is None
@@ -778,6 +800,7 @@ def test_check_service_over_total_message_limit(mocker, sample_service):
get_redis_mock = mocker.patch("app.notifications.validators.redis_store.get")
get_redis_mock.return_value = None
service_stats = check_service_over_total_message_limit(
KeyType.NORMAL, sample_service
KeyType.NORMAL,
sample_service,
)
assert service_stats == 0