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 +