[19.0][IMP] contract: harden the recurring-invoice cron

contract/models/contract.py

@@ -125,6 +125,22 @@ class ContractContract(models.Model):
     # === Dates ===
     date_end = fields.Date(compute="_compute_date_end", store=True, readonly=False)
 
+    # === Cron status ===
+    invoice_generation_error = fields.Text(
+        readonly=True,
+        copy=False,
+        help="Error message from the most recent failed scheduled invoicing run.",
+    )
+    invoice_generation_error_date = fields.Datetime(
+        readonly=True,
+        copy=False,
+    )
+    has_invoice_generation_error = fields.Boolean(
+        compute="_compute_has_invoice_generation_error",
+        store=True,
+        help="Set when the last scheduled invoicing run failed for this contract.",
+    )
+
     # === Compute Methods ===
 
     def _compute_access_url(self):
@@ -210,6 +226,11 @@ def _compute_date_end(self):
             if date_end and all(date_end):
                 contract.date_end = max(date_end)
 
+    @api.depends("invoice_generation_error")
+    def _compute_has_invoice_generation_error(self):
+        for rec in self:
+            rec.has_invoice_generation_error = bool(rec.invoice_generation_error)
+
     def _inverse_partner_id(self):
         for rec in self:
             if not rec.invoice_partner_id:
@@ -668,9 +689,22 @@ def _get_recurring_create_func(self, create_type="invoice"):
 
     @api.model
     def _cron_recurring_create(self, date_ref=False, create_type="invoice"):
