From 433eb60806c5e5464e4defd1db0dbdae46e9f608 Mon Sep 17 00:00:00 2001 From: twtrubiks Date: Thu, 29 Oct 2026 10:43:25 +0800 Subject: [PATCH] [FIX] base_tier_validation_correction: revert message lists restored reviewers only The correction and revert messages listed the reviewers of all the reviews of the item, including reviews approved in the meantime, which are left untouched. Only list the waiting and pending reviews, the ones actually changed, and post nothing when there are none. Fixes #101 --- .../models/tier_correction_item.py | 8 ++- .../tests/test_tier_validation.py | 64 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/base_tier_validation_correction/models/tier_correction_item.py b/base_tier_validation_correction/models/tier_correction_item.py index fa727805f..4545a8f0b 100644 --- a/base_tier_validation_correction/models/tier_correction_item.py +++ b/base_tier_validation_correction/models/tier_correction_item.py @@ -37,8 +37,12 @@ class TierCorrectionItem(models.Model): def _notify_reviewer_change(self, ttype="correct"): self.ensure_one() post = "message_post" - if hasattr(self.resource_ref, post): - tier_reviews = self.review_ids + # Only the waiting / pending reviews are changed by a correction or a + # revert: list those, not the reviews approved in the meantime. + tier_reviews = self.review_ids.filtered( + lambda record: record.status in ["waiting", "pending"] + ) + if tier_reviews and hasattr(self.resource_ref, post): reviews = ", ".join(tier_reviews.filtered("name").mapped("name")) reviewers = ", ".join( tier_reviews.reviewer_ids.filtered("name").mapped("name") diff --git a/base_tier_validation_correction/tests/test_tier_validation.py b/base_tier_validation_correction/tests/test_tier_validation.py index a07983375..3fa77a073 100644 --- a/base_tier_validation_correction/tests/test_tier_validation.py +++ b/base_tier_validation_correction/tests/test_tier_validation.py @@ -141,3 +141,67 @@ def test_01_tier_correction_by_scheduler(self): self.env["tier.correction"]._tier_correction_auto_run() self.assertTrue(doc_user1.can_review) self.assertFalse(doc_user2.can_review) + + def _get_correction_messages(self): + subtype = self.env.ref( + "base_tier_validation_correction.mt_tier_validation_correction" + ) + return self.test_record.message_ids.filtered(lambda m: m.subtype_id == subtype) + + def _make_correction(self, new_reviewer): + correction = self.env["tier.correction"].create( + { + "name": "Correction", + "model_id": self.tester_model.id, + "document_domain": str([("id", "=", self.test_record.id)]), + "old_reviewer_ids": [Command.set(self.test_user_1.ids)], + "new_reviewer_ids": [Command.set(new_reviewer.ids)], + } + ) + correction.action_prepare() + correction.action_done() + return correction + + def test_revert_message_lists_restored_reviewers_only(self): + """A review approved by the new reviewer is not reverted, so the + revert message does not name that reviewer.""" + new_reviewer = self.test_user_3_multi_company + # Tiers in sequence: user 1, user 2, then user 1 again + self.definition_1.approve_sequence = True + for sequence, reviewer in ((10, self.test_user_1), (20, self.test_user_2)): + self._create_tier_definition( + model_id=self.tester_model.id, + review_type="individual", + reviewer_id=reviewer.id, + definition_domain="[('test_field', '=', 1.0)]", + approve_sequence=True, + sequence=sequence, + ) + self.test_record.request_validation() + reviews = self.test_record.review_ids.sorted("sequence") + first, last = reviews[0], reviews[-1] + correction = self._make_correction(new_reviewer) + self.assertEqual(first.reviewer_ids, new_reviewer) + self.assertEqual(last.reviewer_ids, new_reviewer) + # The new reviewer approves the first tier, the last one stays open + self.test_record.with_user(new_reviewer).validate_tier() + self.assertEqual(first.status, "approved") + self.assertIn(last.status, ["waiting", "pending"]) + correction.action_revert() + self.assertEqual(first.reviewer_ids, new_reviewer) + self.assertEqual(last.reviewer_ids, self.test_user_1) + message = self._get_correction_messages()[0] + self.assertIn("reverted reviewers", message.body) + self.assertIn(self.test_user_1.name, message.body) + self.assertNotIn(new_reviewer.name, message.body) + + def test_revert_message_skipped_when_nothing_reverted(self): + """No revert message when the new reviewer approved every review.""" + new_reviewer = self.test_user_3_multi_company + self.test_record.request_validation() + correction = self._make_correction(new_reviewer) + self.test_record.with_user(new_reviewer).validate_tier() + self.assertEqual(self.test_record.review_ids.status, "approved") + messages_before = self._get_correction_messages() + correction.action_revert() + self.assertEqual(self._get_correction_messages(), messages_before)