From 0778937c2c75a554e0cf9397c4a5afecff44897b Mon Sep 17 00:00:00 2001
From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me>
Date: Thu, 14 May 2026 18:03:30 +0200
Subject: [PATCH] [ADD] base_tier_validation: tier.definition.exclude_requester
for four-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.
---
.../models/tier_definition.py | 11 +++
base_tier_validation/models/tier_review.py | 24 +++++--
.../tests/test_tier_validation.py | 68 +++++++++++++++++++
.../views/tier_definition_view.xml | 4 ++
4 files changed, 101 insertions(+), 6 deletions(-)
diff --git a/base_tier_validation/models/tier_definition.py b/base_tier_validation/models/tier_definition.py
index b36a82f6..1eb07a86 100644
--- a/base_tier_validation/models/tier_definition.py
+++ b/base_tier_validation/models/tier_definition.py
@@ -109,6 +109,17 @@ def _get_tier_validation_model_names(self):
help="Bypassed (auto validated), if previous tier was validated "
"by same reviewer",
)
+ exclude_requester = fields.Boolean(
+ string="Four-eyes Principle",
+ default=False,
+ help="When set, the user who requested the validation is removed "
+ "from this tier's reviewer pool. Anyone else in the configured "
+ "group can validate -- except the requester themselves. Without "
+ "this flag, a requester who happens to be in the same group as "
+ "the configured reviewers can auto-validate their own request. "
+ "Only meaningful for review_type='group'; the form hides this "
+ "field for the other review types.",
+ )
@api.onchange("review_type")
def onchange_review_type(self):
diff --git a/base_tier_validation/models/tier_review.py b/base_tier_validation/models/tier_review.py
index bed4caaf..36b0b68d 100644
--- a/base_tier_validation/models/tier_review.py
+++ b/base_tier_validation/models/tier_review.py
@@ -131,7 +131,13 @@ def _can_review_value(self):
@api.model
def _get_reviewer_fields(self):
- return ["reviewer_id", "reviewer_group_id", "reviewer_group_id.user_ids"]
+ return [
+ "reviewer_id",
+ "reviewer_group_id",
+ "reviewer_group_id.user_ids",
+ "definition_id.exclude_requester",
+ "requested_by",
+ ]
@api.depends(lambda self: self._get_reviewer_fields())
def _compute_reviewer_ids(self):
@@ -154,16 +160,17 @@ def _compute_todo_by(self):
rec.todo_by = todo_by
def _get_reviewers(self):
+ reviewers = self.env["res.users"]
if self.reviewer_id or self.reviewer_group_id.user_ids:
- return self.reviewer_id + self.reviewer_group_id.user_ids
- if self.reviewer_field_id:
+ reviewers = self.reviewer_id + self.reviewer_group_id.user_ids
+ elif self.reviewer_field_id:
resource = self.env[self.model].browse(self.res_id)
reviewer_field = getattr(resource, self.reviewer_field_id.name, False)
if reviewer_field:
if reviewer_field._name == "res.groups":
- return reviewer_field.user_ids
+ reviewers = reviewer_field.user_ids
elif reviewer_field._name == "res.users":
- return reviewer_field
+ reviewers = reviewer_field
else:
raise ValidationError(
self.env._(
@@ -171,7 +178,12 @@ def _get_reviewers(self):
"should be of the appropriate type"
)
)
- return self.env["res.users"]
+ # Four-eyes: drop the requester from the reviewer pool if the
+ # definition opted in. Done as a post-filter so it applies
+ # uniformly across individual / group / field review types.
+ if self.definition_id.exclude_requester and self.requested_by:
+ reviewers -= self.requested_by
+ return reviewers
def _notify_pending_status(self, review_ids):
"""Method to call and reuse abstract notification method"""
diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py
index 344004fd..adc8ee55 100644
--- a/base_tier_validation/tests/test_tier_validation.py
+++ b/base_tier_validation/tests/test_tier_validation.py
@@ -40,6 +40,74 @@ def test_03_request_validation_approved(self):
record.validate_tier()
self.assertEqual(record.validation_status, "validated")
+ def test_exclude_requester_drops_requester_from_group(self):
+ """With ``exclude_requester=True`` on a group review type, the
+ requester is removed from the tier's reviewer pool so they
+ cannot auto-validate their own request -- enforcing a
+ four-eyes principle without custom Python."""
+ # Build a group containing both users and a tier definition
+ # that points at it. test_user_1 will be the requester; the
+ # group also includes test_user_1, so without the four-eyes
+ # flag they'd be in their own review's reviewer_ids.
+ group = self.env["res.groups"].create(
+ {
+ "name": "Four-eyes Reviewers",
+ "user_ids": [
+ Command.link(self.test_user_1.id),
+ Command.link(self.test_user_2.id),
+ ],
+ }
+ )
+ # Use the existing definition so we don't get extra reviews on
+ # tier_definition_1's individual reviewer.
+ self.tier_definition.write(
+ {
+ "review_type": "group",
+ "reviewer_id": False,
+ "reviewer_group_id": group.id,
+ "exclude_requester": True,
+ }
+ )
+ record = self.test_model.create({"test_field": 1.0})
+ record.with_user(self.test_user_1).request_validation()
+ review = record.review_ids.filtered(
+ lambda r: r.definition_id == self.tier_definition
+ )
+ # The requester (test_user_1) is excluded; only test_user_2
+ # remains as a valid reviewer.
+ self.assertIn(self.test_user_2, review.reviewer_ids)
+ self.assertNotIn(self.test_user_1, review.reviewer_ids)
+ # The requester therefore cannot self-validate.
+ self.assertFalse(record.with_user(self.test_user_1).can_review)
+ self.assertTrue(record.with_user(self.test_user_2).can_review)
+
+ def test_exclude_requester_off_keeps_legacy_behavior(self):
+ """Without the flag, a requester who happens to be a member of
+ the configured reviewer group is included in reviewer_ids and
+ can auto-validate -- which is the legacy behaviour and must
+ keep working for backwards compatibility."""
+ group = self.env["res.groups"].create(
+ {
+ "name": "Open Reviewers",
+ "user_ids": [Command.link(self.test_user_1.id)],
+ }
+ )
+ self.tier_definition.write(
+ {
+ "review_type": "group",
+ "reviewer_id": False,
+ "reviewer_group_id": group.id,
+ # exclude_requester left at its default of False
+ }
+ )
+ record = self.test_model.create({"test_field": 1.0})
+ record.with_user(self.test_user_1).request_validation()
+ review = record.review_ids.filtered(
+ lambda r: r.definition_id == self.tier_definition
+ )
+ self.assertIn(self.test_user_1, review.reviewer_ids)
+ self.assertTrue(record.with_user(self.test_user_1).can_review)
+
def test_04_request_validation_rejected(self):
"""Request validation, rejection and reset."""
self.assertFalse(self.test_record.review_ids)
diff --git a/base_tier_validation/views/tier_definition_view.xml b/base_tier_validation/views/tier_definition_view.xml
index 922727ed..ff882142 100644
--- a/base_tier_validation/views/tier_definition_view.xml
+++ b/base_tier_validation/views/tier_definition_view.xml
@@ -91,6 +91,10 @@
Bypass, if previous tier was validated by same reviewer
+