diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 91747d14d..745155f39 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -144,10 +144,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_message_is_not_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. " \ - f"Your message is {template_with_content.content_count_without_prefix} characters" + message = "Your message is too long." + 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) @@ -173,7 +182,7 @@ def validate_template(template_id, personalisation, service, notification_type): check_notification_content_is_not_empty(template_with_content) - check_content_char_count(template_with_content) + check_message_is_not_too_long(template_with_content) return template, template_with_content diff --git a/requirements-app.txt b/requirements-app.txt index ded61f2c9..e9d24d4ad 100644 --- a/requirements-app.txt +++ b/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.1.0#egg=notifications-utils==43.1.0 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains prometheus-client==0.8.0 diff --git a/requirements.txt b/requirements.txt index cae9c641a..244235f30 100644 --- a/requirements.txt +++ b/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.1.0#egg=notifications-utils==43.1.0 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains prometheus-client==0.8.0 @@ -42,14 +42,14 @@ alembic==1.4.3 amqp==1.4.9 anyjson==0.3.3 attrs==20.2.0 -awscli==1.18.164 +awscli==1.18.167 bcrypt==3.2.0 billiard==3.3.0.23 bleach==3.1.4 blinker==1.4 boto==2.49.0 boto3==1.10.38 -botocore==1.19.4 +botocore==1.19.7 certifi==2020.6.20 chardet==3.0.4 click==7.1.2 diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index de6ce99ce..93727b48f 100644 --- a/tests/app/notifications/test_validators.py +++ b/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_message_is_not_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,51 @@ 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_message_is_not_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_message_is_not_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_message_is_not_too_long_for_too_long_sms_messages(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_message_is_not_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. ' \ - f'Your message is {char_count} characters' + f'Your message is {char_count} characters long.' + assert e.value.fields == [] + + +def test_check_message_is_not_too_long_for_too_big_email_messages(notify_db_session): + with pytest.raises(BadRequestError) as e: + service = create_service() + t = create_template(service=service, content='a' * 1000000, template_type='email') + 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_message_is_not_too_long(template_with_content) + assert e.value.status_code == 400 + assert e.value.message == ( + f"Emails cannot be longer than 1000000 bytes. " + f"Your message is 1000081 bytes." + ) assert e.value.fields == [] @pytest.mark.parametrize('template_type', ['email', 'letter']) -def test_check_content_char_count_passes_for_long_email_or_letter(sample_service, template_type): +def test_check_message_is_not_too_long_passes_for_long_email_or_letter(sample_service, template_type): t = create_template(service=sample_service, content='a' * 1000, template_type=template_type) 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 + assert check_message_is_not_too_long(template_with_content) is None def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service): @@ -394,7 +409,7 @@ 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_message_is_not_too_long') template, template_with_content = validate_template(template.id, {}, sample_service, "email") mock_check_type.assert_called_once_with("email", "email") diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 2f6e1ee60..cfc025157 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -318,7 +318,7 @@ def test_send_one_off_notification_raises_if_message_too_long(persist_mock, noti send_one_off_notification(service.id, post_data) assert e.value.message == f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \ - f'Your message is {1029} characters' + f'Your message is {1029} characters long.' def test_send_one_off_notification_fails_if_created_by_other_service(sample_template):