diff --git a/base_tier_validation/README.rst b/base_tier_validation/README.rst index 5c97d71c7..1ed2fd4f3 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/models/res_users.py b/base_tier_validation/models/res_users.py index c26e04ea7..6e31d5921 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,133 @@ 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) - ) - 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, + 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) ) - 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()) + + @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/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 8d3911153..06f43ab15 100644 --- a/base_tier_validation/static/description/index.html +++ b/base_tier_validation/static/description/index.html @@ -447,6 +447,22 @@

Configuration

  • 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. +
    2. Create or edit the base_tier_validation.late_after_days +parameter.
      3. +
    +

    Note:

    +

    To configure Tier Validation Exceptions, you need to:

    1. Go to Settings > Technical > Tier Validations > Tier Validation 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 553796b88..132435821 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,13 +21,32 @@ 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) { - 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; + // ``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() { @@ -39,10 +58,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 +84,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..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 @@ -42,7 +42,7 @@
      Review
      @@ -52,17 +52,39 @@ t-out="review.name" />
      + + Late + + Pending + + - Pending - + Future +
      +
      + Show all reviews +
      diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py index 3f2ab80cf..870ab5dcc 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,96 @@ 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_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})