From 0fe89cff7f0a6211494dfa34aa05b063057a7f7d Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 14:47:55 -0400 Subject: [PATCH 1/6] [FIX] subscription_oca: fix test fixtures and cron error test Add stage_in_progress fixture with type in_progress and assign it to sub7, sub8, sub9 so their in_progress=True matches a proper stage type. Update test_subscription_oca_sub_cron_error to verify the cron catches and logs exceptions instead of propagating them. --- .../tests/test_subscription_oca.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/subscription_oca/tests/test_subscription_oca.py b/subscription_oca/tests/test_subscription_oca.py index 0d9ca7afca..4cee83f9ef 100644 --- a/subscription_oca/tests/test_subscription_oca.py +++ b/subscription_oca/tests/test_subscription_oca.py @@ -7,7 +7,6 @@ from dateutil.relativedelta import relativedelta from odoo import Command, exceptions, fields -from odoo.tools import mute_logger from odoo.addons.base.tests.common import BaseCommon @@ -130,6 +129,12 @@ def setUpClass(cls): "type": "pre", } ) + cls.stage_in_progress = cls.env["sale.subscription.stage"].create( + { + "name": "Test Sub Stage In Progress", + "type": "in_progress", + } + ) cls.tag = cls.env["sale.subscription.tag"].create( { "name": "Test Tag", @@ -176,6 +181,7 @@ def setUpClass(cls): "pricelist_id": cls.pricelist2.id, "date_start": fields.Date.today() - relativedelta(days=100), "in_progress": True, + "stage_id": cls.stage_in_progress.id, } ) cls.sub8 = cls.create_sub( @@ -184,6 +190,7 @@ def setUpClass(cls): "pricelist_id": cls.pricelist2.id, "date_start": fields.Date.today() - relativedelta(days=100), "in_progress": True, + "stage_id": cls.stage_in_progress.id, "journal_id": cls.cash_journal.id, } ) @@ -192,6 +199,7 @@ def setUpClass(cls): "template_id": cls.tmpl3.id, "date_start": fields.Date.today() - relativedelta(days=100), "in_progress": True, + "stage_id": cls.stage_in_progress.id, "recurring_rule_boundary": True, } ) @@ -349,12 +357,19 @@ 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 + # When generate_invoice() fails the cron must NOT raise — + # it must log the error and continue processing other subscriptions. 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() + with self.assertLogs( + "odoo.addons.subscription_oca.models.sale_subscription", level="ERROR" + ) as log_watcher: + self.sub1.cron_subscription_management() + self.assertTrue( + any( + "Error on subscription management for subscription" in msg + for msg in log_watcher.output + ) + ) def test_subscription_oca_sub_cron(self): # sale.subscription From 7852739817e2e9a6cf47f5c7b453073a55d17fc8 Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 14:48:02 -0400 Subject: [PATCH 2/6] [FIX] subscription_oca: consolidate close_subscription atomic write Merge recurring_next_date, close_reason_id, and stage_id into a single write() call to eliminate the split-write window that could leave recurring_next_date=False with in_progress=True. --- subscription_oca/models/sale_subscription.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index 114db35968..22255c3588 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -253,14 +253,14 @@ def action_close_subscription(self): def close_subscription(self, close_reason_id=False): self.ensure_one() - self.recurring_next_date = False closed_stage = self.env["sale.subscription.stage"].search( [("type", "=", "post")], limit=1 ) self.write( { + "recurring_next_date": False, "close_reason_id": close_reason_id, - "stage_id": closed_stage, + "stage_id": closed_stage.id, } ) From fc598cc59229864084b5403e486faff173b5180a Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 14:48:09 -0400 Subject: [PATCH 3/6] [FIX] subscription_oca: add constraint for in_progress consistency Enforce in_progress=True requires stage_id.type==in_progress and a valid recurring_next_date via @api.constrains. --- subscription_oca/models/sale_subscription.py | 23 +++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index 22255c3588..c9138a562e 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -7,7 +7,7 @@ from markupsafe import Markup from odoo import Command, api, fields, models -from odoo.exceptions import AccessError +from odoo.exceptions import AccessError, ValidationError logger = logging.getLogger(__name__) @@ -197,6 +197,27 @@ def _compute_rule_boundary(self): ) record.recurring_rule_boundary = False + @api.constrains("in_progress", "stage_id", "recurring_next_date") + def _check_in_progress_consistency(self): + for record in self: + if record.in_progress and record.stage_id.type != "in_progress": + raise ValidationError( + self.env._( + "Subscription '%s': in_progress=True requires a stage of " + "type 'in_progress' (current stage type: '%s')." + ) + % (record.display_name, record.stage_id.type) + ) + if record.in_progress and not record.recurring_next_date: + raise ValidationError( + self.env._( + "Subscription '%s' is marked as in-progress but has no " + "next invoice date. Set a recurring next date before " + "activating the subscription." + ) + % record.display_name + ) + @api.depends("template_id") def _compute_terms(self): for record in self: From 9bd6590a3ab7b7b2e51f9c357befde886b060f4c Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 14:48:19 -0400 Subject: [PATCH 4/6] [FIX] subscription_oca: harden cron with savepoint and domain filter Add per-record savepoint isolation so a single subscription failure does not kill the entire cron. Filter to pre/in_progress stages only. Add defensive guards on recurring_next_date and date comparisons. --- subscription_oca/models/sale_subscription.py | 46 ++++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index c9138a562e..1301443d27 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -139,27 +139,35 @@ def _read_group_stage_ids(self, stages, domain): @api.model def cron_subscription_management(self): today = date.today() - for subscription in self.search([], order="recurring_next_date asc"): + domain = [("stage_id.type", "in", ["pre", "in_progress"])] + for subscription in self.search(domain, order="recurring_next_date asc"): subscription = subscription.with_company(subscription.company_id) - if subscription.in_progress: - if ( - subscription.recurring_next_date <= today - and subscription.sale_subscription_line_ids - ): - try: + try: + with self.env.cr.savepoint(): + if subscription.in_progress: + if ( + subscription.recurring_next_date + and subscription.recurring_next_date <= today + and subscription.sale_subscription_line_ids + ): + subscription.generate_invoice() + if ( + not subscription.recurring_rule_boundary + and subscription.date + and subscription.date <= today + ): + subscription.close_subscription() + elif ( + subscription.date_start <= today + and subscription.stage_id.type == "pre" + ): + subscription.action_start_subscription() 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" - ): - subscription.action_start_subscription() - subscription.generate_invoice() + except Exception: + logger.exception( + "Error on subscription management for subscription %s", + subscription.display_name, + ) @api.depends("sale_subscription_line_ids") def _compute_total(self): From 6c02c2aee26b69e375f4adfd18d9517c16c304cb Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 14:48:30 -0400 Subject: [PATCH 5/6] [FIX] subscription_oca: set in_progress=False in close_subscription Ensure the @api.constrains sees consistent state during write by including in_progress=False in the write dict, so the constraint fires before the write() override sets it. --- subscription_oca/models/sale_subscription.py | 1 + 1 file changed, 1 insertion(+) diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index 1301443d27..8e5e6e562c 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -287,6 +287,7 @@ def close_subscription(self, close_reason_id=False): ) self.write( { + "in_progress": False, "recurring_next_date": False, "close_reason_id": close_reason_id, "stage_id": closed_stage.id, From 5e3044df9e895caddd6f88784f9b6be6f1eb7764 Mon Sep 17 00:00:00 2001 From: "r.perez" Date: Wed, 10 Jun 2026 16:13:32 -0400 Subject: [PATCH 6/6] [FIX] subscription_oca: prevent ValidationError when changing stage via write() Before super().write(), inject in_progress=False into values when the target stage type is not 'in_progress'. This ensures the @api.constrains validator sees consistent state during flush, preventing ValidationError for any write() that changes stage_id on an in-progress subscription. --- subscription_oca/models/sale_subscription.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index 8e5e6e562c..352a71fa9f 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -476,6 +476,12 @@ def _check_dates(self, start, next_invoice): return False def write(self, values): + if "stage_id" in values and "in_progress" not in values: + stage = values["stage_id"] + if isinstance(stage, int): + stage = self.env["sale.subscription.stage"].browse(stage) + if stage.type != "in_progress": + values["in_progress"] = False res = super().write(values) if "stage_id" in values: for record in self: