deferred-fields-patch #1064
deferred-fields-patch #1064
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1064 +/- ##
=======================================
Coverage 97.24% 97.25%
=======================================
Files 23 23
Lines 1234 1238 +4
Branches 200 201 +1
=======================================
+ Hits 1200 1204 +4
Misses 16 16
Partials 18 18
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Anything I can do to help get this PR accepted? |
simple_history/models.py
Outdated
| field_attrs = {field.attname for field in fields} | ||
| deferred_attrs = instance.get_deferred_fields() | ||
| # Load all deferred fields that are present in fields_included | ||
| for attr in field_attrs.intersection(deferred_attrs): |
There was a problem hiding this comment.
Maybe it would be better to load all deferred fields in a single query, as Django loads fields one at a time on access (See https://docs.djangoproject.com/en/5.1/ref/models/querysets/#defer)
Updated code may look something like this:
instance.refresh_from_db(fields=field_attrs.intersection(deferred_attrs))
There was a problem hiding this comment.
Yes, good idea, I have implemented this. Hopefully this gets reviewed and accepted soon!
|
Hi @JordanHyatt, |
|
New pull request #1393 |
|
Yeah I originally did it on my forks master branch, but I wanted to work on HistoricOneToOneField so I had to open a new PR which it looks like you found, thanks! |
* added patch and test * added myself to AUTHORS * adde my change to CHANGES.rst * fixed bug to connect to pre_delete instead of post_delete * implemented suggestion made by @jwaschkau in issue #1064 * added .python-version to the .gitignore * - added check to signal to see if instance is history enabled before loading its deferred fields * check that fields is "truthy" before calling refresh_from_db * added to test to ensure by-pass logic is covered
Provided a patch that allows objects to be deleted when they have deferred fields present
Description
Added a pre_delete signal that checks to see if there are any deferred fields on the instance that are present in the included_fileds
Related Issue
This is related to issue #678
Motivation and Context
This allows users of simple_history to delete objects without having to consider the consequences of deferring fields in their query
How Has This Been Tested?
Added a test that confirms objects with deferred fields can in-fact be deleted now
Types of changes
Checklist:
pre-commit runcommand to format and lint.AUTHORS.rstCHANGES.rst