[IMP] base_tier_validation: clearer, self-checking tier definitions

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()

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:

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):