Merge pull request #2657 from alphagov/validate-against-empty-messages

Validate against messages with no content

app/notifications/validators.py

@@ -121,6 +121,12 @@ def check_sms_content_char_count(content_count):
         raise BadRequestError(message=message)
 
 
+def check_notification_content_is_not_empty(template_with_content):
+    if template_with_content.is_message_empty():
+        message = 'Your message is empty.'
+        raise BadRequestError(message=message)
+
+
 def validate_template(template_id, personalisation, service, notification_type):
     try:
         template = templates_dao.dao_get_template_by_id_and_service_id(
@@ -134,9 +140,12 @@ def validate_template(template_id, personalisation, service, notification_type):
 
     check_template_is_for_notification_type(notification_type, template.template_type)
     check_template_is_active(template)
+
     template_with_content = create_content_for_notification(template, personalisation)
+    check_notification_content_is_not_empty(template_with_content)
     if template.template_type == SMS_TYPE:
         check_sms_content_char_count(template_with_content.content_count)
+
     return template, template_with_content
 
 

requirements-app.txt

@@ -29,6 +29,6 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@36.1.2#egg=notifications-utils==36.1.2
+git+https://github.com/alphagov/notifications-utils.git@36.2.0#egg=notifications-utils==36.2.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

requirements.txt

@@ -31,7 +31,7 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@36.1.2#egg=notifications-utils==36.1.2
+git+https://github.com/alphagov/notifications-utils.git@36.2.0#egg=notifications-utils==36.2.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3
 
@@ -40,12 +40,12 @@ alembic==1.3.1
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.16.283
+awscli==1.16.287
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.0
 boto3==1.9.221
-botocore==1.13.19
+botocore==1.13.23
 certifi==2019.9.11
 chardet==3.0.4
 Click==7.0

tests/app/notifications/test_validators.py

@@ -4,19 +4,23 @@ from flask import current_app
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
 
 import app
+from app.dao import templates_dao
 from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE
+from app.notifications.process_notifications import create_content_for_notification
 from app.notifications.validators import (
+    check_notification_content_is_not_empty,
     check_service_over_daily_message_limit,
     check_template_is_for_notification_type,
     check_template_is_active,
-    service_can_send_to_recipient,
     check_sms_content_char_count,
     check_service_over_api_rate_limit,
-    validate_and_format_recipient,
     check_service_email_reply_to_id,
     check_service_sms_sender_id,
     check_service_letter_contact_id,
     check_reply_to,
+    service_can_send_to_recipient,
+    validate_and_format_recipient,
+    validate_template
 )
 
 from app.v2.errors import (
@@ -286,6 +290,52 @@ def test_check_sms_content_char_count_fails(char_count, notify_api):
     assert e.value.fields == []
 
 
+def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service):
+    template_id = create_template(sample_service, content="Content is not empty").id
+    template = templates_dao.dao_get_template_by_id_and_service_id(
+        template_id=template_id,
+        service_id=sample_service.id
+    )
+    template_with_content = create_content_for_notification(template, {})
+    assert check_notification_content_is_not_empty(template_with_content) is None
+
+
+@pytest.mark.parametrize('template_content,notification_values', [
+    ("", {}),
+    ("((placeholder))", {"placeholder": ""})
+])
+def test_check_notification_content_is_not_empty_fails(
+    notify_api, mocker, sample_service, template_content, notification_values
+):
+    template_id = create_template(sample_service, content=template_content).id
+    template = templates_dao.dao_get_template_by_id_and_service_id(
+        template_id=template_id,
+        service_id=sample_service.id
+    )
+    template_with_content = create_content_for_notification(template, notification_values)
+    with pytest.raises(BadRequestError) as e:
+        check_notification_content_is_not_empty(template_with_content)
+    assert e.value.status_code == 400
+    assert e.value.message == 'Your message is empty.'
+    assert e.value.fields == []
+
+
+def test_validate_template(mocker, fake_uuid, sample_service):
+    template = create_template(sample_service, template_type="email")
+    mock_check_type = mocker.patch('app.notifications.validators.check_template_is_for_notification_type')
+    mock_check_if_active = mocker.patch('app.notifications.validators.check_template_is_active')
+    mock_create_conent = mocker.patch(
+        'app.notifications.validators.create_content_for_notification', return_value="content"
+    )
+    mock_check_not_empty = mocker.patch('app.notifications.validators.check_notification_content_is_not_empty')
+    validate_template(template.id, {}, sample_service, "email")
+
+    mock_check_type.assert_called_once_with("email", "email")
+    mock_check_if_active.assert_called_once_with(template)
+    mock_create_conent.assert_called_once_with(template, {})
+    mock_check_not_empty.assert_called_once_with("content")
+
+
 @pytest.mark.parametrize('key_type', ['team', 'live', 'test'])
 def test_that_when_exceed_rate_limit_request_fails(
         key_type,