[19.0][ADD] base_tier_validation: tier.definition.exclude_requester (four-eyes)#43
Open
bosd wants to merge 1 commit into
Open
[19.0][ADD] base_tier_validation: tier.definition.exclude_requester (four-eyes)#43bosd wants to merge 1 commit into
bosd wants to merge 1 commit into
Conversation
Contributor
|
Hi @LoisRForgeFlow, |
OCA-git-bot
added
mod:base_tier_validation
…r-eyes Add a per-definition Boolean ``exclude_requester`` that removes the requester from the tier's reviewer pool. Solves the common "four-eyes" pattern: anyone in this group can validate, except the user who requested the validation. Without this flag, when the requester happens to be a member of the configured reviewer group (or is the user returned by a reviewer field) they get included in their own review's ``reviewer_ids`` and can auto-validate -- defeating the purpose of asking for review. The current workaround requires custom Python; this PR makes it a single checkbox on the definition form. Implementation: - ``tier.definition.exclude_requester`` Boolean, default ``False`` (backwards compatible -- existing definitions and tests unchanged). - ``tier.review._get_reviewers`` post-filter subtracts ``requested_by`` when the definition opted in. Applies uniformly across ``review_type`` values (individual / group / field). - ``_get_reviewer_fields`` extended with the new dependency keys so the computed ``reviewer_ids`` recomputes when either ``definition_id.exclude_requester`` or ``requested_by`` changes. - Field exposed on the tier definition form view under the approve-sequence block. Tests cover both directions: with the flag set the requester is excluded and cannot ``can_review``; without it the legacy auto- include behaviour is preserved. Notes: - For ``review_type='individual'`` with the flag on, if the designated reviewer *is* the requester the pool ends up empty. That's intentional -- the workflow surfaces as blocked, signalling a misconfiguration. Combine the flag with ``group`` or ``field`` for meaningful four-eyes setups. - This is a per-definition gate. A per-user "courtesy reviewer" variant (where only some users in the group can act, others just see) remains future work; ``allow_reject`` was the first step in that direction.
bosd
force-pushed
the
19.0-imp-base_tier_validation-four-eyes
branch
from
20342cc to
0778937
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
Solve the much-asked "four-eyes principle" pattern (a co-worker needs to validate, but not me) with a single checkbox on the tier definition, no custom Python required.
When set, the user who requested the validation is removed from this tier's reviewer pool. Anyone else in the configured group can act; the requester cannot self-validate.
Why this is needed
Currently, when a requester is a member of the configured reviewer group, the auto-validation logic includes them in their own review's
reviewer_idsand lets them validate themselves. The only workaround is custom Python on every model. This PR puts the gate on the definition.How it works
tier.definition.exclude_requester(defaultFalse, backwards compatible).tier.review._get_reviewerspost-filters the computed pool by subtractingrequested_bywhen the definition opted in. Applies uniformly acrossreview_type(individual / group / field)._get_reviewer_fieldsextended so the computedreviewer_idsrecomputes when eitherdefinition_id.exclude_requesterorrequested_bychanges.Edge case worth knowing
For
review_type='individual'with the flag set, if the designated reviewer is the requester the pool ends up empty — the workflow surfaces as blocked. That's intentional and signals a misconfiguration. The flag is meaningful forgroup/fieldreview types.Tests
test_exclude_requester_drops_requester_from_group— flag on, requester is one of N group members. After request_validation,reviewer_idsexcludes them;can_reviewis False for the requester and True for the other group member.test_exclude_requester_off_keeps_legacy_behavior— flag off, requester remains inreviewer_idsand cancan_review. Backwards-compatibility check.Companion PR
This complements #42 (
allow_rejectfor sign-off tiers). Both shape per-tier reviewer behaviour via simple booleans ontier.definition. They can land in either order — no overlap.