django-model-utils
django-model-utils copied to clipboard
#435 Fix instance creation state check
Problem
FieldTracker has incorrect behaviour if PK field isn't a AutoFIeld
. Link to issue https://github.com/jazzband/django-model-utils/issues/435.
Solution
Django has a ModelState
object to determine if objects is in creation state or already exists in db. Replacing if not self.instance.pk is None
with if self.instance._state.adding
solve the problem.
Commandments
- [x] Write PEP8 compliant code.
- [ ] Cover it with tests.
- [x] Update
CHANGES.rst
file to describe the changes, and quote according issue withGH-<issue_number>
. - [x] Pay attention to backward compatibility, or if it breaks it, explain why.
- [ ] Update documentation (if relevant).
@auvipy I fixed one of two failed tests. Trying to fix last test but can't figure out what is the problem. Stucked with fixing this test https://github.com/jazzband/django-model-utils/blob/master/tests/test_fields/test_field_tracker.py#L590.
Can't understand why there is {'id': None}
in changed()
call on deffered_instance
.
def test_post_save_changed(self):
self.update_instance(some_file=self.some_file)
self.assertChanged()
previous_file = self.instance.some_file
self.instance.some_file = self.another_file
self.assertChanged(some_file=previous_file)
# test deferred file field
deferred_instance = self.tracked_class.objects.defer('some_file')[0]
deferred_instance.some_file # access field to fetch from database
> self.assertChanged(tracker=deferred_instance.tracker)
tests/test_fields/test_field_tracker.py:601:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_fields/test_field_tracker.py:36: in assertChanged
self.assertEqual(tracker.changed(), kwargs)
E AssertionError: {'id': None} != {}
E - {'id': None}
E + {}
Will be glad if you can help me.
@auvipy I'm not sure why a review by me was requested. Was this an error, or do you want me to help with something?
GitHub shows you recently modified the files that's why I requested a review
Looking at my previous contributions to this project, the only file I have edited that is in this pull request is AUTHORS.rst
. Since this pull request does not contain substantial changes to that file, I will refrain from providing a review.
Looks like at the moment of execution set_saved_fields
method self.instance._state.adding
is always True
. It becomes False
inside django's Model.from_db
method
Found the problem.
initialize_tracker
called on model's post_init
signal and _state
at this moment contains default values
https://docs.djangoproject.com/en/3.0/ref/signals/#post-init
Note instance._state isn’t set before sending the post_init signal, so _state attributes always have their default values. For example, _state.db is None.