From ad8640aff374b5c1d0b57c6a2e1401c14e2a89ea Mon Sep 17 00:00:00 2001 From: alvaro-domatix Date: Wed, 27 May 2026 14:21:47 +0000 Subject: [PATCH] [REF] subscription_oca: split subscription cron responsibilities cron_subscription_management used to walk every sale.subscription with self.search([]) and decide inside the loop whether to start, invoice or close each record. A single exception aborted the rest of the batch when the failure happened outside the inner try/except (e.g. while starting a subscription), and there was no way to cap the number of records processed in one run. Split the entry point into three small dedicated methods with restrictive domains, each guarded by per-record try/except: * _cron_start_due_subscriptions: activates draft/pre subscriptions whose date_start has passed and issues their first invoice. * _cron_invoice_due_subscriptions: invoices in-progress subscriptions whose recurring_next_date is due and that actually have lines. * _cron_close_ended_subscriptions: closes in-progress subscriptions whose end date has passed. cron_subscription_management keeps working as an entry point that calls all three in order, with a new optional limit argument forwarded to each. The external behavior of the cron is preserved: existing subscriptions get invoiced, started and closed exactly as before. The legacy cron error test used to assert UserError propagation, which only happened when the failure occurred in the unguarded start branch; with errors now logged per record across all three methods the cron finishes the batch and the test verifies the call rather than the exception. --- subscription_oca/models/sale_subscription.py | 77 ++++++--- subscription_oca/tests/__init__.py | 1 + .../tests/test_subscription_cron.py | 147 ++++++++++++++++++ .../tests/test_subscription_oca.py | 9 +- 4 files changed, 207 insertions(+), 27 deletions(-) create mode 100644 subscription_oca/tests/test_subscription_cron.py diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index e44e9da11f..f16e4291df 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -137,32 +137,63 @@ def _read_group_stage_ids(self, stages, domain): to_renew = fields.Boolean(default=False, string="To renew") @api.model - def cron_subscription_management(self): + def cron_subscription_management(self, limit=None): + self._cron_start_due_subscriptions(limit=limit) + self._cron_invoice_due_subscriptions(limit=limit) + self._cron_close_ended_subscriptions(limit=limit) + + @api.model + def _cron_start_due_subscriptions(self, limit=None): today = date.today() - subscription_count = self.search_count([]) - for subscription in self.search( - [], order="recurring_next_date asc", limit=subscription_count - ): - subscription = subscription.with_company(subscription.company_id) - if subscription.in_progress: - if ( - subscription.recurring_next_date <= today - and subscription.sale_subscription_line_ids - ): - try: - subscription.generate_invoice() - except Exception: - logger.exception("Error on subscription invoice generate") - if ( - not subscription.recurring_rule_boundary - and subscription.date <= today - ): - subscription.close_subscription() - elif ( - subscription.date_start <= today and subscription.stage_id.type == "pre" - ): + domain = [ + ("in_progress", "=", False), + ("date_start", "<=", today), + ("stage_id.type", "=", "pre"), + ] + subscriptions = self.search(domain, limit=limit) + for subscription in subscriptions: + try: + subscription = subscription.with_company(subscription.company_id) subscription.action_start_subscription() subscription.generate_invoice() + except Exception: + logger.exception("Error starting subscription %s", subscription.id) + + @api.model + def _cron_invoice_due_subscriptions(self, limit=None): + today = date.today() + domain = [ + ("in_progress", "=", True), + ("recurring_next_date", "<=", today), + ("sale_subscription_line_ids", "!=", False), + ] + subscriptions = self.search( + domain, order="recurring_next_date asc", limit=limit + ) + for subscription in subscriptions: + try: + subscription.with_company(subscription.company_id).generate_invoice() + except Exception: + logger.exception( + "Error on subscription invoice generate (id=%s)", + subscription.id, + ) + + @api.model + def _cron_close_ended_subscriptions(self, limit=None): + today = date.today() + domain = [ + ("in_progress", "=", True), + ("recurring_rule_boundary", "=", False), + ("date", "!=", False), + ("date", "<=", today), + ] + subscriptions = self.search(domain, limit=limit) + for subscription in subscriptions: + try: + subscription.with_company(subscription.company_id).close_subscription() + except Exception: + logger.exception("Error closing subscription %s", subscription.id) @api.depends("sale_subscription_line_ids") def _compute_total(self): diff --git a/subscription_oca/tests/__init__.py b/subscription_oca/tests/__init__.py index f445239d7f..d68d99101a 100644 --- a/subscription_oca/tests/__init__.py +++ b/subscription_oca/tests/__init__.py @@ -1,3 +1,4 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). from . import test_subscription_oca +from . import test_subscription_cron diff --git a/subscription_oca/tests/test_subscription_cron.py b/subscription_oca/tests/test_subscription_cron.py new file mode 100644 index 0000000000..d8bcfa862c --- /dev/null +++ b/subscription_oca/tests/test_subscription_cron.py @@ -0,0 +1,147 @@ +# Copyright 2026 Domatix - Alvaro Domatix +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). + +from unittest.mock import patch + +from dateutil.relativedelta import relativedelta + +from odoo import fields +from odoo.exceptions import UserError +from odoo.tools import mute_logger + +from odoo.addons.base.tests.common import BaseCommon +from odoo.addons.product.tests.common import ProductCommon + + +class TestSubscriptionCron(ProductCommon, BaseCommon): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True)) + cls.partner = cls.env["res.partner"].create({"name": "Cron partner"}) + cls.pricelist = cls.env["product.pricelist"].create({"name": "Cron pricelist"}) + cls.template = cls.env["sale.subscription.template"].create( + { + "name": "Cron template", + "code": "CRON-MTH", + "recurring_rule_type": "months", + "recurring_rule_boundary": "unlimited", + "invoicing_mode": "draft", + } + ) + cls.product = cls._create_product( + name="Cron product", + lst_price=10.0, + subscribable=True, + uom_id=cls.uom_unit.id, + ) + + def _make_sub(self, **vals): + defaults = { + "partner_id": self.partner.id, + "pricelist_id": self.pricelist.id, + "template_id": self.template.id, + "date_start": fields.Date.today(), + } + defaults.update(vals) + sub = self.env["sale.subscription"].create(defaults) + self.env["sale.subscription.line"].create( + { + "sale_subscription_id": sub.id, + "product_id": self.product.id, + } + ) + return sub + + def test_cron_does_not_invoice_future_subscription(self): + sub = self._make_sub( + recurring_next_date=fields.Date.today() + relativedelta(days=10), + in_progress=True, + ) + self.env["sale.subscription"]._cron_invoice_due_subscriptions() + self.assertFalse(sub.invoice_ids) + + def test_cron_does_not_invoice_subscription_without_lines(self): + sub = self.env["sale.subscription"].create( + { + "partner_id": self.partner.id, + "pricelist_id": self.pricelist.id, + "template_id": self.template.id, + "date_start": fields.Date.today(), + "recurring_next_date": fields.Date.today(), + "in_progress": True, + } + ) + self.env["sale.subscription"]._cron_invoice_due_subscriptions() + self.assertFalse(sub.invoice_ids) + + def test_cron_invoices_due_subscription(self): + sub = self._make_sub( + recurring_next_date=fields.Date.today(), + in_progress=True, + ) + self.env["sale.subscription"]._cron_invoice_due_subscriptions() + self.assertEqual(len(sub.invoice_ids), 1) + + def test_cron_error_on_one_does_not_stop_batch(self): + sub_ok = self._make_sub( + recurring_next_date=fields.Date.today(), + in_progress=True, + ) + sub_bad = self._make_sub( + recurring_next_date=fields.Date.today(), + in_progress=True, + ) + original_generate = type(sub_ok).generate_invoice + sub_bad_id = sub_bad.id + + def side_effect(records): + if records.id == sub_bad_id: + raise UserError(records.env._("boom")) + return original_generate(records) + + with patch.object( + type(sub_ok), "generate_invoice", autospec=True, side_effect=side_effect + ): + with mute_logger("odoo.addons.subscription_oca.models.sale_subscription"): + self.env["sale.subscription"]._cron_invoice_due_subscriptions() + + self.assertEqual(len(sub_ok.invoice_ids), 1) + self.assertFalse(sub_bad.invoice_ids) + + def test_cron_close_ended_subscription(self): + template_limited = self.env["sale.subscription.template"].create( + { + "name": "Limited template", + "code": "CRON-LIM", + "recurring_rule_type": "months", + "recurring_rule_boundary": "limited", + "recurring_rule_count": 1, + } + ) + sub = self.env["sale.subscription"].create( + { + "partner_id": self.partner.id, + "pricelist_id": self.pricelist.id, + "template_id": template_limited.id, + "date_start": fields.Date.today() - relativedelta(months=2), + "in_progress": True, + } + ) + self.env["sale.subscription"]._cron_close_ended_subscriptions() + self.assertFalse(sub.in_progress) + + def test_cron_limit_param(self): + for _ in range(3): + self._make_sub( + recurring_next_date=fields.Date.today(), + in_progress=True, + ) + self.env["sale.subscription"]._cron_invoice_due_subscriptions(limit=1) + invoiced = self.env["sale.subscription"].search_count( + [ + ("partner_id", "=", self.partner.id), + ("invoice_ids", "!=", False), + ] + ) + self.assertEqual(invoiced, 1) diff --git a/subscription_oca/tests/test_subscription_oca.py b/subscription_oca/tests/test_subscription_oca.py index 29d301ee1f..6ab52a9685 100644 --- a/subscription_oca/tests/test_subscription_oca.py +++ b/subscription_oca/tests/test_subscription_oca.py @@ -381,12 +381,13 @@ def test_subscription_oca_sub_lines(self): "SaleSubscription.generate_invoice" ) def test_subscription_oca_sub_cron_error(self, generate_invoice_patch): - # Simulate something failing in generating an invoice, - # we expect something being logged + # Simulate something failing in generating an invoice. + # The cron logs the error per record and continues, so the batch + # finishes without propagating the exception. generate_invoice_patch.side_effect = exceptions.UserError("Error") with mute_logger("odoo.addons.subscription_oca.models.sale_subscription"): - with self.assertRaises(exceptions.UserError): - self.sub1.cron_subscription_management() + self.sub1.cron_subscription_management() + self.assertTrue(generate_invoice_patch.called) def test_subscription_oca_sub_cron(self): # sale.subscription