FieldTracker broken in 4.1.1
resurrexi opened this issue · 2 comments
Problem
It seems like FieldTracker isn't able to detect changes on a field anymore. I'm not sure if it's because there were changes made to its implementation that prevents it from observing a field's change state when the field's model instance is being saved.
What I would do is override the model's save
method and add a "post-save hook" where I check if the field has changed, and if so, then I'll run some additional logic.
Let's take for example, I have an Employee
model that's tied to an extended auth user model. If an employee is activated/deactivated, I would like the employee's associated user profile to also be activated/deactivated.
class Employee(models.Model):
...
is_active = models.BooleanField(default=True)
field_tracker = FieldTracker(fields=["is_active"])
def save(self, *args, **kwargs):
super().save(*args, **kwargs)
# post-save
if self.field_tracker.has_changed("is_active"):
get_user_model().objects.filter(employee_profile=self).update(
is_active=self.is_active
)
This post-save logic would previously work on django-model-utils==4.0.0
, as my unit test would pass:
class EmployeeModelTest(TestCase):
def test_employee_active_flag_updates_login_account_active(self):
# prep data
employee = EmployeeFactory.create()
user = ProfileFactory.create(employee_profile=employee)
# first ensure user is active
self.assertTrue(user.is_active)
employee.is_active = False
employee.save()
# user should also now be deactivated
user.refresh_from_db()
self.assertFalse(user.is_active) # fails here
For completeness (to rule out factoryboy), I also did a manual test, where I would log into django admin to deactivate the employee. The employee's associated user account still wouldn't deactivate.
Environment
- Django Model Utils version: 4.1.1
- Django version: 2.2.22
- Python version: 3.8.3
- Other libraries used, if any:
factory-boy==2.12.0
for testing
Relates to #404
Before 4.1 this code will behave like this:
model_patched_save().start
save()
super().save() start
save_base() start
pre_save signal handling
db insert/update
post_save signal handling
save_base() finish
super().save() finish
if has_changed:
...
reset_tracker_state()
model_patched_save().finish
New implementation moves reset_tracker_state to earlier stage
save()
super().save() start
model_patched_save_base().start
save_base() start
pre_save signal handling
db insert/update
post_save signal handling
save_base() finish
reset_tracker_state()
model_patched_save().finish
super().save() finish
if has_changed:
...
You may check "changed" before super().save or use signals.
Ok. Thanks for clarifying the order of where the reset state is executed. I want to avoid using signals as much as possible so I'll just move the check before the super().save
call.