From c4075f1fc039a8bd26792d769d71c6bf124aa81e Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 3 Nov 2020 10:55:15 +0000 Subject: [PATCH] Revert "Tailor message-too-long error message depending on the notification type" --- app/notifications/validators.py | 17 +++------- requirements-app.txt | 2 +- requirements.txt | 6 ++-- tests/app/notifications/test_validators.py | 33 +++++-------------- .../service/test_send_one_off_notification.py | 2 +- 5 files changed, 18 insertions(+), 42 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 745155f39..91747d14d 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -144,19 +144,10 @@ def check_if_service_can_send_to_number(service, number): return international_phone_info -def check_message_is_not_too_long(template_with_content): +def check_content_char_count(template_with_content): if template_with_content.is_message_too_long(): - 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." - ) + 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" raise BadRequestError(message=message) @@ -182,7 +173,7 @@ def validate_template(template_id, personalisation, service, notification_type): check_notification_content_is_not_empty(template_with_content) - check_message_is_not_too_long(template_with_content) + check_content_char_count(template_with_content) return template, template_with_content diff --git a/requirements-app.txt b/requirements-app.txt index e9d24d4ad..ded61f2c9 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.1.0#egg=notifications-utils==43.1.0 +git+https://github.com/alphagov/notifications-utils.git@43.0.0#egg=notifications-utils==43.0.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 244235f30..cae9c641a 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.1.0#egg=notifications-utils==43.1.0 +git+https://github.com/alphagov/notifications-utils.git@43.0.0#egg=notifications-utils==43.0.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.167 +awscli==1.18.164 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.7 +botocore==1.19.4 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 93727b48f..de6ce99ce 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_message_is_not_too_long, + check_content_char_count, check_if_service_can_send_files_by_email, check_notification_content_is_not_empty, check_service_over_daily_message_limit, @@ -319,51 +319,36 @@ 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_message_is_not_too_long_passes(notify_db_session, show_prefix, char_count, template_type): +def test_check_content_char_count_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_message_is_not_too_long(template_with_content) is None + assert check_content_char_count(template_with_content) is None @pytest.mark.parametrize('char_count', [919, 6000]) @pytest.mark.parametrize('show_prefix', [True, False]) -def test_check_message_is_not_too_long_for_too_long_sms_messages(notify_db_session, show_prefix, char_count): +def test_check_content_char_count_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_message_is_not_too_long(template_with_content) + check_content_char_count(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 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." - ) + f'Your message is {char_count} characters' assert e.value.fields == [] @pytest.mark.parametrize('template_type', ['email', 'letter']) -def test_check_message_is_not_too_long_passes_for_long_email_or_letter(sample_service, template_type): +def test_check_content_char_count_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_message_is_not_too_long(template_with_content) is None + assert check_content_char_count(template_with_content) is None def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service): @@ -409,7 +394,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_message_is_not_too_long') + mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_content_char_count') 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 cfc025157..2f6e1ee60 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 long.' + f'Your message is {1029} characters' def test_send_one_off_notification_fails_if_created_by_other_service(sample_template):