From e23cde97a9d1c10bc8fa9dd0de2aa5dade0f66e5 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/2] [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 | 157 +++++++++++++----- .../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, 215 insertions(+), 56 deletions(-) diff --git a/base_tier_validation/models/res_users.py b/base_tier_validation/models/res_users.py index c26e04ea..98231be5 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,60 +23,123 @@ 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: - Model = self.env[model] - # Skip Models not having Tier Validation enabled (example: was unistalled) - if tier_review and hasattr(Model, "can_review"): - records_domain = ( - Domain("id", "in", tier_review.mapped("res_id")) - & Domain("validation_status", "!=", "rejected") - & Domain("can_review", "=", True) - ) - try: - records = ( - Model.with_user(user) - .with_context(active_test=False) - .search(records_domain) + for model, tier_reviews in review_groups: + Model = self.env.get(model) + if Model is None or not hasattr(Model, "can_review"): + # Tier-validation has been uninstalled for this model + # since the reviews were created. Skip silently. + 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 + ) + try: + # Resolve pending reviews to their underlying records, + # honouring ACL + record rules + the active flag. + pending_records = ( + Model.with_user(user) + .with_context(active_test=False) + .search( + Domain("id", "in", pending_reviews.mapped("res_id")) + & Domain("validation_status", "!=", "rejected") + & Domain("can_review", "=", True) ) - 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 - # Excludes any cancelled records depending on the structure of the model - if Model._state_field in Model._fields: - records = records.filtered( - lambda x: x[x._state_field] != x._cancel_state + ) + future_records = ( + Model.with_user(user) + .with_context(active_test=False) + .search( + Domain("id", "in", future_reviews.mapped("res_id")) + & Domain("validation_status", "!=", "rejected") ) - 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), - } + ) + except AccessError: + # Reviewer was assigned to a model they have no read + # access to. Drop it from the systray; the workflow is + # stuck for a config reason and #30 / #32 surface that + # elsewhere. + _logger.debug( + "User %s has no read access to %s; skipping in systray.", + user.login, + model, + ) + continue + # Filter out cancelled records the same way the model does + # when it has a state/cancel convention. + if Model._state_field in Model._fields: + pending_records = pending_records.filtered( + lambda x: x[x._state_field] != x._cancel_state + ) + future_records = future_records.filtered( + lambda x: x[x._state_field] != x._cancel_state + ) + late_res_ids = set(late_reviews.mapped("res_id")) + late_records = pending_records.filtered( + lambda x, late_res_ids=late_res_ids: 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 553796b8..1f0b67cf 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 @@ -24,7 +24,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; @@ -39,10 +42,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]]); } @@ -50,9 +68,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 f4b1c323..877acdd0 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 3f2ab80c..2afe82f2 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 @@ -126,6 +127,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 859ea6ce4a30b63ffb6ab3e3313f83f26ede166f 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/2] [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 98231be5..6e31d592 100644 --- a/base_tier_validation/models/res_users.py +++ b/base_tier_validation/models/res_users.py @@ -143,3 +143,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 1f0b67cf..13243582 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 @@ -21,7 +21,10 @@ export class TierReviewMenu extends Component { } async fetchSystrayReviewer() { - 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 @@ -31,6 +34,19 @@ 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; + } + + openDashboard() { + const action = this.store.tierReviewDashboardAction; + if (!action) { + return; + } + this.dropdown.close(); + this.action.doAction(action, {clearBreadcrumbs: true}); } availableViews() { 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 877acdd0..8d4fd961 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 2afe82f2..870ab5dc 100644 --- a/base_tier_validation/tests/test_tier_validation.py +++ b/base_tier_validation/tests/test_tier_validation.py @@ -185,6 +185,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})