mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-04 10:21:14 -05:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user