[FIX] base_tier_validation_correction: honest revert via snapshot

base_tier_validation_correction/models/tier_correction_item.py

@@ -33,6 +33,15 @@ class TierCorrectionItem(models.Model):
         string="Affected Tier Reviews",
         help="Tier reivews that will be affected by this correction.",
     )
+    original_reviewer_data = fields.Json(
+        string="Original Reviewers Snapshot",
+        readonly=True,
+        help="Snapshot of each affected review's reviewer_ids taken at the "
+        "moment of correction. Used to restore the exact same reviewers on "
+        "revert, so that subsequent edits to the tier definition (e.g. a "
+        "group membership change) do not silently leak into the reverted "
+        "state.",
+    )
 
     def _notify_reviewer_change(self, ttype="correct"):
         self.ensure_one()
@@ -73,6 +82,9 @@ def correct(self):
             reviews = item.review_ids.filtered(
                 lambda record: record.status in ["waiting", "pending"]
             )
+            item.original_reviewer_data = {
+                str(review.id): review.reviewer_ids.ids for review in reviews
+            }
             reviews.write({"reviewer_ids": [Command.set(item.new_reviewer_ids.ids)]})
             item._notify_reviewer_change("correct")
 
@@ -81,6 +93,13 @@ def revert(self):
             reviews = item.review_ids.filtered(
                 lambda record: record.status in ["waiting", "pending"]
             )
+            snapshot = item.original_reviewer_data or {}
             for review in reviews:
-                review.reviewer_ids = review._get_reviewers()
+                original_ids = snapshot.get(str(review.id))
+                if original_ids is None:
+                    # Fallback for legacy items created before the snapshot
+                    # field existed: recompute from the current definition.
+                    review.reviewer_ids = review._get_reviewers()
+                else:
+                    review.reviewer_ids = [Command.set(list(original_ids))]
             item._notify_reviewer_change("revert")

base_tier_validation_correction/tests/test_tier_validation.py

@@ -97,6 +97,62 @@ def test_01_tier_correction(self):
         res = doc_user1.with_context(**ctx).view_tier_correction()
         self.assertEqual(res["domain"][0][2], [correction.id])
 
+    def test_revert_restores_snapshot_not_current_definition(self):
+        """Reverting a correction must restore the original reviewer_ids
+        captured at correction time, even if the tier.definition was
+        re-pointed at a different reviewer in the meantime.
+        """
+        doc_user2 = self.test_record.with_user(self.test_user_2.id)
+        doc_user2.request_validation()
+        correction = self.env["tier.correction"].create(
+            {
+                "name": "Snapshot test",
+                "model_id": self.tester_model.id,
+                "correction_type": "reviewer",
+                "search_name": self.test_record.display_name,
+                "new_reviewer_ids": [Command.set(self.test_user_2.ids)],
+                "old_reviewer_ids": [Command.set(self.test_user_1.ids)],
+            }
+        )
+        correction.action_prepare()
+        correction.action_done()
+        review = self.test_record.review_ids
+        self.assertEqual(review.reviewer_ids, self.test_user_2)
+        snapshot = correction.item_ids.original_reviewer_data
+        self.assertEqual(snapshot[str(review.id)], self.test_user_1.ids)
+        # Re-point the underlying definition at a different user. A naive
+        # revert that recomputes from the definition would now restore this
+        # user instead of the originally-snapshotted test_user_1.
+        self.definition_1.reviewer_id = self.test_user_3_multi_company
+        correction.action_revert()
+        review.invalidate_recordset()
+        self.assertEqual(review.reviewer_ids, self.test_user_1)
+
+    def test_revert_legacy_item_without_snapshot(self):
+        """An item created before the snapshot field existed has an empty
+        original_reviewer_data; revert must fall back to recomputing from
+        the current definition rather than crashing or wiping reviewers.
+        """
+        doc_user2 = self.test_record.with_user(self.test_user_2.id)
+        doc_user2.request_validation()
+        correction = self.env["tier.correction"].create(
+            {
+                "name": "Legacy revert",
+                "model_id": self.tester_model.id,
+                "correction_type": "reviewer",
+                "search_name": self.test_record.display_name,
+                "new_reviewer_ids": [Command.set(self.test_user_2.ids)],
+            }
+        )
+        correction.action_prepare()
+        correction.action_done()
+        # Simulate an item written by a pre-fix version of the module.
+        correction.item_ids.original_reviewer_data = False
+        correction.action_revert()
+        review = self.test_record.review_ids
+        review.invalidate_recordset()
+        self.assertEqual(review.reviewer_ids, self.test_user_1)
+
     def test_01_tier_correction_by_scheduler(self):
         """With the document in validation,
         - User click on Change Reviewer to creat new correction