diff --git a/contract/models/contract.py b/contract/models/contract.py index 8008e24fa9..04273a8b3c 100644 --- a/contract/models/contract.py +++ b/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( + "⚠ Recurring invoice generation failed
%s
" + ) + % 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("
%s
") % 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") diff --git a/contract/tests/test_contract.py b/contract/tests/test_contract.py index c42de5ad82..3b0eea1809 100644 --- a/contract/tests/test_contract.py +++ b/contract/tests/test_contract.py @@ -5,6 +5,7 @@ import logging from collections import namedtuple +from unittest import mock from dateutil.relativedelta import relativedelta from freezegun import freeze_time @@ -12,6 +13,7 @@ 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" diff --git a/contract/views/contract.xml b/contract/views/contract.xml index 50dab8bac4..354880391a 100644 --- a/contract/views/contract.xml +++ b/contract/views/contract.xml @@ -24,6 +24,40 @@