Field tracker previous should return default for new instances
Opened this issue · 0 comments
Field tracker documentation says this:
Returns None when the model instance isn’t saved yet.
And also this:
The has_changed method relies on previous to determine whether a field’s values has changed.
The resulting behavior is fine for fields that accept and default to None
, but as soon as you're wanting to track fields that have a custom default
, or only accept blank
, you'll get odd results particularly from has_changed
.
For example on name = models.TextField(blank=True)
on a new instance, previous
will return None
, while the current value will be ""
, which means that when has_changed
is called, it'll return True
, because "" != None
This creates goitchas that require the developer to be mindful of whether the instance is new, and do additional checks in that case, by returning the default value in previous, the result of has_changed
is more reliable for unsaved instances.
Environment
- Django Model Utils version: 4.2.0
- Django version: 3.2.12
- Python version: 3.8.12
Code examples
This example extension covers everything that I need right now – it might get hairy in the case of a callable default
; but this results in the behavior I expected and covers things like bools that default to True
and blank=True, null=False
fields:
from django.db.models import NOT_PROVIDED
import model_utils
class FieldInstanceTracker(model_utils.tracker.FieldInstanceTracker):
def previous(self, attr):
result = super().previous(attr)
if self.instance._state.adding and result is None:
field = self.instance._meta.get_field(attr)
if field.default is not NOT_PROVIDED:
result = field.default
elif field.blank and not field.null:
result = ""
return result
class FieldTracker(model_utils.tracker.FieldTracker):
tracker_class = FieldInstanceTracker
I don't care so much that previous
is inaccurate, that kind of makes sense for new instances, but given previous
is then used for has_changed
you can get lots of false positives, especially when migrating from ad-hoc __init__
based solves.