diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 907983508..5c7f8fdea 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -39,7 +39,7 @@ from app.schemas import ( day_schema ) from app.service.utils import service_allowed_to_send_to -from app.utils import pagination_links, get_template_instance +from app.utils import pagination_links, get_template_instance, get_public_notify_type_text from notifications_utils.recipients import get_international_phone_info @@ -121,20 +121,14 @@ def send_notification(notification_type): template_object = create_template_object_for_notification(template, notification_form.get('personalisation', {})) _service_allowed_to_send_to(notification_form, authenticated_service) - if notification_type == SMS_TYPE: - if service_has_permission(SMS_TYPE, authenticated_service.permissions) is False: - raise InvalidRequest( - {'to': ["Cannot send text messages"]}, - status_code=400 - ) + if not service_has_permission(notification_type, authenticated_service.permissions): + raise InvalidRequest( + {'service': ["Cannot send {}".format(get_public_notify_type_text(notification_type, plural=True))]}, + status_code=400 + ) + if notification_type == SMS_TYPE: _service_can_send_internationally(authenticated_service, notification_form['to']) - elif notification_type == EMAIL_TYPE: - if service_has_permission(EMAIL_TYPE, authenticated_service.permissions) is False: - raise InvalidRequest( - {'to': ["Cannot send emails"]}, - status_code=400 - ) # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(notification_form['to'], notification_type) diff --git a/app/template/rest.py b/app/template/rest.py index 90d7e114a..9a30aab4b 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -45,10 +45,11 @@ def create_template(service_id): permissions = fetched_service.permissions new_template = template_schema.load(request.get_json()).data - if service_has_permission(new_template.template_type, permissions) is False: - raise InvalidRequest( - "Creating {} templates is not allowed".format( - get_public_notify_type_text(new_template.template_type)), 403) + if not service_has_permission(new_template.template_type, permissions): + message = "Creating {} templates is not allowed".format( + get_public_notify_type_text(new_template.template_type)) + errors = {'template_type': [message]} + raise InvalidRequest(errors, 403) new_template.service = fetched_service over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type) @@ -66,10 +67,12 @@ def create_template(service_id): def update_template(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) - if service_has_permission(fetched_template.template_type, fetched_template.service.permissions) is False: - raise InvalidRequest( - "Updating {} templates is not allowed".format( - get_public_notify_type_text(fetched_template.template_type)), 403) + if not service_has_permission(fetched_template.template_type, fetched_template.service.permissions): + message = "Updating {} templates is not allowed".format( + get_public_notify_type_text(fetched_template.template_type)) + errors = {'template_type': [message]} + + raise InvalidRequest(errors, 403) data = request.get_json() diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 8f2706f7b..b26e1da56 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -32,13 +32,13 @@ def post_notification(notification_type): else: form = validate(request.get_json(), post_sms_request) - if service_has_permission(notification_type, authenticated_service.permissions) is False: + if not service_has_permission(notification_type, authenticated_service.permissions): raise BadRequestError(message="Cannot send {}".format( get_public_notify_type_text(notification_type, plural=True))) scheduled_for = form.get("scheduled_for", None) if scheduled_for: - if service_has_permission(SCHEDULE_NOTIFICATIONS, authenticated_service.permissions) is False: + if not service_has_permission(SCHEDULE_NOTIFICATIONS, authenticated_service.permissions): raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)") check_rate_limiting(authenticated_service, api_user) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 034614a97..aa7ab2d86 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -1198,4 +1198,4 @@ def test_should_not_allow_notification_if_service_permission_not_set( error_json = json.loads(response.get_data(as_text=True)) assert error_json['result'] == 'error' - assert error_json['message']['to'][0] == expected_error + assert error_json['message']['service'][0] == expected_error diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 00e9b4895..2c0f8ea96 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -91,9 +91,9 @@ def test_should_raise_error_if_service_does_not_exist_on_create(client, sample_u @pytest.mark.parametrize('permissions, template_type, subject, expected_error', [ - ([EMAIL_TYPE], SMS_TYPE, None, 'Creating text message templates is not allowed'), - ([SMS_TYPE], EMAIL_TYPE, 'subject', 'Creating email templates is not allowed'), - ([SMS_TYPE], LETTER_TYPE, 'subject', 'Creating letter templates is not allowed'), + ([EMAIL_TYPE], SMS_TYPE, None, {'template_type': ['Creating text message templates is not allowed']}), + ([SMS_TYPE], EMAIL_TYPE, 'subject', {'template_type': ['Creating email templates is not allowed']}), + ([SMS_TYPE], LETTER_TYPE, 'subject', {'template_type': ['Creating letter templates is not allowed']}), ]) def test_should_raise_error_on_create_if_no_permission( client, sample_user, permissions, template_type, subject, expected_error): @@ -123,9 +123,9 @@ def test_should_raise_error_on_create_if_no_permission( @pytest.mark.parametrize('template_factory, expected_error', [ - (sample_template_without_sms_permission, 'Updating text message templates is not allowed'), - (sample_template_without_email_permission, 'Updating email templates is not allowed'), - (sample_template_without_letter_permission, 'Updating letter templates is not allowed') + (sample_template_without_sms_permission, {'template_type': ['Updating text message templates is not allowed']}), + (sample_template_without_email_permission, {'template_type': ['Updating email templates is not allowed']}), + (sample_template_without_letter_permission, {'template_type': ['Updating letter templates is not allowed']}) ]) def test_should_be_error_on_update_if_no_permission( client, sample_user, template_factory, expected_error, notify_db, notify_db_session):