mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-14 09:12:06 -05:00
Revert "Tailor message-too-long error message depending on the notification type"
This commit is contained in:
@@ -144,19 +144,10 @@ def check_if_service_can_send_to_number(service, number):
|
|||||||
return international_phone_info
|
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():
|
if template_with_content.is_message_too_long():
|
||||||
message = "Your message is too long."
|
message = f"Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. " \
|
||||||
if template_with_content.template_type == SMS_TYPE:
|
f"Your message is {template_with_content.content_count_without_prefix} characters"
|
||||||
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)
|
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_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
|
return template, template_with_content
|
||||||
|
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ notifications-python-client==5.7.0
|
|||||||
# PaaS
|
# PaaS
|
||||||
awscli-cwlogs>=1.4,<1.5
|
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
|
# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
|
||||||
prometheus-client==0.8.0
|
prometheus-client==0.8.0
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ notifications-python-client==5.7.0
|
|||||||
# PaaS
|
# PaaS
|
||||||
awscli-cwlogs>=1.4,<1.5
|
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
|
# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
|
||||||
prometheus-client==0.8.0
|
prometheus-client==0.8.0
|
||||||
@@ -42,14 +42,14 @@ alembic==1.4.3
|
|||||||
amqp==1.4.9
|
amqp==1.4.9
|
||||||
anyjson==0.3.3
|
anyjson==0.3.3
|
||||||
attrs==20.2.0
|
attrs==20.2.0
|
||||||
awscli==1.18.167
|
awscli==1.18.164
|
||||||
bcrypt==3.2.0
|
bcrypt==3.2.0
|
||||||
billiard==3.3.0.23
|
billiard==3.3.0.23
|
||||||
bleach==3.1.4
|
bleach==3.1.4
|
||||||
blinker==1.4
|
blinker==1.4
|
||||||
boto==2.49.0
|
boto==2.49.0
|
||||||
boto3==1.10.38
|
boto3==1.10.38
|
||||||
botocore==1.19.7
|
botocore==1.19.4
|
||||||
certifi==2020.6.20
|
certifi==2020.6.20
|
||||||
chardet==3.0.4
|
chardet==3.0.4
|
||||||
click==7.1.2
|
click==7.1.2
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ from app.models import (
|
|||||||
)
|
)
|
||||||
from app.notifications.process_notifications import create_content_for_notification
|
from app.notifications.process_notifications import create_content_for_notification
|
||||||
from app.notifications.validators import (
|
from app.notifications.validators import (
|
||||||
check_message_is_not_too_long,
|
check_content_char_count,
|
||||||
check_if_service_can_send_files_by_email,
|
check_if_service_can_send_files_by_email,
|
||||||
check_notification_content_is_not_empty,
|
check_notification_content_is_not_empty,
|
||||||
check_service_over_daily_message_limit,
|
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('char_count', [612, 0, 494, 200, 918])
|
||||||
@pytest.mark.parametrize('show_prefix', [True, False])
|
@pytest.mark.parametrize('show_prefix', [True, False])
|
||||||
@pytest.mark.parametrize('template_type', ['sms', 'email', 'letter'])
|
@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)
|
service = create_service(prefix_sms=show_prefix)
|
||||||
t = create_template(service=service, content='a' * char_count, template_type=template_type)
|
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 = 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={})
|
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('char_count', [919, 6000])
|
||||||
@pytest.mark.parametrize('show_prefix', [True, False])
|
@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:
|
with pytest.raises(BadRequestError) as e:
|
||||||
service = create_service(prefix_sms=show_prefix)
|
service = create_service(prefix_sms=show_prefix)
|
||||||
t = create_template(service=service, content='a' * char_count, template_type='sms')
|
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 = 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={})
|
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.status_code == 400
|
||||||
assert e.value.message == f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \
|
assert e.value.message == f'Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. ' \
|
||||||
f'Your message is {char_count} characters long.'
|
f'Your message is {char_count} characters'
|
||||||
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 == []
|
assert e.value.fields == []
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('template_type', ['email', 'letter'])
|
@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)
|
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,
|
template = templates_dao.dao_get_template_by_id_and_service_id(template_id=t.id,
|
||||||
service_id=t.service_id)
|
service_id=t.service_id)
|
||||||
template_with_content = get_template_instance(template=template.__dict__, values={})
|
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):
|
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"
|
'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_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")
|
template, template_with_content = validate_template(template.id, {}, sample_service, "email")
|
||||||
|
|
||||||
mock_check_type.assert_called_once_with("email", "email")
|
mock_check_type.assert_called_once_with("email", "email")
|
||||||
|
|||||||
@@ -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)
|
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'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):
|
def test_send_one_off_notification_fails_if_created_by_other_service(sample_template):
|
||||||
|
|||||||
Reference in New Issue
Block a user