From 17f5742e2c4ff3085bb9406d93c6a62699de54b7 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Thu, 23 Nov 2017 16:44:14 +0000 Subject: [PATCH 1/2] Use model constructors when creating history instances `create_history` used to initialise an empty model instance and assign attributes to it one by one. Assigned attributes are collected by iterating over internal state of the modified record and stored in a dictionary, so the order in which they are set on the history instance is undefined. This works fine for most model instances, but recent changes to the Template models require `template_type` to be set before `reply_to`, otherwise Template doesn't know which column to use to persist the `reply_to` value. This means that we need to order the attributes when they're being assigned. We achieved this by defining a constructor for Template objects that assigns `template_type` first. In order to make it work with `create_history` we replace individual `setattr` calls with a single call to the Template constructor which passes the entire data dictionary as keyword arguments. This works because all persisted attributes are defined as columns and SQLAlchemy accepts all column values as constructor arguments. --- app/history_meta.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/history_meta.py b/app/history_meta.py index d49c39e8a..623f86ba3 100644 --- a/app/history_meta.py +++ b/app/history_meta.py @@ -176,7 +176,6 @@ def create_history(obj, history_cls=None): history_mapper = obj.__history_mapper__ history_cls = history_mapper.class_ - history = history_cls() obj_mapper = object_mapper(obj) obj_state = attributes.instance_state(obj) @@ -201,7 +200,7 @@ def create_history(obj, history_cls=None): # not yet have a value before insert elif isinstance(prop, RelationshipProperty): - if hasattr(history, prop.key + '_id'): + if hasattr(history_cls, prop.key + '_id'): foreign_obj = getattr(obj, prop.key) # if it's a nullable relationship, foreign_obj will be None, and we actually want to record that data[prop.key + '_id'] = getattr(foreign_obj, 'id', None) @@ -218,9 +217,4 @@ def create_history(obj, history_cls=None): data['version'] = obj.version data['created_at'] = obj.created_at - for key, value in data.items(): - if not hasattr(history_cls, key): - raise AttributeError("{} has no attribute '{}'".format(history_cls.__name__, key)) - setattr(history, key, value) - - return history + return history_cls(**data) From 54ef53cefe6f78f829e89f6a890a10f64c9d7317 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Thu, 23 Nov 2017 17:02:15 +0000 Subject: [PATCH 2/2] Add a test for creating a template with reply_to through the API --- tests/app/template/test_rest.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 68987f4b6..f754baa24 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -565,6 +565,29 @@ def test_update_set_process_type_on_template(client, sample_template): assert template.process_type == 'priority' +def test_create_a_template_with_reply_to(admin_request, sample_user): + service = create_service(service_permissions=['letter']) + letter_contact = create_letter_contact(service, "Edinburgh, ED1 1AA") + data = { + 'name': 'my template', + 'subject': 'subject', + 'template_type': 'letter', + 'content': 'template content', + 'service': str(service.id), + 'created_by': str(sample_user.id), + 'reply_to': str(letter_contact.id), + } + + json_resp = admin_request.post('template.create_template', service_id=service.id, _data=data, _expected_status=201) + + assert json_resp['data']['template_type'] == 'letter' + assert json_resp['data']['reply_to'] == str(letter_contact.id) + + template = Template.query.get(json_resp['data']['id']) + from app.schemas import template_schema + assert sorted(json_resp['data']) == sorted(template_schema.dump(template).data) + + def test_get_template_reply_to(client, sample_letter_template): auth_header = create_authorization_header() letter_contact = create_letter_contact(sample_letter_template.service, "Edinburgh, ED1 1AA")