From d92fc310fe45968e090b618a97d3de5e1e09ddba Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 14 May 2026 17:21:01 +0200 Subject: [PATCH 1/4] [IMP] base_tier_validation: late / pending / future systray buckets Mirror Odoo's activity systray layout in the tier-review dropdown so reviewers can see -- and click into -- three buckets per model instead of just one "Pending" column. - ``Late`` (red, hidden when 0 only via greyed style): pending reviews older than ``base_tier_validation.late_after_days`` system parameter (default 7). The reviewer should prioritise these to avoid blocking downstream work. - ``Pending``: pending reviews within the threshold. Standard "act today" work; what the systray showed before. - ``Future`` (only rendered when count > 0): waiting reviews where the current user is assigned as reviewer but the sequence has not yet promoted the review to pending. "What is incoming for me". The headline systray badge now reflects late + pending (the work the reviewer must act on now); future is a heads-up and is intentionally excluded so it doesn't inflate the urgency badge. Backend: ``res.users.review_user_count`` returns per-model dicts with ``late_count`` / ``pending_count`` / ``future_count`` plus the matching record id lists, so clicking a bucket in the systray opens exactly those records via a server-supplied ``[("id", "in", ids)]`` domain rather than re-running the lookup client-side. Frontend: the OWL ``tier_review_menu`` template renders the three columns in the order Late | Pending | Future. Clicking each calls ``openReviewGroup(group, bucket)`` which composes the action's domain off the bucket-specific id list and labels the breadcrumb accordingly. Test exercises both the "fresh" and "older than threshold" cases by ``freeze_time``-ing forward past the late cutoff and asserting the counts shift from ``pending_count`` into ``late_count`` while the sequenced waiting review stays in ``future_count``. --- base_tier_validation/models/res_users.py | 185 ++++++++++++------ .../tier_review_menu/tier_review_menu.esm.js | 30 ++- .../tier_review_menu/tier_review_menu.xml | 23 ++- .../tests/test_tier_validation.py | 61 +++++- 4 files changed, 225 insertions(+), 74 deletions(-) diff --git a/base_tier_validation/models/res_users.py b/base_tier_validation/models/res_users.py index 70748f985..393334061 100644 --- a/base_tier_validation/models/res_users.py +++ b/base_tier_validation/models/res_users.py @@ -9,6 +9,10 @@ _logger = logging.getLogger(__name__) +# Default for the ``base_tier_validation.late_after_days`` system parameter +# used by the systray to split pending reviews into a "Late" bucket. +DEFAULT_LATE_AFTER_DAYS = 7 + class Users(models.Model): _inherit = "res.users" @@ -19,79 +23,134 @@ class Users(models.Model): @api.model def review_user_count(self): + """Counts driving the tier-review systray dropdown. + + Each returned dict represents one model that has reviews touching + the current user and reports three buckets so the dropdown can + mirror Odoo's activity systray layout: + + - ``late_count`` -- pending reviews older than the configured + *late* threshold (``base_tier_validation.late_after_days``, + default ``7``). The reviewer should prioritise these. + - ``pending_count`` -- pending reviews within the threshold. + Standard work the reviewer can act on today. + - ``future_count`` -- waiting reviews where the current user is + assigned as reviewer but the sequence has not yet promoted the + review to pending. "What is incoming for me". + + The per-bucket record ids are returned alongside the counts so + clicking a bucket in the systray opens exactly those records. + """ user_reviews = {} user = self.env.user user.review_ids._update_review_status() - domain = ( - Domain("status", "=", "pending") - & Domain("can_review", "=", True) - & Domain("id", "in", user.review_ids.ids) + late_after_days = int( + self.env["ir.config_parameter"] + .sudo() + .get_param( + "base_tier_validation.late_after_days", + default=str(DEFAULT_LATE_AFTER_DAYS), + ) + ) + late_cutoff = fields.Datetime.subtract( + fields.Datetime.now(), days=late_after_days + ) + # Fetch every actionable review touching this user (pending + + # waiting) in one read_group so we only pay the SQL cost once. + domain = Domain("status", "in", ["pending", "waiting"]) & Domain( + "id", "in", user.review_ids.ids ) review_groups = self.env["tier.review"]._read_group( domain=domain, groupby=["model"], aggregates=["id:recordset"], ) - for model, tier_review in review_groups: + for model, tier_reviews in review_groups: # Skip Models not having Tier Validation enabled (example: was unistalled) - if not tier_review or model not in self.env: + if model not in self.env: continue Model = self.env[model] - if hasattr(Model, "can_review"): - records_domain = Domain( - "id", "in", tier_review.mapped("res_id") - ) & Domain("validation_status", "!=", "rejected") - # Excludes any cancelled records depending on the structure of - # the model. Let PostgreSQL do it whenever the state field is a - # real column -- which it is on every model that stores its - # state -- and keep the Python pass only as the fallback for - # models whose state field is computed and not stored. - state_field = Model._fields.get(Model._state_field) - state_in_db = state_field is not None and state_field.store - if state_in_db: - records_domain &= Domain( - Model._state_field, "!=", Model._cancel_state - ) - try: - records = ( - Model.with_user(user) - .with_context(active_test=False) - .search(records_domain) - ) - except AccessError: - # The user is a reviewer on records of a model whose - # ir.model.access does not grant them read access (e.g. - # a tier definition pointing at account.move while the - # reviewer has no accounting group). Skip silently so - # the systray keeps working; the reviewer cannot act on - # these reviews anyway without read access. - _logger.debug( - "User %s has no read access to %s; skipping in systray.", - user.login, - model, - ) - continue - # ``can_review`` is deliberately *not* part of ``records_domain``. - # Its search method re-searches the reviewer's whole backlog on - # this model and then evaluates the field in Python over all of - # it, so putting it in the domain made the systray cost grow - # with the backlog -- on every call, for every reviewer. The - # candidate set is already narrowed to the pending reviews found - # above, so evaluate the field on those records instead: same - # answer, one prefetched batch instead of a second search. - records = records.filtered("can_review") - if not state_in_db and Model._state_field in Model._fields: - records = records.filtered( - lambda x: x[x._state_field] != x._cancel_state - ) - if records: - user_reviews[model] = { - "id": records[0].id, - "name": Model._description, - "model": model, - "active_field": "active" in Model._fields, - "icon": modules.module.get_module_icon(Model._original_module), - "type": "tier_review", - "pending_count": len(records), - } + if not hasattr(Model, "can_review"): + continue + pending_reviews = tier_reviews.filtered( + lambda r: r.status == "pending" and r.can_review + ) + future_reviews = tier_reviews.filtered(lambda r: r.status == "waiting") + late_reviews = pending_reviews.filtered( + lambda r: r.create_date and r.create_date < late_cutoff + ) + records_domain = Domain( + "id", "in", tier_reviews.mapped("res_id") + ) & Domain("validation_status", "!=", "rejected") + # Excludes any cancelled records depending on the structure of + # the model. Let PostgreSQL do it whenever the state field is a + # real column -- which it is on every model that stores its + # state -- and keep the Python pass only as the fallback for + # models whose state field is computed and not stored. + state_field = Model._fields.get(Model._state_field) + state_in_db = state_field is not None and state_field.store + if state_in_db: + records_domain &= Domain(Model._state_field, "!=", Model._cancel_state) + try: + # Resolve pending and waiting reviews to their records in a + # single search, honouring ACL + record rules + the active + # flag. + records = ( + Model.with_user(user) + .with_context(active_test=False) + .search(records_domain) + ) + except AccessError: + # The user is a reviewer on records of a model whose + # ir.model.access does not grant them read access (e.g. + # a tier definition pointing at account.move while the + # reviewer has no accounting group). Skip silently so + # the systray keeps working; the reviewer cannot act on + # these reviews anyway without read access. + _logger.debug( + "User %s has no read access to %s; skipping in systray.", + user.login, + model, + ) + continue + if not state_in_db and Model._state_field in Model._fields: + records = records.filtered( + lambda x: x[x._state_field] != x._cancel_state + ) + pending_res_ids = set(pending_reviews.mapped("res_id")) + future_res_ids = set(future_reviews.mapped("res_id")) + late_res_ids = set(late_reviews.mapped("res_id")) + # ``can_review`` is deliberately *not* part of ``records_domain``. + # Its search method re-searches the reviewer's whole backlog on + # this model and then evaluates the field in Python over all of + # it, so putting it in the domain made the systray cost grow + # with the backlog -- on every call, for every reviewer. The + # candidate set is already narrowed to the reviews found above, + # so evaluate the field on those records instead: same answer, + # one prefetched batch instead of a second search. + pending_records = records.filtered( + lambda x: x.id in pending_res_ids and x.can_review + ) + future_records = records.filtered(lambda x: x.id in future_res_ids) + late_records = pending_records.filtered(lambda x: x.id in late_res_ids) + pending_ontime_records = pending_records - late_records + if not (late_records or pending_ontime_records or future_records): + continue + first_record = ( + late_records[:1] or pending_ontime_records[:1] or future_records[:1] + ) + user_reviews[model] = { + "id": first_record.id, + "name": Model._description, + "model": model, + "active_field": "active" in Model._fields, + "icon": modules.module.get_module_icon(Model._original_module), + "type": "tier_review", + "late_count": len(late_records), + "pending_count": len(pending_ontime_records), + "future_count": len(future_records), + "late_ids": late_records.ids, + "pending_ids": pending_ontime_records.ids, + "future_ids": future_records.ids, + } return list(user_reviews.values()) diff --git a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js index 40980ccd2..9635ece88 100644 --- a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js +++ b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js @@ -57,7 +57,10 @@ export class TierReviewMenu extends Component { const groups = await this.orm.call("res.users", "review_user_count"); let total = 0; for (const group of groups) { - total += group.pending_count || 0; + // Headline counter mirrors what the reviewer must act on + // *right now*: late + pending. Future is a heads-up only + // and shouldn't inflate the urgency badge. + total += (group.late_count || 0) + (group.pending_count || 0); } this.store.tierReviewCounter = total; this.store.tierReviewGroups = groups; @@ -79,10 +82,25 @@ export class TierReviewMenu extends Component { ]; } - openReviewGroup(group) { + openReviewGroup(group, bucket = "pending") { this.dropdown.close(); - const context = {}; - const domain = [["can_review", "=", true]]; + // Per-bucket ids are pre-computed server-side so we just hand + // them to the action's domain. Empty buckets are no-ops. + const idsByBucket = { + late: group.late_ids || [], + pending: group.pending_ids || [], + future: group.future_ids || [], + }; + const ids = idsByBucket[bucket] || []; + if (!ids.length) { + return; + } + const labelByBucket = { + late: "Late", + pending: "Pending", + future: "Future", + }; + const domain = [["id", "in", ids]]; if (group.active_field) { domain.push(["active", "in", [true, false]]); } @@ -90,9 +108,9 @@ export class TierReviewMenu extends Component { this.action.doAction( { - context, + context: {}, domain, - name: group.name, + name: `${group.name} - ${labelByBucket[bucket] || bucket}`, res_model: group.model, search_view_id: [false], type: "ir.actions.act_window", diff --git a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml index f4b1c323a..877acdd06 100644 --- a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml +++ b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml @@ -42,7 +42,7 @@
Review
@@ -52,12 +52,27 @@ t-out="review.name" />
+ + Late + + Pending + + - Pending - + Future +
diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py index b4428090e..7794ac6b8 100644 --- a/base_tier_validation/tests/test_tier_validation.py +++ b/base_tier_validation/tests/test_tier_validation.py @@ -4,9 +4,10 @@ from unittest import mock +from freezegun import freeze_time from lxml import etree -from odoo import Command +from odoo import Command, fields from odoo.exceptions import AccessError, ValidationError from odoo.fields import Domain from odoo.tests import Form @@ -194,6 +195,64 @@ def test_10_systray_counter(self): 1, ) + def test_systray_counter_late_pending_future_buckets(self): + """``review_user_count`` splits the user's reviews into three + buckets so the systray can mirror Odoo's activity menu: + + - ``late_count`` -- pending reviews older than the configured + ``base_tier_validation.late_after_days`` threshold; + - ``pending_count`` -- pending reviews within the threshold; + - ``future_count`` -- waiting reviews (sequence-queued). + + Also returns the matching record id lists so the systray click + opens exactly those records. + """ + # Two sequenced definitions for the same model: tier 1 = user 1, + # tier 2 = user 1 again. Once tier 1 is pending, tier 2 stays in + # "waiting" -- that's the future bucket from user 1's POV. + self.tier_def_obj.create( + { + "model_id": self.tester_model.id, + "review_type": "individual", + "reviewer_id": self.test_user_1.id, + "definition_domain": "[('test_field', '>', 1.0)]", + "sequence": 10, + "approve_sequence": True, + } + ) + self.tier_def_obj.create( + { + "model_id": self.tester_model.id, + "review_type": "individual", + "reviewer_id": self.test_user_1.id, + "definition_domain": "[('test_field', '>', 1.0)]", + "sequence": 20, + "approve_sequence": True, + } + ) + record = self.test_model.create({"test_field": 2.5}) + record.with_user(self.test_user_2).request_validation() + # Sanity: the user now has 1 pending review on this record and + # 1 waiting one queued behind it. + docs = self.test_user_1.with_user(self.test_user_1).review_user_count() + tester_doc = next(d for d in docs if d["model"] == "tier.validation.tester") + self.assertEqual(tester_doc["late_count"], 0) + self.assertEqual(tester_doc["pending_count"], 1) + self.assertEqual(tester_doc["future_count"], 1) + self.assertIn(record.id, tester_doc["pending_ids"]) + self.assertIn(record.id, tester_doc["future_ids"]) + # Push "now" past the late threshold (default 7 days) -- the + # still-pending review moves from `pending_count` into + # `late_count`. The waiting (future) review is unaffected. + later = fields.Datetime.add(fields.Datetime.now(), days=14) + with freeze_time(later): + docs = self.test_user_1.with_user(self.test_user_1).review_user_count() + tester_doc = next(d for d in docs if d["model"] == "tier.validation.tester") + self.assertEqual(tester_doc["late_count"], 1) + self.assertEqual(tester_doc["pending_count"], 0) + self.assertEqual(tester_doc["future_count"], 1) + self.assertIn(record.id, tester_doc["late_ids"]) + def test_11_add_comment(self): # Create new test record test_record = self.test_model.create({"test_field": 2.5}) From 055096501dc87e91479b3a65c5b0c704acabc53d Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 14 May 2026 17:26:05 +0200 Subject: [PATCH 2/4] [IMP] base_tier_validation: optional "Show all reviews" footer link in the systray Mirror Odoo's activity systray pattern where the dropdown ends with a "View all activities" link to the global activity dashboard. The tier-review dropdown gets the same footer when a downstream module exposes a global review dashboard. The hook is a new ``tier_review_dashboard_action`` method on ``res.users`` that returns ``False`` by default and can be overridden by a module shipping a dashboard (e.g. ``base_tier_validation_board``) to return a serialised ``ir.actions.act_window`` dict. The systray fetches the action alongside the bucket counts in parallel and renders the link conditionally on the response. No additional RPC round-trip cost beyond the single ``Promise.all``. When no dashboard is exposed (default install, no board module), the link is simply not rendered -- the dropdown behaves exactly as before. --- base_tier_validation/models/res_users.py | 10 ++++++ .../tier_review_menu/tier_review_menu.esm.js | 18 ++++++++++- .../tier_review_menu/tier_review_menu.xml | 9 +++++- .../tests/test_tier_validation.py | 32 +++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/base_tier_validation/models/res_users.py b/base_tier_validation/models/res_users.py index 393334061..5fa5be028 100644 --- a/base_tier_validation/models/res_users.py +++ b/base_tier_validation/models/res_users.py @@ -154,3 +154,13 @@ def review_user_count(self): "future_ids": future_records.ids, } return list(user_reviews.values()) + + @api.model + def tier_review_dashboard_action(self): + """Optional hook returning the action a downstream module wants the + systray's "Show all reviews" footer link to open. Default is + ``False`` so no link is rendered. ``base_tier_validation_board`` + (and any other dashboard module) overrides this to point at its + global review dashboard, matching the pattern of Odoo's + "View all activities" footer in the activity systray.""" + return False diff --git a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js index 9635ece88..fc544716e 100644 --- a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js +++ b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.esm.js @@ -54,7 +54,10 @@ export class TierReviewMenu extends Component { } this.fetchRunning = true; try { - const groups = await this.orm.call("res.users", "review_user_count"); + const [groups, dashboardAction] = await Promise.all([ + this.orm.call("res.users", "review_user_count"), + this.orm.call("res.users", "tier_review_dashboard_action"), + ]); let total = 0; for (const group of groups) { // Headline counter mirrors what the reviewer must act on @@ -64,6 +67,10 @@ export class TierReviewMenu extends Component { } this.store.tierReviewCounter = total; this.store.tierReviewGroups = groups; + // ``dashboardAction`` is False when no downstream module exposes + // a global review dashboard (default); a serialised action dict + // when one does (e.g. ``base_tier_validation_board``). + this.store.tierReviewDashboardAction = dashboardAction || null; } finally { this.fetchRunning = false; } @@ -73,6 +80,15 @@ export class TierReviewMenu extends Component { } } + openDashboard() { + const action = this.store.tierReviewDashboardAction; + if (!action) { + return; + } + this.dropdown.close(); + this.action.doAction(action, {clearBreadcrumbs: true}); + } + availableViews() { return [ [false, "kanban"], diff --git a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml index 877acdd06..8d4fd961a 100644 --- a/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml +++ b/base_tier_validation/static/src/components/tier_review_menu/tier_review_menu.xml @@ -68,7 +68,7 @@ Future @@ -78,6 +78,13 @@ +
+ Show all reviews +
diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py index 7794ac6b8..57590c46a 100644 --- a/base_tier_validation/tests/test_tier_validation.py +++ b/base_tier_validation/tests/test_tier_validation.py @@ -253,6 +253,38 @@ def test_systray_counter_late_pending_future_buckets(self): self.assertEqual(tester_doc["future_count"], 1) self.assertIn(record.id, tester_doc["late_ids"]) + def test_systray_counter_skips_when_no_acl(self): + """When the reviewer has no read access on the validated model, + ``review_user_count`` swallows the AccessError and drops the + model from the systray rather than crashing the dropdown.""" + self.tier_def_obj.create( + { + "model_id": self.tester_model.id, + "review_type": "individual", + "reviewer_id": self.test_user_2.id, + "definition_domain": "[('test_field', '>', 1.0)]", + } + ) + record = self.test_model.create({"test_field": 2.5}) + record.with_user(self.test_user_1).request_validation() + # Restrict the model's ACL to admins; test_user_2 keeps the + # tier review but loses read access on the validated record. + self.env["ir.model.access"].search( + Domain("model_id", "=", self.tester_model.id) + ).write({"group_id": self.env.ref("base.group_system").id}) + self.env["ir.model.access"].call_cache_clearing_methods() + # The endpoint must not raise and must drop the inaccessible + # model from its output. + docs = self.test_user_2.with_user(self.test_user_2).review_user_count() + self.assertNotIn("tier.validation.tester", [d["model"] for d in docs]) + + def test_tier_review_dashboard_action_stub(self): + """The base module exposes ``tier_review_dashboard_action`` as an + optional hook for downstream dashboard modules. Without an + override it returns ``False`` so the systray omits its + "Show all reviews" footer link.""" + self.assertFalse(self.env["res.users"].tier_review_dashboard_action()) + def test_11_add_comment(self): # Create new test record test_record = self.test_model.create({"test_field": 2.5}) From b063a12b448b4372aba495e4a5fd1cb092ca47ee Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 23 Jul 2026 08:58:40 +0200 Subject: [PATCH 3/4] [IMP] base_tier_validation: document late_after_days system parameter The systray late/pending/future buckets are driven by the base_tier_validation.late_after_days system parameter (default 7), but it was not mentioned in the readme. Document it under CONFIGURE so integrators know how to tune the Late threshold. --- base_tier_validation/README.rst | 16 ++++++++++++++++ base_tier_validation/readme/CONFIGURE.md | 15 +++++++++++++++ .../static/description/index.html | 18 +++++++++++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/base_tier_validation/README.rst b/base_tier_validation/README.rst index d53adf548..7ba69bdef 100644 --- a/base_tier_validation/README.rst +++ b/base_tier_validation/README.rst @@ -81,6 +81,22 @@ To configure this module, you need to: - If check *Approve by sequence*, reviewers is forced to review by specified sequence. +To tune when a pending review is flagged as *Late* in the reviewer +systray, you need to: + +1. Go to *Settings > Technical > Parameters > System Parameters*. +2. Create or edit the ``base_tier_validation.late_after_days`` + parameter. + +**Note:** + +- The tier-review systray splits your reviews into *Late*, *Pending* and + *Future* buckets. A pending review is counted as *Late* once it has + been waiting for more than ``base_tier_validation.late_after_days`` + days (default ``7``). Lower the value to be alerted sooner, raise it + to be more tolerant. The parameter is read on each systray refresh, so + a change takes effect immediately without restarting. + To configure Tier Validation Exceptions, you need to: 1. Go to *Settings > Technical > Tier Validations > Tier Validation diff --git a/base_tier_validation/readme/CONFIGURE.md b/base_tier_validation/readme/CONFIGURE.md index 4bd817e66..66f4c57db 100644 --- a/base_tier_validation/readme/CONFIGURE.md +++ b/base_tier_validation/readme/CONFIGURE.md @@ -16,6 +16,21 @@ To configure this module, you need to: - If check *Approve by sequence*, reviewers is forced to review by specified sequence. +To tune when a pending review is flagged as *Late* in the reviewer +systray, you need to: + +1. Go to *Settings \> Technical \> Parameters \> System Parameters*. +2. Create or edit the `base_tier_validation.late_after_days` parameter. + +**Note:** + +- The tier-review systray splits your reviews into *Late*, *Pending* + and *Future* buckets. A pending review is counted as *Late* once it + has been waiting for more than `base_tier_validation.late_after_days` + days (default `7`). Lower the value to be alerted sooner, raise it to + be more tolerant. The parameter is read on each systray refresh, so a + change takes effect immediately without restarting. + To configure Tier Validation Exceptions, you need to: 1. Go to *Settings > Technical > Tier Validations > Tier Validation Exceptions*. diff --git a/base_tier_validation/static/description/index.html b/base_tier_validation/static/description/index.html index 3a0625744..9764b1106 100644 --- a/base_tier_validation/static/description/index.html +++ b/base_tier_validation/static/description/index.html @@ -3,7 +3,7 @@ -Base Tier Validation +README.rst