From 7e8dd03e32964eec67f2d3441459fbc0f1ed6f07 Mon Sep 17 00:00:00 2001 From: alvaro-domatix Date: Wed, 27 May 2026 14:18:49 +0000 Subject: [PATCH] [FIX] subscription_oca: avoid duplicated invoices per period If the cron failed mid-batch and was relaunched, or if a user clicked the manual invoice action twice, the subscription happily produced two invoices for the same billing period. There was no defense against duplicates at the model level. This change adds two helpers on sale.subscription: * _get_existing_invoice_for_period(period_start, period_end): returns the first account.move whose lines link back to this subscription with the given period and whose state is not 'cancel'. Cancelled invoices do not block, since they can legitimately be re-issued. * _can_create_invoice_for_period(period_start, period_end): the boolean wrapper used by callers. manual_invoice raises a UserError with the formatted period when a blocking invoice exists, so the user can act on it. generate_invoice (called by the cron) logs a warning and returns False so the batch continues with the next subscription. The existing _collect_all_sub_test_results helper called create_invoice and manual_invoice in immediate succession on the same period, which under the new policy would mean creating duplicates. The fixture now advances recurring_next_date between the two calls so it tests the intended two-period workflow rather than the previously unprotected duplicate. --- subscription_oca/README.rst | 54 ++++---- subscription_oca/models/sale_subscription.py | 53 ++++++- subscription_oca/readme/USAGE.md | 8 ++ .../static/description/index.html | 10 +- subscription_oca/tests/__init__.py | 1 + .../test_subscription_duplicate_invoices.py | 129 ++++++++++++++++++ .../tests/test_subscription_oca.py | 1 + 7 files changed, 231 insertions(+), 25 deletions(-) create mode 100644 subscription_oca/tests/test_subscription_duplicate_invoices.py diff --git a/subscription_oca/README.rst b/subscription_oca/README.rst index 3bed5854f2..d6bd567e3c 100644 --- a/subscription_oca/README.rst +++ b/subscription_oca/README.rst @@ -11,7 +11,7 @@ Subscription management !! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - !! source digest: sha256:ea96c858a40ee527d0e32ee03859918c082521f205ccc1cb26e96cb3bef27800 + !! source digest: sha256:f284a98a2170cda678d0f33678999997099c9b35bafd0705c49a659120f75f87 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png @@ -74,21 +74,29 @@ To create subscriptions with the sale of a product: Invoice and delivery addresses: -- Each subscription has an *Invoice address* and a *Delivery address*. - They default to the customer's corresponding addresses and can be - overridden per subscription. -- These addresses are propagated to the recurring invoices (the invoice - is addressed to the invoice address and records the delivery one) and - to the sale orders generated by the subscription. -- The fields are shown when the *Customer Addresses* setting (group - *Display Delivery / Invoice addresses*) is enabled. +- Each subscription has an *Invoice address* and a *Delivery address*. + They default to the customer's corresponding addresses and can be + overridden per subscription. +- These addresses are propagated to the recurring invoices (the invoice + is addressed to the invoice address and records the delivery one) and + to the sale orders generated by the subscription. +- The fields are shown when the *Customer Addresses* setting (group + *Display Delivery / Invoice addresses*) is enabled. + +Duplicate invoice prevention: + +- A subscription will not be invoiced twice for the same period. If a + non-cancelled invoice already exists for the period being billed, + *Create Invoice* raises an error and the cron skips the subscription, + so an interrupted and re-run batch cannot duplicate invoices. + Cancelled invoices do not block re-invoicing. Known issues / Roadmap ====================== -- Refactor all the onchanges that have business logic to computed - write-able fields when possible. Keep onchanges only for UI purposes. -- Add tests. +- Refactor all the onchanges that have business logic to computed + write-able fields when possible. Keep onchanges only for UI purposes. +- Add tests. Bug Tracker =========== @@ -112,22 +120,22 @@ Authors Contributors ------------ -- Carlos Martínez -- Carolina Ferrer -- `Ooops404 `__: +- Carlos Martínez +- Carolina Ferrer +- `Ooops404 `__: - - Ilyas + - Ilyas -- `Sygel `__: +- `Sygel `__: - - Harald Panten - - Valentin Vinagre - - Alberto Martínez + - Harald Panten + - Valentin Vinagre + - Alberto Martínez -- Dennis Sluijk -- `IKU Solutions `__: +- Dennis Sluijk +- `IKU Solutions `__: - - Yan Chirino + - Yan Chirino Maintainers ----------- diff --git a/subscription_oca/models/sale_subscription.py b/subscription_oca/models/sale_subscription.py index b53cbe55b9..747a2f06af 100644 --- a/subscription_oca/models/sale_subscription.py +++ b/subscription_oca/models/sale_subscription.py @@ -7,7 +7,8 @@ from markupsafe import Markup from odoo import Command, api, fields, models -from odoo.exceptions import AccessError +from odoo.exceptions import AccessError, UserError +from odoo.tools.misc import format_date, get_lang logger = logging.getLogger(__name__) @@ -376,6 +377,31 @@ def _prepare_account_move(self, line_ids): values["journal_id"] = self.journal_id.id return values + def _get_existing_invoice_for_period(self, period_start, period_end): + self.ensure_one() + line = self.env["account.move.line"].search( + [ + ("subscription_id", "=", self.id), + ("subscription_period_start", "=", period_start), + ("subscription_period_end", "=", period_end), + ("move_id.state", "!=", "cancel"), + ], + limit=1, + ) + return line.move_id + + def _can_create_invoice_for_period(self, period_start, period_end): + self.ensure_one() + return not self._get_existing_invoice_for_period(period_start, period_end) + + def _format_period_for_message(self, period_start, period_end): + self.ensure_one() + lang_code = get_lang(self.env, self.partner_id.lang).code + return ( + format_date(self.env, period_start, lang_code=lang_code), + format_date(self.env, period_end, lang_code=lang_code), + ) + def create_invoice(self): if not self.env["account.move"].has_access("create"): try: @@ -411,6 +437,19 @@ def create_sale_order(self): return order_id def generate_invoice(self): + period_start, period_end = self._get_invoice_period() + if not self._can_create_invoice_for_period(period_start, period_end): + start_str, end_str = self._format_period_for_message( + period_start, period_end + ) + logger.info( + "Subscription %s: an invoice already exists " + "for the period %s - %s, skipping", + self.id, + start_str, + end_str, + ) + return False invoice_number = "" message_body = "" msg_static = self.env._("Created invoice with reference") @@ -449,6 +488,18 @@ def generate_invoice(self): self.message_post(body=Markup(message_body)) def manual_invoice(self): + period_start, period_end = self._get_invoice_period() + if not self._can_create_invoice_for_period(period_start, period_end): + start_str, end_str = self._format_period_for_message( + period_start, period_end + ) + raise UserError( + self.env._( + "An invoice already exists for the period %(start)s - %(end)s.", + start=start_str, + end=end_str, + ) + ) invoice_id = self.create_invoice() self._set_next_invoice_date_after_invoice() context = dict(self.env.context) diff --git a/subscription_oca/readme/USAGE.md b/subscription_oca/readme/USAGE.md index 8f484f45d0..fad539a48a 100644 --- a/subscription_oca/readme/USAGE.md +++ b/subscription_oca/readme/USAGE.md @@ -36,3 +36,11 @@ Invoice and delivery addresses: to the sale orders generated by the subscription. - The fields are shown when the *Customer Addresses* setting (group *Display Delivery / Invoice addresses*) is enabled. + +Duplicate invoice prevention: + +- A subscription will not be invoiced twice for the same period. If a + non-cancelled invoice already exists for the period being billed, + *Create Invoice* raises an error and the cron skips the + subscription, so an interrupted and re-run batch cannot duplicate + invoices. Cancelled invoices do not block re-invoicing. diff --git a/subscription_oca/static/description/index.html b/subscription_oca/static/description/index.html index 5fcfbe2af6..87d2a50529 100644 --- a/subscription_oca/static/description/index.html +++ b/subscription_oca/static/description/index.html @@ -372,7 +372,7 @@

