mirror of
https://github.com/GSA/notifications-api.git
synced 2026-09-09 17:45:41 -04:00
Quietly ignore extra personalisation
> If a user makes an API request with additional personalisation fields, > we should simply discard any fields that the template doesn't have. > > This gives a couple of related advantages: > > - modifying template parameters no longer requires downtime for > clients - as they can pass in extra new parameters before a template > change, or continue passing in old unused parameters after removing > them from a template > > - services can pass in large user objects, for example, and then play > around with templates adding and removing fields at will > > we should make sure we still return an error if a user doesn't pass in > a required parameter. – https://www.pivotaltracker.com/story/show/140774195
This commit is contained in:
@@ -23,11 +23,6 @@ def check_placeholders(template_object):
|
|||||||
message = 'Template missing personalisation: {}'.format(", ".join(template_object.missing_data))
|
message = 'Template missing personalisation: {}'.format(", ".join(template_object.missing_data))
|
||||||
raise BadRequestError(fields=[{'template': message}], message=message)
|
raise BadRequestError(fields=[{'template': message}], message=message)
|
||||||
|
|
||||||
if template_object.additional_data:
|
|
||||||
message = 'Template personalisation not needed for template: {}'.format(
|
|
||||||
", ".join(template_object.additional_data))
|
|
||||||
raise BadRequestError(fields=[{'template': message}], message=message)
|
|
||||||
|
|
||||||
|
|
||||||
def persist_notification(template_id,
|
def persist_notification(template_id,
|
||||||
template_version,
|
template_version,
|
||||||
|
|||||||
@@ -300,11 +300,6 @@ def create_template_object_for_notification(template, personalisation):
|
|||||||
errors = {'template': [message]}
|
errors = {'template': [message]}
|
||||||
raise InvalidRequest(errors, status_code=400)
|
raise InvalidRequest(errors, status_code=400)
|
||||||
|
|
||||||
if template_object.additional_data:
|
|
||||||
message = 'Personalisation not needed for template: {}'.format(", ".join(template_object.additional_data))
|
|
||||||
errors = {'template': [message]}
|
|
||||||
raise InvalidRequest(errors, status_code=400)
|
|
||||||
|
|
||||||
if (
|
if (
|
||||||
template_object.template_type == SMS_TYPE and
|
template_object.template_type == SMS_TYPE and
|
||||||
template_object.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT')
|
template_object.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT')
|
||||||
|
|||||||
@@ -101,13 +101,6 @@ def preview_template_by_id_and_service_id(service_id, template_id):
|
|||||||
]}, status_code=400
|
]}, status_code=400
|
||||||
)
|
)
|
||||||
|
|
||||||
if template_object.additional_data:
|
|
||||||
raise InvalidRequest(
|
|
||||||
{'template': [
|
|
||||||
'Personalisation not needed for template: {}'.format(", ".join(template_object.additional_data))
|
|
||||||
]}, status_code=400
|
|
||||||
)
|
|
||||||
|
|
||||||
data['subject'], data['content'] = template_object.subject, str(template_object)
|
data['subject'], data['content'] = template_object.subject, str(template_object)
|
||||||
|
|
||||||
return jsonify(data)
|
return jsonify(data)
|
||||||
|
|||||||
@@ -913,14 +913,12 @@ def test_create_template_raises_invalid_request_exception_with_missing_personali
|
|||||||
assert {'template': ['Missing personalisation: Name']} == e.value.message
|
assert {'template': ['Missing personalisation: Name']} == e.value.message
|
||||||
|
|
||||||
|
|
||||||
def test_create_template_raises_invalid_request_exception_with_too_much_personalisation_data(
|
def test_create_template_doesnt_raise_with_too_much_personalisation(
|
||||||
sample_template_with_placeholders
|
sample_template_with_placeholders
|
||||||
):
|
):
|
||||||
from app.notifications.rest import create_template_object_for_notification
|
from app.notifications.rest import create_template_object_for_notification
|
||||||
template = Template.query.get(sample_template_with_placeholders.id)
|
template = Template.query.get(sample_template_with_placeholders.id)
|
||||||
with pytest.raises(InvalidRequest) as e:
|
|
||||||
create_template_object_for_notification(template, {'name': 'Jo', 'extra': 'stuff'})
|
create_template_object_for_notification(template, {'name': 'Jo', 'extra': 'stuff'})
|
||||||
assert {'template': ['Personalisation not needed for template: foo']} in e.value.message
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(
|
@pytest.mark.parametrize(
|
||||||
|
|||||||
@@ -36,11 +36,9 @@ def test_create_content_for_notification_fails_with_missing_personalisation(samp
|
|||||||
create_content_for_notification(template, None)
|
create_content_for_notification(template, None)
|
||||||
|
|
||||||
|
|
||||||
def test_create_content_for_notification_fails_with_additional_personalisation(sample_template_with_placeholders):
|
def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders):
|
||||||
template = Template.query.get(sample_template_with_placeholders.id)
|
template = Template.query.get(sample_template_with_placeholders.id)
|
||||||
with pytest.raises(BadRequestError) as e:
|
|
||||||
create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
|
create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
|
||||||
assert e.value.message == 'Template personalisation not needed for template: Additional placeholder'
|
|
||||||
|
|
||||||
|
|
||||||
@freeze_time("2016-01-01 11:09:00.061258")
|
@freeze_time("2016-01-01 11:09:00.061258")
|
||||||
|
|||||||
@@ -374,8 +374,9 @@ def test_should_get_a_single_template(
|
|||||||
'about your ((thing))',
|
'about your ((thing))',
|
||||||
'hello ((name)) we’ve received your ((thing))',
|
'hello ((name)) we’ve received your ((thing))',
|
||||||
'/service/{}/template/{}/preview?name=Amala&thing=document&foo=bar',
|
'/service/{}/template/{}/preview?name=Amala&thing=document&foo=bar',
|
||||||
None, None,
|
'about your document',
|
||||||
'Personalisation not needed for template: foo'
|
'hello Amala we’ve received your document',
|
||||||
|
None,
|
||||||
)
|
)
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user