Rename check_character_count method to check_is_message_to_long.

Add different error message for email and text if content is too long.
Use utils version with is_message_too_long method implemented for email templates.

app/notifications/validators.py

@@ -143,10 +143,19 @@ def check_if_service_can_send_to_number(service, number):
         return international_phone_info
 
 
-def check_content_char_count(template_with_content):
+def check_is_message_too_long(template_with_content):
     if template_with_content.is_message_too_long():
-        message = f"Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. " \
+        message = "Your message is too long. "
-            f"Your message is {template_with_content.content_count_without_prefix} characters"
+        if template_with_content.template_type == SMS_TYPE:
+            message += (
+                f"Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. "
+                f"Your message is {template_with_content.content_count_without_prefix} characters long."
+            )
+        elif template_with_content.template_type == EMAIL_TYPE:
+            message += (
+                f"Emails cannot be longer than 1000000 bytes. "
+                f"Your message is {template_with_content.content_size_in_bytes} bytes."
+            )
         raise BadRequestError(message=message)
 
 
@@ -175,7 +184,7 @@ def validate_template(template_id, personalisation, service, notification_type,
     # which means the length of the message can be exceeded because it's including the file.
     # The document download feature is only available through the api.
     if check_char_count:
-        check_content_char_count(template_with_content)
+        check_is_message_too_long(template_with_content)
 
     return template, template_with_content
 

app/v2/notifications/post_notifications.py

@@ -52,7 +52,7 @@ from app.notifications.validators import (
     validate_address,
     validate_and_format_recipient,
     validate_template,
-    check_content_char_count)
+    check_is_message_too_long)
 from app.schema_validation import validate
 from app.v2.errors import BadRequestError
 from app.v2.notifications.create_response import (
@@ -191,7 +191,7 @@ def process_sms_or_email_notification(
         template_with_content.values = personalisation
 
     # validate content length after url is replaced in personalisation.
-    check_content_char_count(template_with_content)
+    check_is_message_too_long(template_with_content)
 
     resp = create_response_for_post_notification(
         notification_id=notification_id,

requirements-app.txt

@@ -29,7 +29,7 @@ notifications-python-client==5.7.0
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@43.0.0#egg=notifications-utils==43.0.0
+git+https://github.com/alphagov/notifications-utils.git@43.3.0#egg=notifications-utils==43.3.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.8.0

requirements.txt

@@ -31,7 +31,7 @@ notifications-python-client==5.7.0
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@43.0.0#egg=notifications-utils==43.0.0
+git+https://github.com/alphagov/notifications-utils.git@43.3.0#egg=notifications-utils==43.3.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.8.0

tests/app/notifications/test_validators.py

@@ -13,7 +13,7 @@ from app.models import (
 )
 from app.notifications.process_notifications import create_content_for_notification
 from app.notifications.validators import (
-    check_content_char_count,
+    check_is_message_too_long,
     check_if_service_can_send_files_by_email,
     check_notification_content_is_not_empty,
     check_service_over_daily_message_limit,
@@ -319,36 +319,46 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(s
 @pytest.mark.parametrize('char_count', [612, 0, 494, 200, 918])
 @pytest.mark.parametrize('show_prefix', [True, False])
 @pytest.mark.parametrize('template_type', ['sms', 'email', 'letter'])
-def test_check_content_char_count_passes(notify_db_session, show_prefix, char_count, template_type):
+def test_check_is_message_too_long_passes(notify_db_session, show_prefix, char_count, template_type):
     service = create_service(prefix_sms=show_prefix)
     t = create_template(service=service, content='a' * char_count, template_type=template_type)
     template = templates_dao.dao_get_template_by_id_and_service_id(template_id=t.id, service_id=service.id)
     template_with_content = get_template_instance(template=template.__dict__, values={})
-    assert check_content_char_count(template_with_content) is None
+    assert check_is_message_too_long(template_with_content) is None
 
 
 @pytest.mark.parametrize('char_count', [919, 6000])
 @pytest.mark.parametrize('show_prefix', [True, False])
-def test_check_content_char_count_fails(notify_db_session, show_prefix, char_count):
+def test_check_is_message_too_long_fails(notify_db_session, show_prefix, char_count):
     with pytest.raises(BadRequestError) as e:
         service = create_service(prefix_sms=show_prefix)
         t = create_template(service=service, content='a' * char_count, template_type='sms')
         template = templates_dao.dao_get_template_by_id_and_service_id(template_id=t.id, service_id=service.id)
         template_with_content = get_template_instance(template=template.__dict__, values={})
-        check_content_char_count(template_with_content)
+        check_is_message_too_long(template_with_content)
     assert e.value.status_code == 400
-    assert e.value.message == f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \
+    expected_message = f'Your message is too long. '\
-                              f'Your message is {char_count} characters'
+                       f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. '\
+                       f'Your message is {char_count} characters long.'
+    assert e.value.message == expected_message
     assert e.value.fields == []
 
 
-@pytest.mark.parametrize('template_type', ['email', 'letter'])
+def test_check_is_message_too_long_passes_for_long_email(sample_service):
-def test_check_content_char_count_passes_for_long_email_or_letter(sample_service, template_type):
+    email_character_count = 1000003
-    t = create_template(service=sample_service, content='a' * 1000, template_type=template_type)
+    t = create_template(service=sample_service, content='a' * email_character_count, template_type='email')
     template = templates_dao.dao_get_template_by_id_and_service_id(template_id=t.id,
                                                                    service_id=t.service_id)
     template_with_content = get_template_instance(template=template.__dict__, values={})
-    assert check_content_char_count(template_with_content) is None
+    template_with_content.values
+    with pytest.raises(BadRequestError) as e:
+        check_is_message_too_long(template_with_content)
+    assert e.value.status_code == 400
+    expected_message = f'Your message is too long. ' \
+        f'Emails cannot be longer than 1000000 bytes. ' \
+        f'Your message is 1000084 bytes.'
+    assert e.value.message == expected_message
+    assert e.value.fields == []
 
 
 def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service):
@@ -386,7 +396,8 @@ def test_validate_template(sample_service):
     validate_template(template.id, {}, sample_service, "email")
 
 
-def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_service):
+@pytest.mark.parametrize("check_char_count", [True, False])
+def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_service, check_char_count):
     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')
@@ -394,14 +405,19 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic
         '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')
-    mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_content_char_count')
+    mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_is_message_too_long')
-    template, template_with_content = validate_template(template.id, {}, sample_service, "email")
+    template, template_with_content = validate_template(template.id, {}, sample_service, "email",
+                                                        check_char_count=check_char_count
+                                                        )
 
     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")
-    mock_check_message_is_too_long.assert_called_once_with("content")
+    if check_char_count:
+        mock_check_message_is_too_long.assert_called_once_with("content")
+    else:
+        assert not mock_check_message_is_too_long.called
 
 
 def test_validate_template_calls_all_validators_exception_message_too_long(mocker, fake_uuid, sample_service):
@@ -412,7 +428,7 @@ def test_validate_template_calls_all_validators_exception_message_too_long(mocke
         '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')
-    mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_content_char_count')
+    mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_is_message_too_long')
     template, template_with_content = validate_template(template.id, {}, sample_service, "email",
                                                         check_char_count=False)
 

tests/app/service/test_send_one_off_notification.py

@@ -317,8 +317,9 @@ def test_send_one_off_notification_raises_if_message_too_long(persist_mock, noti
     with pytest.raises(BadRequestError) as e:
         send_one_off_notification(service.id, post_data)
 
-    assert e.value.message == f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \
+    assert e.value.message == f'Your message is too long. ' \
-                              f'Your message is {1029} characters'
+        f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \
+        f'Your message is {1029} characters long.'
 
 
 def test_send_one_off_notification_fails_if_created_by_other_service(sample_template):