-        """
-        The cron function in order to create recurrent documents
-        from contracts.
+        """Create recurrent documents from contracts.
+
+        Strategy:
+
+        * Fast path - the company's contracts are processed in one batched
+          ``create()`` call, matching the upstream behaviour and
+          performance.
+        * On failure - fall back to per-contract processing inside
+          savepoints, so a single contract with bad data does not block
+          its whole company batch and SQL-level errors do not poison the
+          transaction for the remaining contracts.
+
+        Failed contracts are flagged via
+        :attr:`has_invoice_generation_error`, notified via chatter and a
+        TODO activity, and cleared automatically on the next successful
+        run.
         """
         _recurring_create_func = self._get_recurring_create_func(
             create_type=create_type
@@ -695,9 +729,123 @@ def _cron_recurring_create(self, date_ref=False, create_type="invoice"):
                     or contract.recurring_next_date <= contract.date_end
                 )
             ).with_company(company)
-            _recurring_create_func(contracts_to_invoice, date_ref)
+            if not contracts_to_invoice:
+                continue
+            # Fast path: one batched call for the whole company.
+            try:
+                with self.env.cr.savepoint():
+                    _recurring_create_func(contracts_to_invoice, date_ref)
+                contracts_to_invoice._clear_invoice_generation_error()
+            except Exception:
+                _logger.warning(
+                    "Batched %s generation failed for %d contract(s) in %s; "
+                    "retrying contracts individually",
+                    create_type,
+                    len(contracts_to_invoice),
+                    company.display_name,
+                )
+                # Slow path: isolate each contract behind its own savepoint
+                # so a failing one does not affect the rest.
+                for contract in contracts_to_invoice:
+                    try:
+                        with self.env.cr.savepoint():
+                            _recurring_create_func(contract, date_ref)
+                        contract._clear_invoice_generation_error()
+                    except Exception as exc:
+                        contract._record_invoice_generation_error(exc, create_type)
+        return True
+
+    def _record_invoice_generation_error(self, exc, create_type):
+        """Flag the contract, log, post chatter, and schedule a TODO activity.
+
+        Chatter and activity creation are de-duplicated so that a contract
+        whose data is broken for several consecutive cron runs does not
+        spam its responsible user.
+        """
+        self.ensure_one()
+        message = f"{type(exc).__name__}: {exc}"
+        self.write(
+            {
+                "invoice_generation_error": message,
+                "invoice_generation_error_date": fields.Datetime.now(),
+            }
+        )
+        # Use ``exc_info=exc`` rather than ``_logger.exception`` so the
+        # traceback is logged when available (real cron failure) and a
+        # clean line is logged when not (e.g. when this helper is invoked
+        # directly to seed a flag in tests).
+        _logger.error(
+            "Failed to create recurring %s for contract %s (id=%s): %s",
+            create_type,
+            self.name,
+            self.id,
+            message,
+            exc_info=exc,
+        )
+        # Avoid posting the same error message repeatedly.
+        last_body = self.message_ids[:1].body or ""
+        if message not in last_body:
+            self.message_post(
+                body=Markup(
+                    "<b>⚠ Recurring invoice generation failed</b><br/><pre>%s</pre>"
+                )
+                % message,
+            )
+        # One open todo per failure; do not duplicate while it is pending.
+        activity_type = self.env.ref(
+            "mail.mail_activity_data_todo", raise_if_not_found=False
+        )
+        if activity_type and not self.activity_ids.filtered(
+            lambda a, t=activity_type: a.activity_type_id == t
+            and a.summary == "Recurring invoice failed"
+        ):
+            self.activity_schedule(
+                "mail.mail_activity_data_todo",
+                summary="Recurring invoice failed",
+                note=Markup("<pre>%s</pre>") % message,
+                user_id=(self.user_id or self.env.user).id,
+            )
+
+    def _clear_invoice_generation_error(self):
+        """Clear the error flag and resolve the related activity."""
+        had_error = self.filtered("has_invoice_generation_error")
+        if not had_error:
+            return
+        had_error.write(
+            {
+                "invoice_generation_error": False,
+                "invoice_generation_error_date": False,
+            }
+        )
+        activity_type = self.env.ref(
+            "mail.mail_activity_data_todo", raise_if_not_found=False
+        )
+        if activity_type:
+            stale = had_error.activity_ids.filtered(
+                lambda a, t=activity_type: a.activity_type_id == t
+                and a.summary == "Recurring invoice failed"
+            )
+            if stale:
+                stale.action_feedback(
+                    feedback="Recurring invoice generated successfully."
+                )
+
+    def action_clear_invoice_generation_error(self):
+        """Manually dismiss the invoice-generation error flag."""
+        self._clear_invoice_generation_error()
         return True
 
+    def action_view_invoice_generation_error(self):
+        """Open the contract form so the user can inspect the error."""
+        self.ensure_one()
+        return {
+            "type": "ir.actions.act_window",
+            "res_model": self._name,
+            "res_id": self.id,
+            "view_mode": "form",
+            "target": "current",
+        }
+
     @api.model
     def cron_recurring_create_invoice(self, date_ref=None):
         return self._cron_recurring_create(date_ref, create_type="invoice")

contract/tests/test_contract.py

@@ -5,13 +5,15 @@
 
 import logging
 from collections import namedtuple
+from unittest import mock
 
 from dateutil.relativedelta import relativedelta
 from freezegun import freeze_time
 
 from odoo import Command, fields
 from odoo.exceptions import ValidationError
 from odoo.tests import Form, new_test_user
+from odoo.tools import mute_logger
 
 from odoo.addons.base.tests.common import BaseCommon
 
@@ -1145,6 +1147,176 @@ def test_cron_recurring_create_invoice(self):
             len(invoice_lines),
         )
 
+    def test_cron_isolates_failing_contract(self):
+        """A contract that raises during invoicing must not block the others."""
+        self.acct_line.date_start = "2018-01-01"
+        self.acct_line.recurring_invoicing_type = "post-paid"
+        self.acct_line.date_end = "2018-03-15"
+        good_a = self.contract2
+        good_b = self.contract.copy()
+        bad = self.contract.copy()
+        boom = RuntimeError("boom: bad data")
+        original = self.env[
+            "contract.contract"
+        ].__class__._prepare_recurring_invoices_values
+
+        def patched(self, date_ref=False):
+            if bad.id in self.ids:
+                raise boom
+            return original(self, date_ref)
+
+        Contract = self.env["contract.contract"]
+        with mock.patch.object(
+            Contract.__class__,
+            "_prepare_recurring_invoices_values",
+            patched,
+        ):
+            with mute_logger("odoo.addons.contract.models.contract", "odoo.sql_db"):
+                Contract.cron_recurring_create_invoice()
+
+        # The two healthy contracts still got their invoice lines.
+        good_lines = self.env["account.move.line"].search(
+            [
+                (
+                    "contract_line_id",
+                    "in",
+                    (good_a | good_b).mapped("contract_line_ids").ids,
+                )
+            ]
+        )
+        self.assertEqual(
+            len(good_lines),
+            len((good_a | good_b).mapped("contract_line_ids")),
+        )
+        # The failing contract is flagged, with chatter and an activity.
+        self.assertTrue(bad.has_invoice_generation_error)
+        self.assertIn("boom", bad.invoice_generation_error)
+        self.assertTrue(bad.invoice_generation_error_date)
+        self.assertEqual(
+            len(
+                bad.activity_ids.filtered(
+                    lambda a: a.summary == "Recurring invoice failed"
+                )
+            ),
+            1,
+        )
+        # The healthy ones are not flagged.
+        self.assertFalse(good_a.has_invoice_generation_error)
+        self.assertFalse(good_b.has_invoice_generation_error)
+
+    def test_cron_clears_error_on_recovery(self):
+        """A successful run clears the error flag and resolves the activity."""
+        self.acct_line.date_start = "2018-01-01"
+        self.acct_line.recurring_invoicing_type = "post-paid"
+        self.acct_line.date_end = "2018-03-15"
+        bad = self.contract2
+        # Simulate a previous failure so the contract is flagged. Mute the
+        # expected ERROR log -- ``checklog-odoo`` would otherwise fail CI.
+        with mute_logger("odoo.addons.contract.models.contract"):
+            bad._record_invoice_generation_error(
+                RuntimeError("earlier failure"), "invoice"
+            )
+        self.assertTrue(bad.has_invoice_generation_error)
+        open_activities = bad.activity_ids.filtered(
+            lambda a: a.summary == "Recurring invoice failed"
+        )
+        self.assertEqual(len(open_activities), 1)
+        # A clean cron run later should clear the flag.
+        self.env["contract.contract"].cron_recurring_create_invoice()
+        self.assertFalse(bad.has_invoice_generation_error)
+        self.assertFalse(bad.invoice_generation_error)
+        self.assertFalse(
+            bad.activity_ids.filtered(lambda a: a.summary == "Recurring invoice failed")
+        )
+
+    def test_cron_does_not_duplicate_activity_or_chatter(self):
+        """Re-running the cron with the same broken data does not spam."""
+        self.acct_line.date_start = "2018-01-01"
+        self.acct_line.recurring_invoicing_type = "post-paid"
+        self.acct_line.date_end = "2018-03-15"
+        bad = self.contract.copy()
+        boom = RuntimeError("boom: persistent bad data")
+        original = self.env[
+            "contract.contract"
+        ].__class__._prepare_recurring_invoices_values
+
+        def patched(self, date_ref=False):
+            if bad.id in self.ids:
+                raise boom
+            return original(self, date_ref)
+
+        Contract = self.env["contract.contract"]
+        with mock.patch.object(
+            Contract.__class__,
+            "_prepare_recurring_invoices_values",
+            patched,
+        ):
+            with mute_logger("odoo.addons.contract.models.contract", "odoo.sql_db"):
+                Contract.cron_recurring_create_invoice()
+                Contract.cron_recurring_create_invoice()
+
+        open_activities = bad.activity_ids.filtered(
+            lambda a: a.summary == "Recurring invoice failed"
+        )
+        self.assertEqual(len(open_activities), 1, "Activity must not duplicate")
+        relevant_chatter = bad.message_ids.filtered(
+            lambda m: m.body and "Recurring invoice generation failed" in m.body
+        )
+        self.assertEqual(len(relevant_chatter), 1, "Chatter must not duplicate")
+
+    def test_cron_uses_batch_fast_path_when_all_succeed(self):
+        """Healthy contracts should hit the single-batched create path:
+        ``_recurring_create_invoice`` is invoked once per company with the
+        whole batch, not once per contract.
+        """
+        self.acct_line.date_start = "2018-01-01"
+        self.acct_line.recurring_invoicing_type = "post-paid"
+        self.acct_line.date_end = "2018-03-15"
+        contracts = self.contract2
+        for _i in range(3):
+            contracts |= self.contract.copy()
+        original = self.env["contract.contract"].__class__._recurring_create_invoice
+        call_recordset_sizes = []
+
+        def spy(self, date_ref=False):
+            call_recordset_sizes.append(len(self))
+            return original(self, date_ref)
+
+        Contract = self.env["contract.contract"]
+        with mock.patch.object(Contract.__class__, "_recurring_create_invoice", spy):
+            Contract.cron_recurring_create_invoice()
+
+        # The batch path should be entered once per company (one company in
+        # this test), with a recordset large enough to cover our contracts.
+        self.assertEqual(
+            len(call_recordset_sizes),
+            1,
+            "Batch path must be called once per company, not per contract",
+        )
+        self.assertGreaterEqual(
+            call_recordset_sizes[0],
+            len(contracts),
+            "Batch should include all healthy contracts",
+        )
+
+    def test_action_clear_invoice_generation_error(self):
+        """The dismiss button clears the flag without running the cron."""
+        bad = self.contract
+        with mute_logger("odoo.addons.contract.models.contract"):
+            bad._record_invoice_generation_error(RuntimeError("oops"), "invoice")
+        self.assertTrue(bad.has_invoice_generation_error)
+        bad.action_clear_invoice_generation_error()
+        self.assertFalse(bad.has_invoice_generation_error)
+        self.assertFalse(bad.invoice_generation_error)
+
+    def test_action_view_invoice_generation_error(self):
+        """The error smart-action opens the contract form view."""
+        action = self.contract.action_view_invoice_generation_error()
+        self.assertEqual(action["type"], "ir.actions.act_window")
+        self.assertEqual(action["res_model"], "contract.contract")
+        self.assertEqual(action["res_id"], self.contract.id)
+        self.assertEqual(action["view_mode"], "form")
+
     def test_get_period_to_invoice_monthlylastday_postpaid(self):
         self.acct_line.date_start = "2018-01-05"
         self.acct_line.recurring_invoicing_type = "post-paid"