[19.0][IMP] base_tier_validation: chatter warning at request_validation if reviewers lack access

base_tier_validation/models/tier_validation.py

@@ -2,18 +2,21 @@
 # Copyright 2024 Moduon Team (https://www.moduon.team)
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
+import logging
 from ast import literal_eval
 
 from lxml import etree
 from psycopg2.extensions import AsIs
 
 from odoo import api, fields, models
 from odoo.api import NewId
-from odoo.exceptions import ValidationError
+from odoo.exceptions import AccessError, ValidationError
 from odoo.fields import Domain
 from odoo.tools import SQL
 from odoo.tools.misc import frozendict
 
+_logger = logging.getLogger(__name__)
+
 BASE_EXCEPTION_FIELDS = [
     "message_follower_ids",
     "access_token",
@@ -795,11 +798,60 @@ def request_validation(self):
         # ``notify_on_create`` definition is present), reviews would stay in
         # ``waiting`` indefinitely and no reviewer would be notified.
         created_trs._update_review_status()
+        self._warn_reviewers_lacking_access(created_trs)
         if any(self.mapped("can_review")):
             self._update_counter({"review_created": True})
         self._notify_review_requested(created_trs)
         return created_trs
 
+    def _warn_reviewers_lacking_access(self, reviews):
+        """Post a chatter message + server warning if any of the reviewers
+        assigned to ``reviews`` cannot read the validated record.
+
+        Without this, a tier definition pointing at a model the reviewer
+        has no ``ir.model.access`` read on causes the workflow to stall
+        silently -- the review exists, but the reviewer can't see the
+        document and so can't act on it. The chatter message makes the
+        misconfiguration visible to the requester.
+
+        Model-level ACL only: per-record ``ir.rule`` restrictions are not
+        evaluated here, so false positives are possible. The message is
+        worded as a "may not be able to" warning rather than a guarantee.
+        """
+        Model = self.env[self._name]
+        for rec in self:
+            rec_reviews = reviews.filtered(lambda r, rec=rec: r.res_id == rec.id)
+            reviewers = rec_reviews.mapped("reviewer_ids")
+            if not reviewers:
+                continue
+            no_access = self.env["res.users"]
+            for user in reviewers:
+                try:
+                    Model.with_user(user).check_access("read")
+                except AccessError:
+                    no_access |= user
+            if not no_access:
+                continue
+            _logger.warning(
+                "Tier validation requested on %s(%s) but reviewer(s) %s "
+                "cannot read this model; the workflow will stall until "
+                "they get access or are replaced.",
+                rec._name,
+                rec.id,
+                no_access.mapped("login"),
+            )
+            if hasattr(rec, "message_post"):
+                rec.sudo().message_post(
+                    body=self.env._(
+                        "Tier validation was requested but the following "
+                        "reviewer(s) may not be able to read this document "
+                        "and so cannot act on it: %(reviewers)s. The "
+                        "workflow will remain pending until they get "
+                        "access or are replaced.",
+                        reviewers=", ".join(no_access.mapped("display_name")),
+                    )
+                )
+
     def _notify_restarted_review_body(self):
         return self.env._("The review has been reset by %s.", self.env.user.name)
 

base_tier_validation/tests/test_tier_validation.py

@@ -11,6 +11,7 @@
 from odoo.fields import Domain
 from odoo.tests import Form
 from odoo.tests.common import tagged
+from odoo.tools import mute_logger
 
 from ..models.tier_validation import BASE_EXCEPTION_FIELDS as BEF
 from ..models.tier_validation import TierValidation as TV
@@ -536,6 +537,120 @@ def test_16b_review_user_count_no_model_access(self):
         result = self.test_user_2.with_user(self.test_user_2).review_user_count()
         self.assertEqual(result, [])
 
+    @mute_logger(
+        "odoo.addons.base_tier_validation.models.tier_validation",
+        "odoo.addons.base.models.ir_model",
+    )
+    def test_request_validation_warns_reviewer_without_access(self):
+        """When request_validation creates reviews for a reviewer who has
+        no read access on the validated model, a chatter message is posted
+        on the document so the requester can see why the workflow won't
+        progress."""
+        test_record = self.test_model.create({"test_field": 2.5})
+        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)]",
+            }
+        )
+        # Restrict the tester model's read access to base.group_system so
+        # the requester (test_user_1, in group_system) keeps access while
+        # the reviewer (test_user_2, only base.group_user) loses it. This
+        # is the "assigned reviewer but no ACL" state the chatter warning
+        # exists to surface.
+        self.env["ir.model.access"].search(
+            Domain("model_id", "=", self.tester_model.id)
+        ).write({"group_id": self.env.ref("base.group_system").id})
+        # Ensure the ACL cache picks up the change before the helper checks.
+        self.env["ir.model.access"].call_cache_clearing_methods()
+        messages_before = len(test_record.message_ids)
+        test_record.with_user(self.test_user_1).request_validation()
+        messages_after = test_record.message_ids
+        self.assertGreater(len(messages_after), messages_before)
+        # The newest message should name the user without access.
+        body = messages_after[0].body
+        self.assertIn(self.test_user_2.display_name, body)
+        self.assertIn("may not be able to read", body)
+
+    def test_request_validation_no_warning_when_reviewer_has_access(self):
+        """No spurious chatter message when reviewers have model access."""
+        test_record = self.test_model.create({"test_field": 2.5})
+        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)]",
+            }
+        )
+        # Default public ACL is in place; test_user_1 can read the model.
+        messages_before = test_record.message_ids
+        test_record.with_user(self.test_user_2).request_validation()
+        new_messages = test_record.message_ids - messages_before
+        bodies = " ".join(message.body or "" for message in new_messages)
+        self.assertNotIn("may not be able to read", bodies)
+
+    def test_warn_reviewers_lacking_access_short_circuits(self):
+        """Direct exercise of the helper's defensive branches: no
+        reviewers and no access-issue early exits, without going through
+        the full ``request_validation`` flow."""
+        # No reviews -> empty for-loop, no chatter.
+        self.test_record._warn_reviewers_lacking_access(self.env["tier.review"])
+        # Real reviews but reviewer has access -> `if not no_access`
+        # exit, again no chatter.
+        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)]",
+            }
+        )
+        record = self.test_model.create({"test_field": 2.5})
+        reviews = record.with_user(self.test_user_2).request_validation()
+        messages_before = record.message_ids
+        record._warn_reviewers_lacking_access(reviews)
+        new_messages = record.message_ids - messages_before
+        bodies = " ".join(msg.body or "" for msg in new_messages)
+        self.assertNotIn("may not be able to read", bodies)
+
+    @mute_logger(
+        "odoo.addons.base_tier_validation.models.tier_validation",
+        "odoo.addons.base.models.ir_model",
+    )
+    def test_request_validation_warning_without_mail_thread(self):
+        """Models without mail.thread can't accept ``message_post``. The
+        warning helper must still log the issue but skip the chatter
+        post without raising. Covers the ``hasattr(rec, "message_post")``
+        False branch."""
+        self.tier_def_obj.create(
+            {
+                "model_id": self.tester_model_2.id,
+                "review_type": "individual",
+                "reviewer_id": self.test_user_2.id,
+                "definition_domain": "[('test_field', '>', 1.0)]",
+            }
+        )
+        # Restrict the model's ACL so the reviewer (user_2) loses read
+        # access and triggers the warning branch.
+        self.env["ir.model.access"].search(
+            Domain("model_id", "=", self.tester_model_2.id)
+        ).write({"group_id": self.env.ref("base.group_system").id})
+        self.env["ir.model.access"].call_cache_clearing_methods()
+        # Create a fresh record whose field actually matches the
+        # tier-definition domain (the shared fixture's test_field is 1.0
+        # which does not trigger the > 1.0 rule).
+        record = self.test_model_2.create({"test_field": 2.5})
+        # `tier.validation.tester2` does not inherit ``mail.thread``;
+        # the helper must not crash trying to post a chatter message
+        # on it.
+        reviews = record.with_user(self.test_user_1).request_validation()
+        self.assertTrue(reviews)
+        # Sanity: no chatter on a non-mail.thread model.
+        self.assertFalse(hasattr(record, "message_ids"))
+
     def test_17_search_records_no_validation(self):
         """Search for records that have no validation process started"""
         records = self.env["tier.validation.tester"].search(