[18.0][FIX] subscription_oca: prevent cron crash and enforce data consistency

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__)
 
@@ -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):
@@ -197,6 +205,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:
@@ -253,14 +282,15 @@ 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(
             {
+                "in_progress": False,
+                "recurring_next_date": False,
                 "close_reason_id": close_reason_id,
-                "stage_id": closed_stage,
+                "stage_id": closed_stage.id,
             }
         )
 
@@ -446,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:

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