fix(mass_model_updater): never skip a deferred tracked field as "unchanged" [sc-13375]#15112
Closed
Maffooch wants to merge 1 commit into
Closed
fix(mass_model_updater): never skip a deferred tracked field as "unchanged" [sc-13375]#15112Maffooch wants to merge 1 commit into
Maffooch wants to merge 1 commit into
Conversation
…anged" skip_unchanged compares each row's tracked fields before/after `function` by reading model.__dict__.get(field), to avoid a deferred-field query. But when a tracked field is deferred (the queryset used .only()/.defer() and omitted it), __dict__ has no entry and reads as None. A real recompute to None then looks unchanged (None == None) and the write is silently dropped, leaving the stale persisted value. Guard the no-op check with model.get_deferred_fields(): if any tracked field is deferred, skip the comparison and always write that row. Add regression tests covering a deferred field recomputed to None (the dropped-write case) and to a new value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dogboat
approved these changes
Jun 30, 2026
Contributor
Author
|
Closing in favor or #15114 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
mass_model_updater(..., skip_unchanged=True)(added in #15046) decides whether to write each row by snapshotting its trackedfieldsbefore callingfunction, readingmodel.__dict__.get(field)to avoid triggering a deferred-field query.But when a tracked field is deferred — the caller's queryset used
.only()/.defer()and omitted it —__dict__has no entry, so the snapshot readsNone. Iffunctionthen recomputes that field toNone, the comparisonNone == Nonemarks the row "unchanged" and the write is silently dropped, leaving the stale persisted value in the DB.This bit DefectDojo Pro's risk-mode SLA recalculation: its queryset deferred
sla_expiration_datewhile updating it, so clearing a finding's SLA toNone(enforcement disabled) never persisted — the finding kept its old SLA. (Companion fix in dojo-pro adds the field to its.only(...); this PR removes the footgun at the source.)Fix
Guard the no-op check with
model.get_deferred_fields(): if any tracked field is deferred, skip the comparison and always write that row. No extra query; non-deferred rows keep the fast skip-unchanged path.Tests
Adds two regression tests to
unittests/test_mass_model_updater.py:test_writes_deferred_field_recomputed_to_none— a deferred field recomputed toNonemust issue an UPDATE and persist (fails without this fix: 0 UPDATEs).test_writes_deferred_field_recomputed_to_value— deferred field updated to a value persists.Full
TestMassModelUpdatersuite (11 tests) passes; reverting the guard makes the None test fail with0 not greater than 0.Scope
skip_unchangedexists only ondev(not in 3.0.200 /bugfix/master), so released versions are unaffected.