Subscription management

!! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -!! source digest: sha256:ea96c858a40ee527d0e32ee03859918c082521f205ccc1cb26e96cb3bef27800 +!! source digest: sha256:f284a98a2170cda678d0f33678999997099c9b35bafd0705c49a659120f75f87 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->

Beta License: AGPL-3 OCA/contract Translate me on Weblate Try me on Runboat

This module allows creating subscriptions that generate recurring @@ -433,6 +433,14 @@

Usage

  • The fields are shown when the Customer Addresses setting (group Display Delivery / Invoice addresses) is enabled.
    • +

    Duplicate invoice prevention:

    +
      +
    • A subscription will not be invoiced twice for the same period. If a +non-cancelled invoice already exists for the period being billed, +Create Invoice raises an error and the cron skips the subscription, +so an interrupted and re-run batch cannot duplicate invoices. +Cancelled invoices do not block re-invoicing.
      • +

    Known issues / Roadmap

    diff --git a/subscription_oca/tests/__init__.py b/subscription_oca/tests/__init__.py index db7813c497..973a272307 100644 --- a/subscription_oca/tests/__init__.py +++ b/subscription_oca/tests/__init__.py @@ -4,3 +4,4 @@ from . import test_subscription_security from . import test_subscription_recurrence_dates from . import test_subscription_partner_addresses +from . import test_subscription_duplicate_invoices diff --git a/subscription_oca/tests/test_subscription_duplicate_invoices.py b/subscription_oca/tests/test_subscription_duplicate_invoices.py new file mode 100644 index 0000000000..3a57c6176a --- /dev/null +++ b/subscription_oca/tests/test_subscription_duplicate_invoices.py @@ -0,0 +1,129 @@ +# Copyright 2026 Domatix - Alvaro Domatix +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). + +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 TestSubscriptionDuplicateInvoices(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": "Dup partner"}) + cls.pricelist = cls.env["product.pricelist"].create({"name": "Dup pricelist"}) + cls.template = cls.env["sale.subscription.template"].create( + { + "name": "Dup template", + "code": "DUP-MTH", + "recurring_rule_type": "months", + "recurring_rule_boundary": "unlimited", + "invoicing_mode": "draft", + } + ) + cls.product = cls._create_product( + name="Dup product", + lst_price=50.0, + subscribable=True, + uom_id=cls.uom_unit.id, + ) + + def _make_subscription(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(), + } + ) + self.env["sale.subscription.line"].create( + { + "sale_subscription_id": sub.id, + "product_id": self.product.id, + } + ) + return sub + + def _rewind_to_invoiced_period(self, sub, period_start): + """Put ``recurring_next_date`` back to the period that was just billed. + + This reproduces the real situation the guard protects against: a run + created (and committed) the invoice for that period but was interrupted + before advancing ``recurring_next_date`` (e.g. the cron crashed, or the + email step failed after the invoice was posted). On the next run the + subscription is still due for the *same* period and must not be billed + twice. + """ + sub.recurring_next_date = period_start + + def test_draft_invoice_blocks_duplicate(self): + sub = self._make_subscription() + period_start = sub.recurring_next_date + sub.manual_invoice() + self._rewind_to_invoiced_period(sub, period_start) + with self.assertRaises(UserError): + sub.manual_invoice() + + def test_posted_invoice_blocks_duplicate(self): + sub = self._make_subscription() + period_start = sub.recurring_next_date + invoice = sub.create_invoice() + invoice.action_post() + self._rewind_to_invoiced_period(sub, period_start) + with self.assertRaises(UserError): + sub.manual_invoice() + + def test_next_period_is_not_blocked(self): + # After a normal invoice the date advances to the next period, which is + # a different (not-yet-billed) period and must be allowed. + sub = self._make_subscription() + sub.manual_invoice() + period_start, period_end = sub._get_invoice_period() + self.assertTrue(sub._can_create_invoice_for_period(period_start, period_end)) + invoice = sub.manual_invoice() + self.assertTrue(invoice) + + def test_cancelled_invoice_does_not_block(self): + sub = self._make_subscription() + period_start = sub.recurring_next_date + invoice = sub.create_invoice() + invoice.button_cancel() + self._rewind_to_invoiced_period(sub, period_start) + period_start, period_end = sub._get_invoice_period() + self.assertTrue(sub._can_create_invoice_for_period(period_start, period_end)) + + def test_can_create_invoice_for_fresh_period(self): + sub = self._make_subscription() + period_start, period_end = sub._get_invoice_period() + self.assertTrue(sub._can_create_invoice_for_period(period_start, period_end)) + + def test_user_error_message_contains_period(self): + sub = self._make_subscription() + period_start = sub.recurring_next_date + sub.manual_invoice() + self._rewind_to_invoiced_period(sub, period_start) + with self.assertRaises(UserError) as ctx: + sub.manual_invoice() + self.assertIn("already exists", str(ctx.exception)) + + def test_generate_invoice_skips_duplicate(self): + sub = self._make_subscription() + period_start = sub.recurring_next_date + sub.manual_invoice() + invoices_before = self.env["account.move"].search_count( + [("subscription_id", "=", sub.id)] + ) + self._rewind_to_invoiced_period(sub, period_start) + with mute_logger("odoo.addons.subscription_oca.models.sale_subscription"): + result = sub.generate_invoice() + self.assertFalse(result) + invoices_after = self.env["account.move"].search_count( + [("subscription_id", "=", sub.id)] + ) + self.assertEqual(invoices_before, invoices_after) diff --git a/subscription_oca/tests/test_subscription_oca.py b/subscription_oca/tests/test_subscription_oca.py index 29d301ee1f..d35cf96be1 100644 --- a/subscription_oca/tests/test_subscription_oca.py +++ b/subscription_oca/tests/test_subscription_oca.py @@ -728,6 +728,7 @@ def _collect_all_sub_test_results(self, subscription): test_res.append(sale_order) move_id = subscription.create_invoice() test_res.append(move_id) + subscription.calculate_recurring_next_date(subscription.recurring_next_date) res = subscription.manual_invoice() test_res.append(res["type"]) inv_ids = self.env["account.move"].search(