diff --git a/app/dao/service_email_reply_to_dao.py b/app/dao/service_email_reply_to_dao.py index 51979caf4..7ecc2a43b 100644 --- a/app/dao/service_email_reply_to_dao.py +++ b/app/dao/service_email_reply_to_dao.py @@ -10,7 +10,8 @@ def dao_get_reply_to_by_service_id(service_id): reply_to = db.session.query( ServiceEmailReplyTo ).filter( - ServiceEmailReplyTo.service_id == service_id + ServiceEmailReplyTo.service_id == service_id, + ServiceEmailReplyTo.archived == False # noqa ).order_by(desc(ServiceEmailReplyTo.is_default), desc(ServiceEmailReplyTo.created_at)).all() return reply_to @@ -20,7 +21,8 @@ def dao_get_reply_to_by_id(service_id, reply_to_id): ServiceEmailReplyTo ).filter( ServiceEmailReplyTo.service_id == service_id, - ServiceEmailReplyTo.id == reply_to_id + ServiceEmailReplyTo.id == reply_to_id, + ServiceEmailReplyTo.archived == False # noqa ).order_by(ServiceEmailReplyTo.created_at).one() return reply_to diff --git a/app/service/send_notification.py b/app/service/send_notification.py index dbb4bd4fc..1aba31259 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -1,3 +1,5 @@ +from sqlalchemy.orm.exc import NoResultFound + from app.config import QueueNames from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id @@ -89,7 +91,11 @@ def get_reply_to_text(notification_type, sender_id, service, template): reply_to = None if sender_id: if notification_type == EMAIL_TYPE: - reply_to = dao_get_reply_to_by_id(service.id, sender_id).email_address + try: + reply_to = dao_get_reply_to_by_id(service.id, sender_id).email_address + except NoResultFound: + message = 'Reply to email address not found' + raise BadRequestError(message=message) elif notification_type == SMS_TYPE: reply_to = dao_get_service_sms_senders_by_id(service.id, sender_id).get_reply_to_text() else: diff --git a/tests/app/dao/test_service_email_reply_to_dao.py b/tests/app/dao/test_service_email_reply_to_dao.py index 73237a9ab..df6aed6f0 100644 --- a/tests/app/dao/test_service_email_reply_to_dao.py +++ b/tests/app/dao/test_service_email_reply_to_dao.py @@ -25,6 +25,23 @@ def test_dao_get_reply_to_by_service_id(notify_db_session): assert second_reply_to == results[2] +def test_dao_get_reply_to_by_service_id_does_not_return_archived_reply_tos(notify_db_session): + service = create_service() + create_reply_to_email(service=service, email_address='something@email.com') + create_reply_to_email(service=service, email_address='another@email.com', is_default=False) + archived_reply_to = create_reply_to_email( + service=service, + email_address='second@email.com', + is_default=False, + archived=True + ) + + results = dao_get_reply_to_by_service_id(service_id=service.id) + + assert len(results) == 2 + assert archived_reply_to not in results + + def test_add_reply_to_email_address_for_service_creates_first_email_for_service(notify_db_session): service = create_service() add_reply_to_email_address_for_service(service_id=service.id, @@ -163,6 +180,18 @@ def test_dao_get_reply_to_by_id_raises_sqlalchemy_error_when_reply_to_does_not_e dao_get_reply_to_by_id(service_id=sample_service.id, reply_to_id=uuid.uuid4()) +def test_dao_get_reply_to_by_id_raises_sqlalchemy_error_when_reply_to_is_archived(sample_service): + create_reply_to_email(service=sample_service, email_address='email@address.com') + archived_reply_to = create_reply_to_email( + service=sample_service, + email_address='email_two@address.com', + is_default=False, + archived=True) + + with pytest.raises(SQLAlchemyError): + dao_get_reply_to_by_id(service_id=sample_service.id, reply_to_id=archived_reply_to.id) + + def test_dao_get_reply_to_by_id_raises_sqlalchemy_error_when_service_does_not_exist(sample_service): reply_to = create_reply_to_email(service=sample_service, email_address='email@address.com') with pytest.raises(SQLAlchemyError): diff --git a/tests/app/db.py b/tests/app/db.py index 94bd40e26..622083a10 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -398,14 +398,16 @@ def create_monthly_billing_entry( def create_reply_to_email( - service, - email_address, - is_default=True + service, + email_address, + is_default=True, + archived=False ): data = { 'service': service, 'email_address': email_address, 'is_default': is_default, + 'archived': archived, } reply_to = ServiceEmailReplyTo(**data) diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 5a88e107d..b03eed941 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -319,8 +319,9 @@ def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot 'created_by': str(sample_email_template.service.created_by_id) } - with pytest.raises(expected_exception=SQLAlchemyError): + with pytest.raises(expected_exception=BadRequestError) as e: send_one_off_notification(service_id=sample_email_template.service.id, post_data=data) + assert e.value.message == 'Reply to email address not found' def test_send_one_off_notification_should_throw_exception_if_sms_sender_id_doesnot_exist( diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 737ab7dcd..eeb629d91 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -700,6 +700,30 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa assert 'BadRequestError' in resp_json['errors'][0]['error'] +def test_post_email_notification_with_archived_reply_to_id_returns_400(client, sample_email_template, mocker): + archived_reply_to = create_reply_to_email( + sample_email_template.service, + 'reply_to@test.com', + is_default=False, + archived=True) + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = { + "email_address": 'test@test.com', + "template_id": sample_email_template.id, + 'email_reply_to_id': archived_reply_to.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 == 400 + resp_json = json.loads(response.get_data(as_text=True)) + assert 'email_reply_to_id {} does not exist in database for service id {}'. \ + format(archived_reply_to.id, sample_email_template.service_id) in resp_json['errors'][0]['message'] + assert 'BadRequestError' in resp_json['errors'][0]['error'] + + def test_post_notification_with_document_upload(client, notify_db, notify_db_session, mocker): service = sample_service(notify_db, notify_db_session, permissions=[EMAIL_TYPE, UPLOAD_DOCUMENT]) template = create_sample_template(