From 2fbe492d5c77cad307ee7eae90103e5140807d55 Mon Sep 17 00:00:00 2001 From: Richard Chapman <31849076+richardc0@users.noreply.github.com> Date: Wed, 4 Oct 2017 14:34:45 +0100 Subject: [PATCH] [2/10] Allow API calls to specify the reply address option (#1291) * Added service_email_reply_to_id to the POST /v2/notifications/email and a test to test the validator * Caught NoResultFound exception in check_service_email_reply_to_id as it was not being caught when there there was no valid service_id or reply_to_id. Fixed failing tests which were not passing due to the NoResultFound exception and added further tests to check for the good path through the code and an test to check for an invalid service_id * Added service_email_reply_to_id to the POST /v2/notifications/email and a test to test the validator * Caught NoResultFound exception in check_service_email_reply_to_id as it was not being caught when there there was no valid service_id or reply_to_id. Fixed failing tests which were not passing due to the NoResultFound exception and added further tests to check for the good path through the code and an test to check for an invalid service_id * Fixed code style in validators.py to confirm with rules Update the name of email_reply_to_id to conform better with other attributes in the schema and the resultant code in post_notifications.py Fixed code style in test_validators.py to confirm with rules Added tests to test_post_notifications.py to test the email_reply_to_id being present and being incorrect, it being optional is being tested by other tests. * Added service_email_reply_to_id to the POST /v2/notifications/email and a test to test the validator * Added service_email_reply_to_id to the POST /v2/notifications/email and a test to test the validator * Caught NoResultFound exception in check_service_email_reply_to_id as it was not being caught when there there was no valid service_id or reply_to_id. Fixed failing tests which were not passing due to the NoResultFound exception and added further tests to check for the good path through the code and an test to check for an invalid service_id * Caught NoResultFound exception in check_service_email_reply_to_id as it was not being caught when there there was no valid service_id or reply_to_id. Fixed failing tests which were not passing due to the NoResultFound exception and added further tests to check for the good path through the code and an test to check for an invalid service_id * Fixed code style in validators.py to confirm with rules Update the name of email_reply_to_id to conform better with other attributes in the schema and the resultant code in post_notifications.py Fixed code style in test_validators.py to confirm with rules Added tests to test_post_notifications.py to test the email_reply_to_id being present and being incorrect, it being optional is being tested by other tests. * Minor update after manual merge to fix check style rule break in test_validators.py where a single space was introduced. * Updates after code review. Moved the template from the exception message as it was not required and updated the error message to match the field name in the sschema for better debugging and error identification. * Fixed test after update of exception message --- app/notifications/validators.py | 10 ++++ app/v2/notifications/notification_schemas.py | 3 +- app/v2/notifications/post_notifications.py | 7 ++- tests/app/notifications/test_validators.py | 27 +++++++++- .../notifications/test_post_notifications.py | 49 ++++++++++++++++++- 5 files changed, 92 insertions(+), 4 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 83b620506..a76e70089 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -17,6 +17,7 @@ from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store from app.notifications.process_notifications import create_content_for_notification from app.utils import get_public_notify_type_text +from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id def check_service_over_api_rate_limit(service, api_key): @@ -132,3 +133,12 @@ def validate_template(template_id, personalisation, service, notification_type): if template.template_type == SMS_TYPE: check_sms_content_char_count(template_with_content.content_count) return template, template_with_content + + +def check_service_email_reply_to_id(service_id, reply_to_id): + if not (reply_to_id is None): + try: + reply_to = dao_get_reply_to_by_id(service_id, reply_to_id) + except NoResultFound: + message = 'email_reply_to_id does not exist in database' + raise BadRequestError(message=message) diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 9ff028d1f..2eeddc532 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -157,7 +157,8 @@ post_email_request = { "email_address": {"type": "string", "format": "email_address"}, "template_id": uuid, "personalisation": personalisation, - "scheduled_for": {"type": ["string", "null"], "format": "datetime"} + "scheduled_for": {"type": ["string", "null"], "format": "datetime"}, + "email_reply_to_id": uuid }, "required": ["email_address", "template_id"] } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index b3cf1c0f8..04c63a3e7 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -28,7 +28,8 @@ from app.notifications.validators import ( check_rate_limiting, check_service_can_schedule_notification, check_service_has_permission, - validate_template + validate_template, + check_service_email_reply_to_id ) from app.schema_validation import validate from app.v2.errors import BadRequestError @@ -59,10 +60,14 @@ def post_notification(notification_type): check_service_has_permission(notification_type, authenticated_service.permissions) scheduled_for = form.get("scheduled_for", None) + service_email_reply_to_id = form.get("email_reply_to_id", None) + check_service_can_schedule_notification(authenticated_service.permissions, scheduled_for) check_rate_limiting(authenticated_service, api_user) + check_service_email_reply_to_id(str(authenticated_service.id), service_email_reply_to_id) + template, template_with_content = validate_template( form['template_id'], form.get('personalisation', {}), diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 94e465895..66779b70e 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -10,7 +10,8 @@ from app.notifications.validators import ( service_can_send_to_recipient, check_sms_content_char_count, check_service_over_api_rate_limit, - validate_and_format_recipient + validate_and_format_recipient, + check_service_email_reply_to_id ) from app.v2.errors import ( @@ -22,6 +23,7 @@ from tests.app.conftest import ( sample_service as create_service, sample_service_whitelist, sample_api_key) +from tests.app.db import create_reply_to_email @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) @@ -322,3 +324,26 @@ def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_s 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' + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_none(): + assert check_service_email_reply_to_id(None, None) is None + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(sample_service, fake_uuid): + with pytest.raises(BadRequestError) as e: + check_service_email_reply_to_id(sample_service.id, fake_uuid) + assert e.value.status_code == 400 + assert e.value.message == 'email_reply_to_id does not exist in database' + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_found(sample_service): + reply_to_email = create_reply_to_email(sample_service, 'test@test.com') + assert check_service_email_reply_to_id(sample_service.id, reply_to_email.id) is None + + +def test_check_service_email_reply_to_id_where_service_id_is_not_found(sample_service, fake_uuid): + with pytest.raises(BadRequestError) as e: + check_service_email_reply_to_id(fake_uuid, fake_uuid) + assert e.value.status_code == 400 + assert e.value.message == 'email_reply_to_id does not exist in database' diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 4e72389c3..d4fdf94ec 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -19,7 +19,7 @@ from tests.app.conftest import ( sample_template_without_sms_permission, sample_template_without_email_permission ) -from tests.app.db import create_inbound_number, create_service, create_template +from tests.app.db import create_inbound_number, create_service, create_template, create_reply_to_email @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -477,3 +477,50 @@ def test_post_notification_raises_bad_request_if_not_valid_notification_type(cli assert response.status_code == 404 error_json = json.loads(response.get_data(as_text=True)) assert 'The requested URL was not found on the server.' in error_json['message'] + + +@pytest.mark.parametrize("reference", [None, "reference_from_client"]) +def test_post_sms_notification_with_invalid_reply_to_email_id( + client, + sample_template_with_placeholders, + reference, + fake_uuid): + data = { + 'phone_number': '+447700900855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'}, + 'email_reply_to_id': fake_uuid + } + if reference: + data.update({"reference": reference}) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 400 + resp_json = json.loads(response.get_data(as_text=True)) + assert 'reply_to_id does not exist in database' in resp_json['errors'][0]['message'] + assert 'BadRequestError' in resp_json['errors'][0]['error'] + + +def test_post_email_notification_with_valid_reply_to_id_returns_201(client, sample_email_template, mocker): + reply_to_email = create_reply_to_email(sample_email_template.service, 'test@test.com') + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = { + "email_address": sample_email_template.service.users[0].email_address, + "template_id": sample_email_template.id, + 'email_reply_to_id': reply_to_email.id + } + auth_header = create_authorization_header(service_id=sample_email_template.service_id) + response = client.post( + path="v2/notifications/email", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_email_response) == resp_json + notification = Notification.query.first() + assert resp_json['id'] == str(notification.id) + assert mocked.called