Change how we validate the length of templates.

We want to add validation for an email that's too long, that way the user knows why the message is failing. At the moment if an email is too long it will get a technical failure, after the retries fail. This way the email post will get a validation error.

Once this: https://github.com/alphagov/notifications-utils/pull/804 is reverted, we can update the utils version.
This commit is contained in:
Rebecca Law
2020-11-04 14:25:22 +00:00
parent 918bf6d97c
commit 5bacfc1df9
3 changed files with 35 additions and 10 deletions

View File

@@ -27,7 +27,6 @@ from app.serialised_models import SerialisedTemplate
from gds_metrics.metrics import Histogram from gds_metrics.metrics import Histogram
REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram( REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram(
'redis_exceeded_rate_limit_duration_seconds', 'redis_exceeded_rate_limit_duration_seconds',
'Time taken to check rate limit', 'Time taken to check rate limit',
@@ -157,8 +156,7 @@ def check_notification_content_is_not_empty(template_with_content):
raise BadRequestError(message=message) 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: try:
template = SerialisedTemplate.from_id_and_service_id(template_id, service.id) template = SerialisedTemplate.from_id_and_service_id(template_id, service.id)
except NoResultFound: except NoResultFound:
@@ -173,6 +171,10 @@ 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)
# 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) check_content_char_count(template_with_content)
return template, template_with_content return template, template_with_content

View File

@@ -52,7 +52,7 @@ from app.notifications.validators import (
validate_address, validate_address,
validate_and_format_recipient, validate_and_format_recipient,
validate_template, validate_template,
) check_content_char_count)
from app.schema_validation import validate from app.schema_validation import validate
from app.v2.errors import BadRequestError from app.v2.errors import BadRequestError
from app.v2.notifications.create_response import ( from app.v2.notifications.create_response import (
@@ -132,6 +132,7 @@ def post_notification(notification_type):
form.get('personalisation', {}), form.get('personalisation', {}),
authenticated_service, authenticated_service,
notification_type, notification_type,
check_char_count=False
) )
reply_to = get_reply_to_text(notification_type, form, template) 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 # We changed personalisation which means we need to update the content
template_with_content.values = personalisation 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( resp = create_response_for_post_notification(
notification_id=notification_id, notification_id=notification_id,
client_reference=form.get('reference', None), client_reference=form.get('reference', None),

View File

@@ -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") 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']) @pytest.mark.parametrize('key_type', ['team', 'live', 'test'])
def test_that_when_exceed_rate_limit_request_fails( def test_that_when_exceed_rate_limit_request_fails(
key_type, key_type,