From 6b431b2b05274e2687f275747f55801e9744f7ef Mon Sep 17 00:00:00 2001
From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me>
Date: Tue, 19 May 2026 21:42:04 +0200
Subject: [PATCH] [IMP] base_tier_validation: clearer, self-checking tier
definitions
Authoring a tier definition is hard for non-technical users: a raw
domain, technical model/field names, and opaque sequence/approve knobs
with no indication of how rules combine -- and the only way to verify a
rule is the full live request/approve loop.
- Add a computed plain-language Summary (form banner + list column)
describing in one sentence which documents the rule covers and who
must validate it.
- Add inline help to the model, filter, reviewer-field, sequence and
approval fields, with a concrete domain example.
- Open the Tier Definition list grouped by document type in sequence
order, so the full ordered tier chain per model is visible at a
glance (and add _order = "sequence, id").
- Block a malformed filter at save time with a clear error (the engine
already literal_eval's it at request time), and warn -- without
blocking -- when a rule has no filter or no reviewer set yet. The
reviewer check is an onchange, not a constraint, so programmatic /
bridge-module creation of not-yet-complete definitions still works.
---
.../models/tier_definition.py | 145 +++++++++++++++++-
base_tier_validation/readme/HISTORY.md | 20 +++
.../tests/test_tier_validation.py | 121 +++++++++++++++
.../views/tier_definition_view.xml | 19 ++-
4 files changed, 299 insertions(+), 6 deletions(-)
diff --git a/base_tier_validation/models/tier_definition.py b/base_tier_validation/models/tier_definition.py
index b36a82f6..2caf73ff 100644
--- a/base_tier_validation/models/tier_definition.py
+++ b/base_tier_validation/models/tier_definition.py
@@ -1,13 +1,17 @@
# Copyright 2017 ForgeFlow S.L. (https://www.forgeflow.com)
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
+from ast import literal_eval
+
from odoo import api, fields, models
+from odoo.exceptions import ValidationError
from odoo.fields import Domain
class TierDefinition(models.Model):
_name = "tier.definition"
_description = "Tier Definition"
+ _order = "company_id, sequence, id"
@api.model
def _get_default_name(self):
@@ -24,10 +28,18 @@ def _get_tier_validation_model_names(self):
default=lambda self: self._get_default_name(),
translate=True,
)
+ tier_summary = fields.Char(
+ string="Summary",
+ compute="_compute_tier_summary",
+ help="Plain-language description of when this rule applies and who "
+ "must validate.",
+ )
model_id = fields.Many2one(
comodel_name="ir.model",
string="Referenced Model",
domain=lambda self: [("model", "in", self._get_tier_validation_model_names())],
+ help="The document type this validation rule applies to. Only models "
+ "that opted in to tier validation are listed.",
)
model = fields.Char(related="model_id.model", index=True, store=True)
review_type = fields.Selection(
@@ -38,6 +50,11 @@ def _get_tier_validation_model_names(self):
("group", "Any user in a specific group"),
("field", "Field in related record"),
],
+ help="Who is allowed to validate this tier:\n"
+ "- Specific user: a named user.\n"
+ "- Any user in a specific group: any member of the chosen group.\n"
+ "- Field in related record: the user/group stored in a field of the "
+ "document itself (e.g. its salesperson or manager).",
)
allow_write_for_reviewer = fields.Boolean(
string="Allow Write For Reviewers",
@@ -51,6 +68,9 @@ def _get_tier_validation_model_names(self):
comodel_name="ir.model.fields",
string="Reviewer field",
domain="[('id', 'in', valid_reviewer_field_ids)]",
+ help="A field of the document that points to the reviewer(s): a "
+ "user field or a group field (e.g. the document's salesperson or "
+ "approving manager).",
)
valid_reviewer_field_ids = fields.One2many(
comodel_name="ir.model.fields",
@@ -59,9 +79,18 @@ def _get_tier_validation_model_names(self):
definition_type = fields.Selection(
string="Definition", selection=[("domain", "Domain")], default="domain"
)
- definition_domain = fields.Char()
+ definition_domain = fields.Char(
+ help="Filter deciding which records of the model this rule applies "
+ "to. Leave empty to apply to every record. Example: "
+ "[('amount_total', '>', 1000)] -- only documents whose total "
+ "exceeds 1000.",
+ )
active = fields.Boolean(default=True)
- sequence = fields.Integer(default=30)
+ sequence = fields.Integer(
+ default=30,
+ help="Order of this tier. Lower numbers are validated first when "
+ "'Approve by sequence' is enabled.",
+ )
company_id = fields.Many2one(
comodel_name="res.company",
string="Company",
@@ -103,7 +132,9 @@ def _get_tier_validation_model_names(self):
approve_sequence = fields.Boolean(
string="Approve by sequence",
default=False,
- help="Approval order by the specified sequence number",
+ help="If set, this tier can only be validated once all tiers with a "
+ "lower sequence number have been validated (strict order). If not "
+ "set, this tier can be validated at any time.",
)
approve_sequence_bypass = fields.Boolean(
help="Bypassed (auto validated), if previous tier was validated "
@@ -135,6 +166,114 @@ def _compute_domain_reviewer_field(self):
rec.model, IrModelFields
)
+ def _describe_domain(self):
+ """Best-effort plain-language rendering of ``definition_domain``.
+
+ Never raises: falls back to the raw string when the domain cannot be
+ parsed or contains operators we do not spell out.
+ """
+ self.ensure_one()
+ raw = (self.definition_domain or "").strip()
+ if not raw or raw == "[]":
+ return self.env._("for all records")
+ try:
+ parsed = literal_eval(raw)
+ except (ValueError, SyntaxError):
+ return self.env._("where %s", raw)
+ if not isinstance(parsed, list | tuple) or not parsed:
+ return self.env._("for all records")
+ model = self.env[self.model] if self.model and self.model in self.env else None
+ leaves = []
+ for item in parsed:
+ if isinstance(item, list | tuple) and len(item) == 3:
+ fname, operator, value = item
+ label = fname
+ if model is not None and fname in model._fields:
+ label = model._fields[fname].string or fname
+ leaves.append(f"{label} {operator} {value!r}")
+ if not leaves:
+ return self.env._("where %s", raw)
+ return self.env._("where %s", " and ".join(leaves))
+
+ def _describe_reviewer(self):
+ self.ensure_one()
+ by_type = {
+ "individual": self.reviewer_id.display_name,
+ "group": self.reviewer_group_id.display_name,
+ "field": self.reviewer_field_id.field_description
+ or self.reviewer_field_id.name,
+ }
+ return by_type.get(self.review_type) or ""
+
+ @api.depends(
+ "model_id",
+ "model",
+ "sequence",
+ "approve_sequence",
+ "review_type",
+ "reviewer_id",
+ "reviewer_group_id",
+ "reviewer_field_id",
+ "definition_domain",
+ )
+ def _compute_tier_summary(self):
+ rt_labels = dict(self.fields_get(["review_type"])["review_type"]["selection"])
+ for rec in self:
+ if not rec.model_id:
+ rec.tier_summary = ""
+ continue
+ target = rec._describe_reviewer()
+ rec.tier_summary = self.env._(
+ "Tier %(seq)s — %(model)s %(cond)s: validated by %(how)s"
+ "%(target)s%(ordered)s",
+ seq=rec.sequence,
+ model=rec.model_id.name or rec.model,
+ cond=rec._describe_domain(),
+ how=rt_labels.get(rec.review_type, rec.review_type or ""),
+ target=(" " + target) if target else "",
+ ordered=self.env._(" (in sequence)") if rec.approve_sequence else "",
+ )
+
+ @api.constrains("definition_domain")
+ def _check_definition_domain(self):
+ for rec in self:
+ raw = (rec.definition_domain or "").strip()
+ if not raw:
+ continue
+ try:
+ parsed = literal_eval(raw)
+ except (ValueError, SyntaxError) as err:
+ raise ValidationError(
+ self.env._(
+ "The filter of tier definition '%(name)s' is not "
+ "valid: %(err)s",
+ name=rec.name,
+ err=err,
+ )
+ ) from err
+ if not isinstance(parsed, list | tuple):
+ raise ValidationError(
+ self.env._(
+ "The filter of tier definition '%(name)s' must be a "
+ "list of conditions, e.g. "
+ "[('amount_total', '>', 1000)].",
+ name=rec.name,
+ )
+ )
+
+ @api.onchange("definition_domain")
+ def _onchange_definition_domain_warn_empty(self):
+ if not (self.definition_domain or "").strip() and self.model_id:
+ return {
+ "warning": {
+ "title": self.env._("Heads up"),
+ "message": self.env._(
+ "This rule has no filter, so it will apply to every %s.",
+ self.model_id.name or self.model,
+ ),
+ }
+ }
+
def _get_review_needing_reminder(self):
"""Return all the reviews that have the reminder setup."""
self.ensure_one()
diff --git a/base_tier_validation/readme/HISTORY.md b/base_tier_validation/readme/HISTORY.md
index 740cd6ca..f82ba1cd 100644
--- a/base_tier_validation/readme/HISTORY.md
+++ b/base_tier_validation/readme/HISTORY.md
@@ -1,3 +1,23 @@
+## 19.0.2.0.0 (2026-05-19)
+
+Improvements:
+
+- Tier definitions now show a plain-language **Summary** (on the form
+ and in the list) describing in one sentence which documents the rule
+ applies to and who must validate.
+- Added inline help to the model, filter, reviewer-field, sequence and
+ approval fields, with a concrete domain example.
+- The Tier Definition list now opens grouped by document type, in
+ sequence order, so the full ordered tier chain per model is visible
+ at a glance. In multi-company databases definitions are ordered by
+ company first (and the Company column is shown by default) so each
+ company's tiers stay together instead of being interleaved.
+- A malformed filter (domain) is now reported with a clear error when
+ the definition is saved, instead of only failing later when
+ validation is requested. The reviewer (user, group or field) is now
+ required in the form once a "Validated by" type is chosen, and the
+ form warns when a rule has no filter.
+
## 19.0.1.0.1 (2026-05-12)
Fixes:
diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py
index 344004fd..657d6fac 100644
--- a/base_tier_validation/tests/test_tier_validation.py
+++ b/base_tier_validation/tests/test_tier_validation.py
@@ -1454,6 +1454,127 @@ def test_34_test_duplicate_new_user_should_not_have_review_ids(self):
# Review_ids should not be copied when duplicating a user
self.assertFalse(new_user.review_ids.ids)
+ def test_36_tier_summary_is_human_readable(self):
+ """The computed summary names the document, the condition and the
+ reviewer in plain language."""
+ summary = self.definition_1.tier_summary
+ self.assertTrue(summary)
+ self.assertIn(self.definition_1.model_id.name, summary)
+ self.assertIn(self.test_user_1.display_name, summary)
+ self.assertIn("Tier", summary)
+ # The condition is rendered with the human field label, not the
+ # technical name -- that is the whole point of the summary.
+ test_field_label = self.test_model._fields["test_field"].string
+ self.assertIn(test_field_label, summary)
+
+ def test_37_invalid_definition_domain_is_blocked(self):
+ """A malformed domain is rejected at save time (not only later at
+ request_validation)."""
+ with self.assertRaises(ValidationError):
+ self.tier_def_obj.create(
+ {
+ "model_id": self.tester_model.id,
+ "review_type": "individual",
+ "reviewer_id": self.test_user_1.id,
+ "definition_domain": "this is not a domain",
+ }
+ )
+
+ def test_38_empty_domain_onchange_warns(self):
+ definition = self.tier_def_obj.new(
+ {"model_id": self.tester_model.id, "definition_domain": ""}
+ )
+ res = definition._onchange_definition_domain_warn_empty()
+ self.assertIn("warning", res)
+
+ def test_39_form_requires_reviewer_for_its_type(self):
+ """The matching reviewer is required in the form when a review type
+ is chosen, but plain create() stays unconstrained so bridge
+ modules can still create a definition and set the reviewer later."""
+ # Form save without the reviewer for the chosen type must fail.
+ form = Form(self.tier_def_obj)
+ form.model_id = self.tester_model
+ form.review_type = "individual"
+ with self.assertRaises(AssertionError):
+ form.save()
+ # Programmatic create stays allowed (no reviewer yet).
+ definition = self.tier_def_obj.create(
+ {"model_id": self.tester_model.id, "review_type": "individual"}
+ )
+ self.assertTrue(definition.id)
+
+ def test_40_summary_helpers_cover_all_branches(self):
+ """Exercise every branch of the summary / lint helpers."""
+ # _describe_domain: every rendering path (use new() so the domain
+ # constraint does not block the intentionally odd values).
+ cases = {
+ "": "all records",
+ "[]": "all records",
+ "garbage": "where",
+ "1": "all records",
+ "['&']": "where",
+ "[('test_field', '>', 1.0)]": "Test Field",
+ "[('unknown_field', '=', 1)]": "unknown_field",
+ }
+ for raw, expected in cases.items():
+ definition = self.tier_def_obj.new(
+ {"model_id": self.tester_model.id, "definition_domain": raw}
+ )
+ self.assertIn(expected, definition._describe_domain())
+
+ # _describe_reviewer + summary for the group review type.
+ group_def = self.tier_def_obj.create(
+ {
+ "model_id": self.tester_model.id,
+ "review_type": "group",
+ "reviewer_group_id": self.test_group.id,
+ "definition_domain": "[]",
+ }
+ )
+ self.assertIn(self.test_group.display_name, group_def.tier_summary)
+
+ # _describe_reviewer + summary for the field review type.
+ user_field = self.env["ir.model.fields"].search(
+ [("model", "=", "tier.validation.tester"), ("name", "=", "user_id")],
+ limit=1,
+ )
+ field_def = self.tier_def_obj.create(
+ {
+ "model_id": self.tester_model.id,
+ "review_type": "field",
+ "reviewer_field_id": user_field.id,
+ "definition_domain": "[]",
+ }
+ )
+ self.assertIn(
+ user_field.field_description or user_field.name,
+ field_def.tier_summary,
+ )
+
+ # _compute_tier_summary: definition without a model -> empty summary.
+ no_model = self.tier_def_obj.create({"name": "No model rule"})
+ self.assertEqual(no_model.tier_summary, "")
+
+ # _check_definition_domain: a parseable but non-list value is rejected.
+ with self.assertRaises(ValidationError):
+ self.tier_def_obj.create(
+ {
+ "model_id": self.tester_model.id,
+ "review_type": "individual",
+ "reviewer_id": self.test_user_1.id,
+ "definition_domain": "1",
+ }
+ )
+
+ # Onchange "no warning" path for a non-empty domain.
+ ok_domain = self.tier_def_obj.new(
+ {
+ "model_id": self.tester_model.id,
+ "definition_domain": "[('test_field', '>', 1.0)]",
+ }
+ )
+ self.assertFalse(ok_domain._onchange_definition_domain_warn_empty())
+
@tagged("at_install")
class TierTierValidationView(CommonTierValidation):
diff --git a/base_tier_validation/views/tier_definition_view.xml b/base_tier_validation/views/tier_definition_view.xml
index 922727ed..e3cc4581 100644
--- a/base_tier_validation/views/tier_definition_view.xml
+++ b/base_tier_validation/views/tier_definition_view.xml
@@ -10,6 +10,7 @@
+
@@ -20,7 +21,7 @@
@@ -49,6 +50,13 @@
/>
+
+
+
@@ -58,14 +66,17 @@
@@ -142,7 +153,7 @@
@@ -155,7 +166,9 @@
ir.actions.act_window
tier.definition
list,form
- {'search_default_all': 1}
+ {'search_default_all': 1, 'search_default_group_by_model': 1}