[19.0][IMP] base_tier_validation: reminder cron — batch limit, recordset semantics, orphan guard

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.2",
+    "version": "19.0.1.0.3",
     "development_status": "Mature",
     "maintainers": ["LoisRForgeFlow"],
     "category": "Tools",

base_tier_validation/models/tier_definition.py

@@ -154,7 +154,7 @@ def _get_review_needing_reminder(self):
         )
         return self.env["tier.review"].search(
             domain,
-            limit=1,
+            limit=50,
         )
 
     def _cron_send_review_reminder(self):

base_tier_validation/models/tier_review.py

@@ -189,16 +189,20 @@ def _notify_review_reminder_body(self):
         return self.env._("A review has been requested %s days ago.", delay)
 
     def _send_review_reminder(self):
-        record = self.env[self.model].browse(self.res_id)
-        # Only schedule activity if reviewer is a single user and model has activities
-        if len(self.reviewer_ids) == 1 and hasattr(record, "activity_ids"):
-            self._schedule_review_reminder_activity(record)
-        elif hasattr(record, "message_post"):
-            self._notify_review_reminder(record)
-        else:
-            msg = f"Could not send reminder for record {record}"
-            _logger.exception(msg)
-        self.last_reminder_date = fields.Datetime.now()
+        for rev in self:
+            record = self.env[rev.model].browse(rev.res_id)
+            if not record.exists():  # <-- skip orphaned reviews
+                continue
+            # Only schedule activity if reviewer is a single user and model
+            # has activities
+            if len(rev.reviewer_ids) == 1 and hasattr(record, "activity_ids"):
+                rev._schedule_review_reminder_activity(record)
+            elif hasattr(record, "message_post"):
+                rev._notify_review_reminder(record)
+            else:
+                msg = f"Could not send reminder for record {record}"
+                _logger.exception(msg)
+            rev.last_reminder_date = fields.Datetime.now()
 
     def _notify_review_reminder(self, record):
         record.message_post(

base_tier_validation/tests/test_tier_validation_reminder.py

@@ -44,3 +44,66 @@ def test_validation_reminder(self):
         with freeze_time(in_9_days):
             self.tier_definition._cron_send_review_reminder()
         self.assertEqual(review.last_reminder_date, in_9_days)
+
+    def test_validation_reminder_batch(self):
+        """A single cron run reminds every eligible review in a definition,
+        not just the first one.
+
+        Regression coverage for the recordset-aware iteration in
+        ``_send_review_reminder`` plus the bumped ``limit`` in
+        ``_get_review_needing_reminder``. Before the fix, only the first
+        review of the batch ever got ``last_reminder_date`` set per cron
+        tick -- a silent backlog on installations with many pending
+        reviews under the same definition.
+        """
+        tier_definition = self.tier_definition
+        tier_definition.notify_reminder_delay = 1
+        extra_records = self.test_model.create([{"test_field": 1.0} for _ in range(2)])
+        records = self.test_record + extra_records
+        for record in records:
+            record.with_user(self.test_user_2.id).request_validation()
+        reviews = self.env["tier.review"].search(
+            [
+                ("definition_id", "=", tier_definition.id),
+                ("res_id", "in", records.ids),
+            ]
+        )
+        self.assertEqual(len(reviews), 3)
+        for rev in reviews:
+            self.assertFalse(rev.last_reminder_date)
+        later = fields.Datetime.add(fields.Datetime.now(), days=2)
+        with freeze_time(later):
+            tier_definition._cron_send_review_reminder()
+        for rev in reviews:
+            self.assertEqual(rev.last_reminder_date, later)
+
+    def test_validation_reminder_skips_orphans(self):
+        """A tier.review whose validated record no longer exists must not
+        crash the cron. Such orphans only happen when the document was
+        deleted by something that bypassed the cascade unlink (raw SQL,
+        broken module uninstall, ...). The reminder cron is then expected
+        to skip them silently."""
+        tier_definition = self.tier_definition
+        tier_definition.notify_reminder_delay = 1
+        self.test_record.with_user(self.test_user_2.id).request_validation()
+        review = self.env["tier.review"].search(
+            [
+                ("definition_id", "=", tier_definition.id),
+                ("res_id", "=", self.test_record.id),
+            ]
+        )
+        self.assertTrue(review)
+        # Drop the validated record at the SQL level so the cascade
+        # unlink defined on ``tier.validation`` does not also remove the
+        # review row -- that's what makes the review an orphan.
+        self.env.cr.execute(
+            "DELETE FROM tier_validation_tester WHERE id = %s",
+            (self.test_record.id,),
+        )
+        self.env.invalidate_all()
+        later = fields.Datetime.add(fields.Datetime.now(), days=2)
+        # Before the orphan guard, this raised on browse(...).message_post().
+        with freeze_time(later):
+            tier_definition._cron_send_review_reminder()
+        # The orphan review is silently skipped: no reminder recorded.
+        self.assertFalse(review.last_reminder_date)