From 17f5742e2c4ff3085bb9406d93c6a62699de54b7 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Thu, 23 Nov 2017 16:44:14 +0000 Subject: [PATCH] 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)