Further refactoring

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)

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()
 

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)

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

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):