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..00b6c455 100644 --- a/base_tier_validation/models/tier_review.py +++ b/base_tier_validation/models/tier_review.py @@ -106,11 +106,28 @@ def _update_review_status(self): reviews = self.filtered(lambda rev: rev.status in ["waiting", "pending"]) if not reviews: return - next_seq = min(reviews.mapped("sequence")) + # Lowest still-open sequence *per record*, computed over the whole + # record's reviews -- NOT just this recordset. Computing it over + # ``self`` promotes a later tier prematurely whenever the method runs on + # a subset that excludes the lower-sequence predecessors, e.g. + # ``review_user_count`` calling ``user.review_ids._update_review_status()`` + # for a second-tier reviewer (their own review is then the only -- and + # thus "minimum" -- sequence in the set, so it wrongly goes ``pending``). + min_seq_by_record = {} + for model, res_id in {(rev.model, rev.res_id) for rev in reviews}: + open_reviews = ( + self.env[model] + .browse(res_id) + .review_ids.filtered(lambda r: r.status in ("waiting", "pending")) + ) + min_seq_by_record[(model, res_id)] = min(open_reviews.mapped("sequence")) for record in reviews: if record.status != "waiting": continue - if record.approve_sequence and record.sequence != next_seq: + if ( + record.approve_sequence + and record.sequence != min_seq_by_record[(record.model, record.res_id)] + ): continue record.status = "pending" if record.definition_id.notify_on_pending: diff --git a/base_tier_validation/tests/test_tier_validation.py b/base_tier_validation/tests/test_tier_validation.py index 3f2ab80c..3414b21b 100644 --- a/base_tier_validation/tests/test_tier_validation.py +++ b/base_tier_validation/tests/test_tier_validation.py @@ -689,6 +689,67 @@ def test_19a_notify_on_pending_sequence_negative(self): "Second-tier reviewer must be notified once promoted to pending.", ) + def test_19c_no_premature_promotion_on_subset(self): + """Regression: ``_update_review_status`` run on a *subset* of a record's + reviews -- as ``review_user_count`` does per user, via + ``user.review_ids._update_review_status()`` -- must gate the sequence + over the *whole* record, not the subset. Otherwise a later-tier review + is promoted to ``pending`` before its predecessor is approved, which + both hides it from the reviewer's systray (``can_review`` is False while + the lower tier is still pending) and fires a ``notify_on_pending`` + message authored by -- and so never delivered to -- that reviewer. + """ + TierDefinition = self.env["tier.definition"] + test_record = self.test_model.create({"test_field": 2.5}) + 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, + "notify_on_pending": True, + "sequence": 20, + "name": "First in sequence -- user 1", + } + ) + 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, + "notify_on_pending": True, + "sequence": 10, + "name": "Second in sequence -- user 2", + } + ) + reviews = test_record.request_validation() + review_second = reviews.filtered(lambda r: self.test_user_2 in r.reviewer_ids) + self.assertEqual(review_second.status, "waiting") + messages_before = test_record.message_ids + + # Simulate the second-tier reviewer opening their systray: the promotion + # runs on *only* their own review -- the subset that excludes the + # first-tier predecessor. + review_second._update_review_status() + review_second.invalidate_recordset() + + self.assertEqual( + review_second.status, + "waiting", + "A later-tier review must stay 'waiting' until its predecessor is " + "approved, even when _update_review_status runs on a subset that " + "excludes the predecessor.", + ) + self.assertEqual( + test_record.message_ids, + messages_before, + "No notify_on_pending message must be posted from promoting a later " + "tier out of order.", + ) + def test_19b_notify_review_available_no_op_when_no_users(self): """``_notify_review_available`` must short-circuit (no follower added, no chatter message posted) when none of the passed reviews