diff --git a/base_tier_validation/__manifest__.py b/base_tier_validation/__manifest__.py index e826d37b..763d20f6 100644 --- a/base_tier_validation/__manifest__.py +++ b/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", diff --git a/base_tier_validation/models/tier_review.py b/base_tier_validation/models/tier_review.py index bed4caaf..0e9904be 100644 --- a/base_tier_validation/models/tier_review.py +++ b/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"] diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py index 3f2ab80c..fac02079 100644 --- a/base_tier_validation/tests/test_tier_validation.py +++ b/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"]