-
Notifications
You must be signed in to change notification settings - Fork 921
Description
❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.
Current behavior
Currently, there's some logic in some of the pre_save methods on the following models, which attempts to ensure that the user fields are set under specific circumstances:
CourseSession.created_by_idCourseSessionAssignment.assigned_by_idUnitTestAssignment.activated_by_id
The logic is the form of if self._state.adding and self.assigned_by is None which ensures the ID is set when the model is added. The logic is made more complicated by the behavior expected in those models' deserialize method (ref)
When Morango deserializes a model, it passes update_dirty_bit_to=False to the model's save method. This is because the dirty bit tracks whether the model's data is updated with respect to the data serialized in the morango store.
Desired behavior
The above behavior can be further restricted by using information that is made available when Morango deserializes a model.
- The
AbstractFacilityDataModelmodel should be updated to pass the same arguments given tosavetopre_save - All descendants of the above model should be updated to ensure they are at least passive to arguments being passed to
pre_save - The current behavior referenced above, in the course models, should be updated to check whether
update_dirty_bit_to=Falsewas passed topre_save(and hencesave) and if so, allow the user ID fields to be null. Otherwise, it should enforce those as not nullable.
Notes
- This is marked as a P1 because I'm unsure whether the existing logic is incompatible with syncing. If it is, then this should be promoted to a P0.
Value add
Better aligns the logic to the conditions under which the fields could be null, and is more restrictive in that sense than the current logic, which protects data integrity.
Possible tradeoffs
I'm fairly sure that the current behavior will not work appropriately during syncing, since the initial sync should still result in self._state.adding being True during deserialization.
