[FIX] base_tier_validation_correction: honest revert via snapshot#45
Open
bosd wants to merge 1 commit into
Open
[FIX] base_tier_validation_correction: honest revert via snapshot#45bosd wants to merge 1 commit into
bosd wants to merge 1 commit into
Conversation
revert() recomputed reviewers from the current tier.definition state via _get_reviewers(), so any edit to the definition between correct and revert (reviewer change, group membership change, field reassignment) would silently leak into the reverted state -- not what "revert" means. Snapshot each affected review's reviewer_ids on the tier.correction.item at correct() time and restore from that snapshot at revert() time. Legacy items without a snapshot fall back to the previous behaviour.
Contributor
|
Hi @kittiu, |
OCA-git-bot
added
series:19.0
mod:base_tier_validation_correction
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
2 participants
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
tier.correction.do_revertrecomputed reviewers from the currenttier.definitionstate by callingreview._get_reviewers(). Any edit tothe definition between
correctandrevert— reviewer reassignment, agroup membership change, a different reviewer field — would silently
leak into the reverted state. That is not what "revert" means: the user
expects to get back exactly the reviewers that were in place before the
correction.
Fix
Snapshot each affected review's
reviewer_idsontier.correction.item.original_reviewer_data(afields.Jsonmappingreview_id→ list ofuser_ids) atcorrect()time, and restore fromthat snapshot at
revert()time. Items written by older versions ofthe module have an empty snapshot and fall back to the previous
_get_reviewers()recompute, so existing in-flight corrections keepworking.
Tests
Two new tests in
test_tier_validation.py:test_revert_restores_snapshot_not_current_definition— repoints theunderlying definition between correct and revert; asserts the
originally-snapshotted user is restored, not the definition's new one.
test_revert_legacy_item_without_snapshot— clears the snapshot fieldto simulate a pre-fix item; asserts the fallback recompute still works.