[19.0][FIX] base_tier_validation: don't promote later tiers out of sequence#53
Open
bosd wants to merge 1 commit into
Open
[19.0][FIX] base_tier_validation: don't promote later tiers out of sequence#53bosd wants to merge 1 commit into
bosd wants to merge 1 commit into
Conversation
``tier.review._update_review_status`` computed the "next sequence to promote"
with ``min(self.mapped("sequence"))`` -- over the recordset it was called on,
not the whole record. ``res.users.review_user_count`` calls it as
``user.review_ids._update_review_status()`` (one user's reviews). For a
second-tier reviewer that subset contains only their own (higher-sequence)
review, so it became the "minimum" and was promoted to ``pending`` before the
first tier was approved.
That single premature promotion cascaded:
- the review's ``can_review`` is then False (the lower, still-pending tier holds
the minimum sequence), so it is hidden from the reviewer's own systray;
- the ``notify_on_pending`` message is posted in -- and authored by -- that
reviewer's transaction, so they are excluded as the author and the alert is
delivered to the *previous* tier's reviewer instead;
- when the first tier is later approved, the review is already ``pending`` and
is skipped, so no correct notification ever fires.
Gate the sequence over the whole record's open reviews (per ``res_id``) instead
of the passed recordset. Add ``test_19c_no_premature_promotion_on_subset``
covering the subset path that ``test_19a`` (full-recordset) did not.
Contributor
|
Hi @LoisRForgeFlow, |
OCA-git-bot
added
mod:base_tier_validation
bosd
pushed a commit
to bosd/tier-validation
that referenced
this pull request
Jun 23, 2026
Backport of the upstream fix (OCA#53) onto this branch. _update_review_status gated promotion on min(self.mapped("sequence")) -- the minimum over the recordset it was called on, not the whole record. Since review_user_count runs it as user.review_ids._update_review_status() (one user's reviews), a second-tier reviewer's review became the "minimum" of that subset and was promoted to pending before the first tier was approved -- hiding it from their systray and mis-routing the notify_on_pending message to the previous reviewer. Gate on the whole record's open reviews instead. Adds test_19c_no_premature_promotion_on_subset. Bump 19.0.1.0.4 -> 19.0.1.0.5.
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
With
approve_sequence, a later-tier review could be promoted topendingbefore its predecessor is approved, which then:can_reviewis False while the lower tier still holds the minimum sequence), andnotify_on_pendingchatter message to the previous reviewer instead of the intended one.Root cause
tier.review._update_review_statusgates promotion onmin(self.mapped("sequence"))— the minimum over the recordset it happens to be called on, not the whole record.res.users.review_user_countcalls it asuser.review_ids._update_review_status(), i.e. on a single user's reviews. For a second-tier reviewer that subset contains only their own (higher-sequence) review, so it becomes the "minimum" and is promoted prematurely the moment they open their systray.The premature
pendingthen cascades:can_reviewevaluates False (the lower tier holds the min sequence) → hidden from the systray; thenotify_on_pendingmessage is posted in that reviewer's own transaction → they are excluded as its author and the alert lands on the previous reviewer; and when the first tier is finally approved the review is alreadypending, so it is skipped and no correct notification ever fires.Fix
Compute the gating sequence over the whole record's open reviews (per
res_id), not the passed recordset. No behaviour change for the normal full-recordset call (request_validation→_update_review_status).Test
Adds
test_19c_no_premature_promotion_on_subset, which runs_update_review_statuson only the later-tier reviewer's review (thereview_user_countpath) and asserts it stayswaitingand posts no message.test_19aonly exercised the full-recordset path, so the regression slipped through.