Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FieldTracker should patch save_base instead of save #404

Merged
merged 4 commits into from Dec 15, 2019

Conversation

tumb1er
Copy link
Contributor

@tumb1er tumb1er commented Dec 15, 2019

Problem

If model with FieldTracker adds extra fields to update_fields in save, these changes are not reflected in FieldTracker state.

Solution

FieldTracker should patch save_base instead of save:

  • save_base is called from save so final update_fields value is captured
  • save_base is never overridden in models.Model subclasses

Commandments

  • Write PEP8 compliant code.
  • Cover it with tests.
  • Update CHANGES.rst file to describe the changes, and quote according issue with GH-<issue_number>.
  • Pay attention to backward compatibility, or if it breaks it, explain why.
  • Update documentation (if relevant).

FieldTracker does not see update_fields changed in tracked model.save() method
This change allows proper capturing update_fields kwarg if is is changed in overridden save() method.
@codecov
Copy link

codecov bot commented Dec 15, 2019

Codecov Report

Merging #404 into master will decrease coverage by 0.13%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
- Coverage   94.08%   93.95%   -0.14%     
==========================================
  Files           6        6              
  Lines         744      744              
==========================================
- Hits          700      699       -1     
- Misses         44       45       +1
Impacted Files Coverage Δ
model_utils/tracker.py 81.44% <100%> (-0.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d756a4a...e20d845. Read the comment docs.

@auvipy auvipy merged commit 1386c37 into jazzband:master Dec 15, 2019
tumb1er added a commit to tumb1er/django-model-utils that referenced this pull request Sep 13, 2021
auvipy pushed a commit that referenced this pull request Oct 8, 2021
* Add tracker context manager and decorator

* Handle auto-field warning in tests

* Use tracker context in monkey-patched methods

* Test delaying set_saved_fields call with context manager

* Docs for tracker context

* Describe FieldsContext context manager in changes

* Fix unused import

* #494 add breaking changes note on 4.1.1 release for #404

* #494 fix typo

* #494 add docstring for FieldsContext

* #494 move breaking changes from 4.1.1 to 4.1.0

* #494 fix typo and add some more docstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants