mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-02 08:25:15 -05:00
Fix the problem with updating the reply_to or service_letter_contact_id for templates.
The history was not being updated properly, we think this is because the declaritive attribute is not being set propery by the property. When reply_to: None it will update the service_letter_contact_id, but not the service_letter_contact, we think when the history_meta is build the history class and checking if the value is updated it depends which attribute it is checking first. In order to fix this issue, there is a new dao method to update the reply_to on the Template and insert a new Template history.
This commit is contained in:
@@ -33,6 +33,36 @@ def dao_update_template(template):
|
||||
db.session.add(template)
|
||||
|
||||
|
||||
@transactional
|
||||
def dao_update_template_reply_to(template_id, reply_to):
|
||||
Template.query.filter_by(id=template_id).update(
|
||||
{"service_letter_contact_id": reply_to,
|
||||
"updated_at": datetime.utcnow(),
|
||||
"version": Template.version + 1,
|
||||
}
|
||||
)
|
||||
template = Template.query.filter_by(id=template_id).one()
|
||||
|
||||
history = TemplateHistory(**
|
||||
{
|
||||
"id": template.id,
|
||||
"name": template.name,
|
||||
"template_type": template.template_type,
|
||||
"created_at": template.created_at,
|
||||
"updated_at": template.updated_at,
|
||||
"content": template.content,
|
||||
"service_id": template.service_id,
|
||||
"subject": template.subject,
|
||||
"created_by_id": template.created_by_id,
|
||||
"version": template.version,
|
||||
"archived": template.archived,
|
||||
"process_type": template.process_type,
|
||||
"service_letter_contact_id": template.service_letter_contact_id
|
||||
})
|
||||
db.session.add(history)
|
||||
return history
|
||||
|
||||
|
||||
@transactional
|
||||
def dao_redact_template(template, user_id):
|
||||
template.template_redacted.redact_personalisation = True
|
||||
|
||||
@@ -11,7 +11,8 @@ from app.dao.templates_dao import (
|
||||
dao_redact_template,
|
||||
dao_get_template_by_id_and_service_id,
|
||||
dao_get_all_templates_for_service,
|
||||
dao_get_template_versions
|
||||
dao_get_template_versions,
|
||||
dao_update_template_reply_to
|
||||
)
|
||||
from notifications_utils.template import SMSMessageTemplate
|
||||
from app.dao.services_dao import dao_fetch_service_by_id
|
||||
@@ -81,6 +82,11 @@ def update_template(service_id, template_id):
|
||||
if data.get('redact_personalisation') is True:
|
||||
return redact_template(fetched_template, data)
|
||||
|
||||
if "reply_to" in data:
|
||||
check_reply_to(service_id, data.get("reply_to"), fetched_template.template_type)
|
||||
updated = dao_update_template_reply_to(template_id=template_id, reply_to=data.get("reply_to"))
|
||||
return jsonify(data=template_schema.dump(updated).data), 200
|
||||
|
||||
current_data = dict(template_schema.dump(fetched_template).data.items())
|
||||
updated_template = dict(template_schema.dump(fetched_template).data.items())
|
||||
updated_template.update(data)
|
||||
@@ -89,8 +95,6 @@ def update_template(service_id, template_id):
|
||||
if _template_has_not_changed(current_data, updated_template):
|
||||
return jsonify(data=updated_template), 200
|
||||
|
||||
check_reply_to(service_id, updated_template.get('reply_to', None), fetched_template.template_type)
|
||||
|
||||
over_limit = _content_count_greater_than_limit(updated_template['content'], fetched_template.template_type)
|
||||
if over_limit:
|
||||
char_count_limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
|
||||
@@ -160,7 +164,7 @@ def get_template_versions(service_id, template_id):
|
||||
def _template_has_not_changed(current_data, updated_template):
|
||||
return all(
|
||||
current_data[key] == updated_template[key]
|
||||
for key in ('name', 'content', 'subject', 'archived', 'process_type', 'reply_to')
|
||||
for key in ('name', 'content', 'subject', 'archived', 'process_type')
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -11,7 +11,8 @@ from app.dao.templates_dao import (
|
||||
dao_update_template,
|
||||
dao_get_template_versions,
|
||||
dao_get_templates_for_cache,
|
||||
dao_redact_template)
|
||||
dao_redact_template, dao_update_template_reply_to
|
||||
)
|
||||
from app.models import Template, TemplateHistory, TemplateRedacted
|
||||
|
||||
from tests.app.conftest import sample_template as create_sample_template
|
||||
@@ -88,7 +89,7 @@ def test_update_template(sample_service, sample_user):
|
||||
assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'new name'
|
||||
|
||||
|
||||
def test_update_template_reply_to(sample_service, sample_user):
|
||||
def test_dao_update_template_reply_to_none_to_some(sample_service, sample_user):
|
||||
letter_contact = create_letter_contact(sample_service, 'Edinburgh, ED1 1AA')
|
||||
|
||||
data = {
|
||||
@@ -100,38 +101,71 @@ def test_update_template_reply_to(sample_service, sample_user):
|
||||
}
|
||||
template = Template(**data)
|
||||
dao_create_template(template)
|
||||
created = dao_get_all_templates_for_service(sample_service.id)[0]
|
||||
created = Template.query.get(template.id)
|
||||
assert created.reply_to is None
|
||||
assert created.service_letter_contact_id is None
|
||||
|
||||
created.reply_to = letter_contact.id
|
||||
dao_update_template(created)
|
||||
assert dao_get_all_templates_for_service(sample_service.id)[0].reply_to == letter_contact.id
|
||||
dao_update_template_reply_to(template_id=template.id,
|
||||
reply_to=letter_contact.id)
|
||||
|
||||
template_history = TemplateHistory.query.filter_by(id=created.id, version=2).one()
|
||||
assert template_history.service_letter_contact_id
|
||||
|
||||
|
||||
def test_update_template_reply_to_updates_history(sample_service, sample_user):
|
||||
letter_contact = create_letter_contact(sample_service, 'Edinburgh, ED1 1AA')
|
||||
|
||||
data = {
|
||||
'name': 'Sample Template',
|
||||
'template_type': "letter",
|
||||
'content': "Template content",
|
||||
'service': sample_service,
|
||||
'created_by': sample_user,
|
||||
}
|
||||
template = Template(**data)
|
||||
dao_create_template(template)
|
||||
created = dao_get_all_templates_for_service(sample_service.id)[0]
|
||||
assert created.reply_to is None
|
||||
|
||||
created.reply_to = letter_contact.id
|
||||
dao_update_template(created)
|
||||
assert dao_get_all_templates_for_service(sample_service.id)[0].reply_to == letter_contact.id
|
||||
updated = Template.query.get(template.id)
|
||||
assert updated.reply_to == letter_contact.id
|
||||
assert updated.version == 2
|
||||
assert updated.updated_at
|
||||
|
||||
template_history = TemplateHistory.query.filter_by(id=created.id, version=2).one()
|
||||
assert template_history.service_letter_contact_id == letter_contact.id
|
||||
assert template_history.updated_at == updated.updated_at
|
||||
|
||||
|
||||
def test_dao_update_tempalte_reply_to_some_to_some(sample_service, sample_user):
|
||||
letter_contact = create_letter_contact(sample_service, 'Edinburgh, ED1 1AA')
|
||||
letter_contact_2 = create_letter_contact(sample_service, 'London, N1 1DE')
|
||||
|
||||
data = {
|
||||
'name': 'Sample Template',
|
||||
'template_type': "letter",
|
||||
'content': "Template content",
|
||||
'service': sample_service,
|
||||
'created_by': sample_user,
|
||||
'service_letter_contact_id': letter_contact.id
|
||||
}
|
||||
template = Template(**data)
|
||||
dao_create_template(template)
|
||||
created = Template.query.get(template.id)
|
||||
dao_update_template_reply_to(template_id=created.id, reply_to=letter_contact_2.id)
|
||||
updated = Template.query.get(template.id)
|
||||
assert updated.reply_to == letter_contact_2.id
|
||||
assert updated.version == 2
|
||||
assert updated.updated_at
|
||||
|
||||
updated_history = TemplateHistory.query.filter_by(id=created.id, version=2).one()
|
||||
assert updated_history.service_letter_contact_id == letter_contact_2.id
|
||||
assert updated_history.updated_at == updated_history.updated_at
|
||||
|
||||
|
||||
def test_dao_update_tempalte_reply_to_some_to_none(sample_service, sample_user):
|
||||
letter_contact = create_letter_contact(sample_service, 'Edinburgh, ED1 1AA')
|
||||
data = {
|
||||
'name': 'Sample Template',
|
||||
'template_type': "letter",
|
||||
'content': "Template content",
|
||||
'service': sample_service,
|
||||
'created_by': sample_user,
|
||||
'service_letter_contact_id': letter_contact.id
|
||||
}
|
||||
template = Template(**data)
|
||||
dao_create_template(template)
|
||||
created = Template.query.get(template.id)
|
||||
dao_update_template_reply_to(template_id=created.id, reply_to=None)
|
||||
updated = Template.query.get(template.id)
|
||||
assert updated.reply_to is None
|
||||
assert updated.version == 2
|
||||
assert updated.updated_at
|
||||
|
||||
history = TemplateHistory.query.filter_by(id=created.id, version=2).one()
|
||||
assert history.service_letter_contact_id is None
|
||||
assert history.updated_at == updated.updated_at
|
||||
|
||||
|
||||
def test_redact_template(sample_template):
|
||||
|
||||
@@ -662,6 +662,28 @@ def test_update_template_reply_to(client, sample_letter_template):
|
||||
assert th.service_letter_contact_id == letter_contact.id
|
||||
|
||||
|
||||
def test_update_template_reply_to_set_to_blank(client, notify_db_session):
|
||||
auth_header = create_authorization_header()
|
||||
service = create_service(service_permissions=['letter'])
|
||||
letter_contact = create_letter_contact(service, "Edinburgh, ED1 1AA")
|
||||
template = create_template(service=service, template_type='letter', reply_to=letter_contact.id)
|
||||
|
||||
data = {
|
||||
'reply_to': None,
|
||||
}
|
||||
|
||||
resp = client.post('/service/{}/template/{}'.format(template.service_id, template.id),
|
||||
data=json.dumps(data),
|
||||
headers=[('Content-Type', 'application/json'), auth_header])
|
||||
|
||||
assert resp.status_code == 200, resp.get_data(as_text=True)
|
||||
|
||||
template = dao_get_template_by_id(template.id)
|
||||
assert template.service_letter_contact_id is None
|
||||
th = TemplateHistory.query.filter_by(id=template.id, version=2).one()
|
||||
assert th.service_letter_contact_id is None
|
||||
|
||||
|
||||
def test_update_template_with_foreign_service_reply_to(client, sample_letter_template):
|
||||
auth_header = create_authorization_header()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user