diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index ca3ae0b53..ee03b537e 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -35,6 +35,7 @@ def check_placeholders(template_object): def persist_notification( + *, template_id, template_version, recipient, diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 3aa86c963..83b620506 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -9,7 +9,7 @@ from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_ from app.dao import services_dao, templates_dao from app.models import ( - INTERNATIONAL_SMS_TYPE, SMS_TYPE, + INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS ) from app.service.utils import service_allowed_to_send_to @@ -104,7 +104,7 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type) number=send_to, international=international_phone_info.international ) - else: + elif notification_type == EMAIL_TYPE: return validate_and_format_email_address(email_address=send_to) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index ac73b36da..407ad2093 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -51,10 +51,18 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 - notification = persist_notification(sample_template.id, sample_template.version, '+447111111111', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, job_id=sample_job.id, - job_row_number=100, reference="ref") + notification = persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + job_id=sample_job.id, + job_row_number=100, + reference="ref") assert Notification.query.get(notification.id) is not None assert NotificationHistory.query.get(notification.id) is not None @@ -127,14 +135,14 @@ def test_persist_notification_does_not_increment_cache_if_test_key( assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 0 persist_notification( - sample_template.id, - sample_template.version, - '+447111111111', - sample_template.service, - {}, - 'sms', - api_key.id, - api_key.key_type, + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=api_key.id, + key_type=api_key.key_type, job_id=sample_job.id, job_row_number=100, reference="ref", @@ -193,18 +201,33 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr') mock_incr_hash_value = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value') - persist_notification(sample_template.id, sample_template.version, '+447111111111', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, reference="ref") + persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111111', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + reference="ref" + ) mock_incr.assert_not_called() mock_incr_hash_value.assert_not_called() mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=1) mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash', return_value={sample_template.id, 1}) - persist_notification(sample_template.id, sample_template.version, '+447111111122', - sample_template.service, {}, 'sms', sample_api_key.id, - sample_api_key.key_type, reference="ref2") + persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient='+447111111122', + service=sample_template.service, + personalisation={}, + notification_type='sms', + api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type, + reference="ref2") mock_incr.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count", ) mock_incr_hash_value.assert_called_once_with(cache_key_for_service_template_counter(sample_template.service_id), sample_template.id) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 394c6bc92..7c8f2b34d 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -10,14 +10,17 @@ from tests import create_authorization_header from tests.app.db import create_service, create_template +pytestmark = pytest.mark.skip('Leters not currently implemented') + + def letter_request(client, data, service_id, _expected_status=201): resp = client.post( url_for('v2_notifications.post_notification', notification_type='letter'), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service_id)] ) - assert resp.status_code == _expected_status json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == _expected_status, json_resp return json_resp @@ -76,7 +79,6 @@ def test_post_letter_notification_returns_400_and_missing_template( assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Template not found'}] - def test_post_notification_returns_403_and_well_formed_auth_error( client, sample_letter_template @@ -139,11 +141,16 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded( assert not persist_mock.called +@pytest.mark.parametrize('service_args', [ + {'service_permissions': [EMAIL_TYPE, SMS_TYPE]}, + {'restricted': True} +]) def test_post_letter_notification_returns_400_if_not_allowed_to_send_notification( client, - notify_db_session + notify_db_session, + service_args ): - service = create_service(service_permissions=[EMAIL_TYPE, SMS_TYPE]) + service = create_service(**service_args) template = create_template(service, template_type=LETTER_TYPE) data = { @@ -154,5 +161,5 @@ def test_post_letter_notification_returns_400_if_not_allowed_to_send_notificatio error_json = letter_request(client, data, service_id=service.id, _expected_status=400) assert error_json['status_code'] == 400 assert error_json['errors'] == [ - {'error': 'BadRequestError', 'message': 'Cannot send text letters'} + {'error': 'BadRequestError', 'message': 'Cannot send letters'} ] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 8a180adff..597cdf1b7 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -433,3 +433,17 @@ def test_post_notification_raises_bad_request_if_service_not_invited_to_schedule error_json = json.loads(response.get_data(as_text=True)) assert error_json['errors'] == [ {"error": "BadRequestError", "message": 'Cannot schedule notifications (this feature is invite-only)'}] + + +def test_post_notification_raises_bad_request_if_not_valid_notification_type(client, sample_service): + auth_header = create_authorization_header(service_id=sample_service.id) + response = client.post( + '/v2/notifications/foo', + data='{}', + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert response.status_code == 400 + error_json = json.loads(response.get_data(as_text=True)) + assert error_json['errors'] == [ + {'error': 'BadRequestError', 'message': 'Unknown notification type foo'} + ]