[19.0][IMP] base_tier_validation: chatter warning at request_validation if reviewers lack access#32
Open
bosd wants to merge 4 commits into
Open
Conversation
…reviewers lack access Make stalled-by-misconfiguration tier validations visible at runtime instead of letting them silently freeze. After request_validation creates the tier.review rows for a record, check each reviewer against the validated model's ir.model.access. For any reviewer that fails the model-level read check: - a server-level WARNING is logged (definition author + reviewer login + model + record id, enough for an admin to debug from logs); - a chatter message is posted on the validated document, naming the affected reviewer(s) and explaining the workflow will stall until they get access or are replaced. This catches the cases the onchange warning at definition save can't: - review_type='field' (reviewer only resolves at validation time); - group membership changed after the definition was saved; - ir.rule-only restrictions that bypass model-level ACL. Model-level ACL only: per-record ir.rule may grant or revoke access at runtime, so false positives are possible. The chatter message says "may not be able to read" rather than asserting a guaranteed block. False positives are still preferable to silent failure. Tests cover both directions: chatter posted when reviewer lacks access, no chatter spam when reviewer has access.
Contributor
|
Hi @LoisRForgeFlow, |
OCA-git-bot
added
mod:base_tier_validation
added 2 commits
May 14, 2026 09:26
…ning - ruff B023: bind `rec` in the per-record filter lambda via `lambda r, rec=rec: ...` so the lint passes and the loop body is not relying on late binding of the cell variable. - Test setup: unlinking the tester model's only ir.model.access row also revoked access for the requester (test_user_1), making request_validation itself blow up before the helper had a chance to warn. Restrict the existing public ACL to `base.group_system` instead -- test_user_1 keeps access, test_user_2 (only base.group_user) loses it, which is the actual misconfiguration the chatter warning is meant to surface. Clear the ACL cache after the write so the helper observes the new state.
…o OCA log-scan passes
bosd
force-pushed
the
19.0-imp-base_tier_validation-warn-no-access-runtime
branch
2 times, most recently
from
b8379f1 to
cbe173e
Compare
bosd
force-pushed
the
19.0-imp-base_tier_validation-warn-no-access-runtime
branch
from
cbe173e to
b9ef4d9
Compare
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.
Summary
Make stalled-by-misconfiguration tier validations visible at runtime instead of letting them silently freeze.
After
request_validationcreates thetier.reviewrows for a record, the new_warn_reviewers_lacking_accesshelper checks each reviewer against the validated model'sir.model.access. For any reviewer that fails the model-level read check:WARNINGis logged (record name, id, reviewer logins) — enough for an admin to debug from logs;This catches the cases the onchange warning at definition save (#31) can't:
review_type='field'(reviewer only resolves at validation time);ir.rule-only restrictions that bypass model-level ACL.Reproduction
account.movewith the demo user as reviewer.WARNING.Tradeoff (worth calling out for review)
check_access('read')is model-level. Per-recordir.rulecan grant or revoke access dynamically:ir.rulewould actually allow access on this specific record → admin sees a noise message. Wording is "may not be able to" to soften this.ir.ruleblocks access on this specific record → still silently broken. Less surface area than today, but not a guarantee.Better than silent failure either way.
Test plan
Two tests:
test_request_validation_warns_reviewer_without_access: unlinks the tester model'sir.model.access, requests validation as test_user_2, asserts a chatter message naming the reviewer was posted.test_request_validation_no_warning_when_reviewer_has_access: same flow with the default public ACL in place, asserts no spurious chatter message.Companion PR
#31 covers the same problem at config time (onchange warning on
tier.definition). They're independent — review either order.