[19.0][FIX] base_tier_validation: recompute can_review when a sibling review changes status

base_tier_validation/__manifest__.py

@@ -3,7 +3,7 @@
 {
     "name": "Base Tier Validation",
     "summary": "Implement a validation process based on tiers.",
-    "version": "19.0.1.0.3",
+    "version": "19.0.1.0.4",
     "development_status": "Mature",
     "maintainers": ["LoisRForgeFlow"],
     "category": "Tools",

base_tier_validation/models/tier_review.py

@@ -129,6 +129,29 @@ def _can_review_value(self):
         sequence = min(reviews.mapped("sequence"))
         return self.sequence == sequence
 
+    def write(self, vals):
+        res = super().write(vals)
+        if "status" in vals:
+            # ``can_review`` is stored, but ``_can_review_value`` reads the
+            # *other* reviews of the same record (it checks which review holds
+            # the lowest pending sequence). That cross-review dependency cannot
+            # be expressed with ``@api.depends`` over the generic
+            # ``(model, res_id)`` reference, so a status change here would
+            # otherwise leave the siblings' ``can_review`` stale -- e.g. a later
+            # tier stays hidden in the reviewer's systray after its predecessor
+            # is approved. Mark the siblings for recomputation explicitly.
+            can_review = self._fields["can_review"]
+            Review = self.env["tier.review"].sudo()
+            for model in set(self.mapped("model")):
+                res_ids = self.filtered(
+                    lambda r, m=model: r.model == m
+                ).mapped("res_id")
+                siblings = Review.search(
+                    [("model", "=", model), ("res_id", "in", res_ids)]
+                )
+                self.env.add_to_compute(can_review, siblings)
+        return res
+
     @api.model
     def _get_reviewer_fields(self):
         return ["reviewer_id", "reviewer_group_id", "reviewer_group_id.user_ids"]

base_tier_validation/tests/test_tier_validation.py

@@ -718,6 +718,62 @@ def test_19b_notify_review_available_no_op_when_no_users(self):
         self.assertEqual(test_record.message_follower_ids, followers_before)
         self.assertEqual(test_record.message_ids, messages_before)
 
+    def test_19d_can_review_recomputes_on_sibling_status_change(self):
+        """``can_review`` is stored but its value reads sibling reviews' status
+        (which review holds the lowest pending sequence). A change to one
+        review's status must therefore recompute its siblings' ``can_review``;
+        otherwise a later tier stays ``can_review=False`` -- hidden from its
+        reviewer's systray -- even after its predecessor is approved.
+        """
+        TierDefinition = self.env["tier.definition"]
+        test_record = self.test_model.create({"test_field": 2.5})
+        def_first = TierDefinition.create(
+            {
+                "model_id": self.tester_model.id,
+                "review_type": "individual",
+                "reviewer_id": self.test_user_1.id,
+                "definition_domain": "[('test_field', '=', 2.5)]",
+                "approve_sequence": True,
+                "sequence": 20,
+                "name": "First in sequence -- user 1",
+            }
+        )
+        def_second = TierDefinition.create(
+            {
+                "model_id": self.tester_model.id,
+                "review_type": "individual",
+                "reviewer_id": self.test_user_2.id,
+                "definition_domain": "[('test_field', '=', 2.5)]",
+                "approve_sequence": True,
+                "sequence": 10,
+                "name": "Second in sequence -- user 2",
+            }
+        )
+        reviews = test_record.request_validation()
+        review_first = reviews.filtered(lambda r: r.definition_id == def_first)
+        review_second = reviews.filtered(lambda r: r.definition_id == def_second)
+
+        # Put the later review into ``pending`` while its predecessor is still
+        # pending (the stale state this guards against). Its ``can_review`` is
+        # correctly False -- it is not the lowest pending sequence.
+        review_second.status = "pending"
+        review_second.flush_recordset()
+        review_second.invalidate_recordset(["can_review"])
+        self.assertFalse(review_second.can_review)
+
+        # Approving the first review changes a *sibling's* status. The second
+        # review's stored ``can_review`` must be recomputed to True even though
+        # its own fields did not change.
+        test_record.with_user(self.test_user_1).validate_tier()
+        review_first.flush_recordset()
+        review_second.invalidate_recordset(["can_review"])
+        self.assertEqual(review_first.status, "approved")
+        self.assertTrue(
+            review_second.can_review,
+            "can_review must be recomputed when a sibling review's status "
+            "changes (a stored field whose value depends on sibling reviews).",
+        )
+
     def test_20_no_sequence(self):
         # Create new test record
         tier_review_obj = self.env["tier.review"]