diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 91747d14d..f51921ab7 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -27,7 +27,6 @@ from app.serialised_models import SerialisedTemplate from gds_metrics.metrics import Histogram - REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram( 'redis_exceeded_rate_limit_duration_seconds', 'Time taken to check rate limit', @@ -106,7 +105,7 @@ def check_if_service_can_send_files_by_email(service_contact_link, service_id): if not service_contact_link: raise BadRequestError( message=f"Send files by email has not been set up - add contact details for your service at " - f"{current_app.config['ADMIN_BASE_URL']}/services/{service_id}/service-settings/send-files-by-email" + f"{current_app.config['ADMIN_BASE_URL']}/services/{service_id}/service-settings/send-files-by-email" ) @@ -147,7 +146,7 @@ def check_if_service_can_send_to_number(service, number): def check_content_char_count(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" + f"Your message is {template_with_content.content_count_without_prefix} characters" raise BadRequestError(message=message) @@ -157,8 +156,7 @@ def check_notification_content_is_not_empty(template_with_content): raise BadRequestError(message=message) -def validate_template(template_id, personalisation, service, notification_type): - +def validate_template(template_id, personalisation, service, notification_type, check_char_count=True): try: template = SerialisedTemplate.from_id_and_service_id(template_id, service.id) except NoResultFound: @@ -173,7 +171,11 @@ 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) + # validating the template in post_notifications happens before the file is uploaded for doc download, + # 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) return template, template_with_content @@ -192,7 +194,7 @@ def check_service_email_reply_to_id(service_id, reply_to_id, notification_type): try: return dao_get_reply_to_by_id(service_id, reply_to_id).email_address except NoResultFound: - message = 'email_reply_to_id {} does not exist in database for service id {}'\ + message = 'email_reply_to_id {} does not exist in database for service id {}' \ .format(reply_to_id, service_id) raise BadRequestError(message=message) @@ -202,7 +204,7 @@ def check_service_sms_sender_id(service_id, sms_sender_id, notification_type): try: return dao_get_service_sms_senders_by_id(service_id, sms_sender_id).sms_sender except NoResultFound: - message = 'sms_sender_id {} does not exist in database for service id {}'\ + message = 'sms_sender_id {} does not exist in database for service id {}' \ .format(sms_sender_id, service_id) raise BadRequestError(message=message) @@ -212,7 +214,7 @@ def check_service_letter_contact_id(service_id, letter_contact_id, notification_ try: return dao_get_letter_contact_by_id(service_id, letter_contact_id).contact_block except NoResultFound: - message = 'letter_contact_id {} does not exist in database for service id {}'\ + message = 'letter_contact_id {} does not exist in database for service id {}' \ .format(letter_contact_id, service_id) raise BadRequestError(message=message) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 97f6dfeac..cb0d23864 100644 --- a/app/v2/notifications/post_notifications.py +++ b/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) from app.schema_validation import validate from app.v2.errors import BadRequestError from app.v2.notifications.create_response import ( @@ -132,6 +132,7 @@ def post_notification(notification_type): form.get('personalisation', {}), authenticated_service, notification_type, + check_char_count=False ) reply_to = get_reply_to_text(notification_type, form, template) @@ -189,6 +190,9 @@ def process_sms_or_email_notification( # We changed personalisation which means we need to update the content template_with_content.values = personalisation + # validate content length after url is replaced in personalisation. + check_content_char_count(template_with_content) + resp = create_response_for_post_notification( notification_id=notification_id, client_reference=form.get('reference', None), diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index de6ce99ce..a7be7e5e0 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -404,6 +404,25 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic mock_check_message_is_too_long.assert_called_once_with("content") +def test_validate_template_calls_all_validators_exception_message_too_long(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') + 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", + check_char_count=False) + + 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") + assert not mock_check_message_is_too_long.called + + @pytest.mark.parametrize('key_type', ['team', 'live', 'test']) def test_that_when_exceed_rate_limit_request_fails( key_type,