diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 4dab98e9f..7c8be70aa 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -45,6 +45,7 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not template_id=notification['template'], to=notification['to'], service_id=service_id, + job_id=notification.get('job', None), status='sent' ) save_notification(notification_db_object) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ba6573fed..40d0e45f1 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -18,7 +18,8 @@ from app.schemas import ( email_notification_schema, sms_template_notification_schema, notification_status_schema, - job_sms_template_notification_schema + job_sms_template_notification_schema, + job_email_template_notification_schema ) from app.celery.tasks import send_sms, send_email from sqlalchemy.orm.exc import NoResultFound @@ -29,6 +30,9 @@ from app.errors import register_errors register_errors(notifications) +SMS_NOTIFICATION = 'sms' +EMAIL_NOTIFICATION = 'email' + def create_notification_id(): return str(uuid.uuid4()) @@ -45,22 +49,36 @@ def get_notifications(notification_id): @notifications.route('/sms', methods=['POST']) def create_sms_notification(): - return base_create_sms_notification(expects_job=False) + return send_notification(notification_type=SMS_NOTIFICATION, expects_job=False) @notifications.route('/sms/service/', methods=['POST']) -def create_sms_for_service(service_id): - return base_create_sms_notification(service_id, expects_job=True) +def create_sms_for_job(service_id): + return send_notification(service_id=service_id, notification_type=SMS_NOTIFICATION, expects_job=True) -def base_create_sms_notification(service_id=None, expects_job=False): +@notifications.route('/email', methods=['POST']) +def create_email_notification(): + return send_notification(notification_type=EMAIL_NOTIFICATION, expects_job=False) + + +@notifications.route('/email/service/', methods=['POST']) +def create_email_notification_for_job(service_id): + return send_notification(service_id=service_id, notification_type=EMAIL_NOTIFICATION, expects_job=True) + + +def send_notification(notification_type, service_id=None, expects_job=False): + assert notification_type + if not service_id: service_id = api_user['client'] if expects_job: - schema = job_sms_template_notification_schema + schema = job_sms_template_notification_schema if notification_type is SMS_NOTIFICATION else \ + job_email_template_notification_schema else: - schema = sms_template_notification_schema + schema = sms_template_notification_schema if notification_type is SMS_NOTIFICATION else \ + email_notification_schema notification, errors = schema.load(request.get_json()) if errors: @@ -85,49 +103,32 @@ def base_create_sms_notification(service_id=None, expects_job=False): if not job: return jsonify(result="error", message={'job': ['Job {} not found'.format(notification['job'])]}), 400 - service = services_dao.dao_fetch_service_by_id(service_id) - - if service.restricted: - if notification['to'] not in [user.email_address for user in service.users]: - return jsonify(result="error", message={'to': ['Invalid phone number for restricted service']}), 400 - - notification_id = create_notification_id() - - send_sms.apply_async(( - api_user['client'], - notification_id, - encryption.encrypt(notification)), - queue='sms') - return jsonify({'notification_id': notification_id}), 201 - - -@notifications.route('/email', methods=['POST']) -def create_email_notification(): - notification, errors = email_notification_schema.load(request.get_json()) - if errors: - return jsonify(result="error", message=errors), 400 - - template = templates_dao.dao_get_template_by_id_and_service_id( - template_id=notification['template'], - service_id=api_user['client'] - ) - - if not template: - return jsonify(result="error", message={'template': ['Template not found']}), 400 - service = services_dao.dao_fetch_service_by_id(api_user['client']) if service.restricted: - if notification['to'] not in [user.email_address for user in service.users]: - return jsonify(result="error", message={'to': ['Email address not permitted for restricted service']}), 400 + if notification_type is SMS_NOTIFICATION: + if notification['to'] not in [user.mobile_number for user in service.users]: + return jsonify( + result="error", message={'to': ['Invalid phone number for restricted service']}), 400 + else: + if notification['to'] not in [user.email_address for user in service.users]: + return jsonify( + result="error", message={'to': ['Email address not permitted for restricted service']}), 400 notification_id = create_notification_id() - send_email.apply_async(( - api_user['client'], - notification_id, - template.subject, - "{}@{}".format(service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), - encryption.encrypt(notification)), - queue='email') + if notification_type is SMS_NOTIFICATION: + send_sms.apply_async(( + api_user['client'], + notification_id, + encryption.encrypt(notification)), + queue='sms') + else: + send_email.apply_async(( + api_user['client'], + notification_id, + template.subject, + "{}@{}".format(service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + encryption.encrypt(notification)), + queue='email') return jsonify({'notification_id': notification_id}), 201 diff --git a/app/schemas.py b/app/schemas.py index 942ca643f..de4c52f16 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -113,6 +113,11 @@ class JobSmsTemplateNotificationSchema(SmsNotificationSchema): job = fields.String(required=True) +class JobEmailTemplateNotificationSchema(EmailNotificationSchema): + template = fields.Int(required=True) + job = fields.String(required=True) + + class SmsAdminNotificationSchema(SmsNotificationSchema): content = fields.Str(required=True) @@ -145,6 +150,7 @@ sms_admin_notification_schema = SmsAdminNotificationSchema() sms_template_notification_schema = SmsTemplateNotificationSchema() job_sms_template_notification_schema = JobSmsTemplateNotificationSchema() email_notification_schema = EmailNotificationSchema() +job_email_template_notification_schema = JobEmailTemplateNotificationSchema() notification_status_schema = NotificationStatusSchema() notifications_status_schema = NotificationStatusSchema(many=True) notification_status_schema_load_json = NotificationStatusSchema(load_json=True) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 6057effd6..18cc4e9f1 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -572,7 +572,11 @@ def test_should_reject_email_notification_with_template_id_that_cant_be_found( app.celery.tasks.send_email.apply_async.assert_not_called() assert response.status_code == 400 assert data['result'] == 'error' - assert data['message']['template'][0] == 'Template not found' + test_string = 'Template {} not found for service {}'.format( + 1234, + sample_email_template.service.id + ) + assert test_string in data['message']['template'] def test_should_not_allow_email_template_from_another_service(notify_api, service_factory, sample_user, mocker): @@ -605,7 +609,8 @@ def test_should_not_allow_email_template_from_another_service(notify_api, servic app.celery.tasks.send_email.apply_async.assert_not_called() assert response.status_code == 400 - assert 'Template not found' in json_resp['message']['template'] + test_string = 'Template {} not found for service {}'.format(service_2_templates[0].id, service_1.id) + assert test_string in json_resp['message']['template'] def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api, sample_email_template, mocker):