Skip to content

[17.0][FIX] account_invoice_facturx: fix Factur-X 4.x schematron errors #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tendil wants to merge 2 commits into
base: 17.0
Choose a base branch
from
Open

[17.0][FIX] account_invoice_facturx: fix Factur-X 4.x schematron errors #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a436c4
[FIX] account_invoice_facturx: fix Factur-X 4.x validation
tendil Apr 7, 2026
7479b80
[FIX] account_invoice_facturx: add delivery date to Factur-X XML
tendil Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 45 additions & 17 deletions account_invoice_facturx/models/account_move.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ def _cii_add_trade_contact_block(self, partner, parent_node, ns):
email_node = etree.SubElement(
trade_contact, ns["ram"] + "EmailURIUniversalCommunication"
)
email_uriid = etree.SubElement(
email_node, ns["ram"] + "URIID", schemeID="SMTP"
)
email_uriid = etree.SubElement(email_node, ns["ram"] + "URIID")
email_uriid.text = partner.email

@api.model
Expand Down Expand Up @@ -270,6 +268,15 @@ def _get_contract_code(self):
So it's difficult to have a common datamodel for it"""
return False

def _cii_get_delivery_date(self):
"""Return the delivery/service date to export in Factur-X XML.

Designed to be inherited by modules that store a dedicated delivery
or service date on invoices.
"""
self.ensure_one()
return self.invoice_date

def _cii_add_trade_delivery_block(self, trade_transaction, ns):
self.ensure_one()
trade_agreement = etree.SubElement(
Expand All @@ -283,6 +290,12 @@ def _cii_add_trade_delivery_block(self, trade_transaction, ns):
self._cii_add_address_block(
self.partner_shipping_id, shipto_trade_party, ns
)
delivery_date = self._cii_get_delivery_date()
if ns["level"] in PROFILES_EN_UP and delivery_date:
delivery_event = etree.SubElement(
trade_agreement, ns["ram"] + "ActualDeliverySupplyChainEvent"
)
self._cii_add_date("OccurrenceDateTime", delivery_date, delivery_event, ns)
return trade_agreement

def _cii_add_trade_settlement_payment_means_block(self, trade_settlement, ns):
Expand Down Expand Up @@ -320,23 +333,38 @@ def _cii_add_trade_settlement_payment_means_block(self, trade_settlement, ns):
and self.payment_mode_id.fixed_journal_id
):
partner_bank = self.payment_mode_id.fixed_journal_id.bank_account_id
if partner_bank and partner_bank.acc_type == "iban":
payment_means_bank_account = etree.SubElement(
payment_means, ns["ram"] + "PayeePartyCreditorFinancialAccount"
if not partner_bank or not partner_bank.sanitized_acc_number:
raise UserError(
_(
"Missing bank account identifier on invoice '%s'. "
"Factur-X requires either an IBAN or a proprietary "
"account identifier (BT-84) for credit transfer "
"payment means."
)
% (self.display_name or self.name)
)
iban = etree.SubElement(

payment_means_bank_account = etree.SubElement(
payment_means, ns["ram"] + "PayeePartyCreditorFinancialAccount"
)
if partner_bank.acc_type == "iban":
account_identifier = etree.SubElement(
payment_means_bank_account, ns["ram"] + "IBANID"
)
iban.text = partner_bank.sanitized_acc_number
if ns["level"] in PROFILES_EN_UP and partner_bank.bank_bic:
payment_means_bank = etree.SubElement(
payment_means,
ns["ram"] + "PayeeSpecifiedCreditorFinancialInstitution",
)
payment_means_bic = etree.SubElement(
payment_means_bank, ns["ram"] + "BICID"
)
payment_means_bic.text = partner_bank.bank_bic
else:
account_identifier = etree.SubElement(
payment_means_bank_account, ns["ram"] + "ProprietaryID"
)
account_identifier.text = partner_bank.sanitized_acc_number
if ns["level"] in PROFILES_EN_UP and partner_bank.bank_bic:
payment_means_bank = etree.SubElement(
payment_means,
ns["ram"] + "PayeeSpecifiedCreditorFinancialInstitution",
)
payment_means_bic = etree.SubElement(
payment_means_bank, ns["ram"] + "BICID"
)
payment_means_bic.text = partner_bank.bank_bic
# Field mandate_id provided by the OCA module account_banking_mandate
elif (
payment_means_code.text in DIRECT_DEBIT_CODES
Expand Down
75 changes: 75 additions & 0 deletions account_invoice_facturx/tests/test_facturx_invoice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,20 @@
# @author: Alexis de Lattre <alexis.delattre@akretion.com>
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

from unittest.mock import patch

from facturx import get_facturx_level
from lxml import etree

from odoo.exceptions import UserError
Comment thread
tendil marked this conversation as resolved.
from odoo.tests.common import TransactionCase

RAM_NS = (
"urn:un:unece:uncefact:data:standard:"
"ReusableAggregateBusinessInformationEntity:100"
)
NSMAP = {"ram": RAM_NS}


class TestFacturXInvoice(TransactionCase):
@classmethod
Expand All @@ -16,6 +25,14 @@ def setUpClass(cls):
cls.company = cls.env.ref("base.main_company")
cls.product1 = cls.env.ref("product.product_product_4")
cls.product2 = cls.env.ref("product.product_product_1")
cls.env.user.partner_id.email = "billing@example.com"
cls.proprietary_bank = cls.env["res.partner.bank"].create(
{
"partner_id": cls.company.partner_id.id,
"acc_number": "ACC-FACTURX-0001",
"acc_type": "bank",
}
)
sale_taxes = cls.env["account.tax"].search(
[
("company_id", "=", cls.company.id),
Expand All @@ -37,6 +54,7 @@ def setUpClass(cls):
"move_type": "out_invoice",
"partner_id": cls.env.ref("base.res_partner_2").id,
"currency_id": cls.company.currency_id.id,
"partner_bank_id": cls.proprietary_bank.id,
"invoice_line_ids": [
(
0,
Expand All @@ -60,6 +78,14 @@ def setUpClass(cls):
}
)
cls.invoice.action_post()
cls.invoice.partner_bank_id = cls.proprietary_bank
Comment thread
tendil marked this conversation as resolved.

def _generate_xml_root(self, invoice=None, level="en16931"):
invoice = invoice or self.invoice
self.company.write({"facturx_level": level})
xml_bytes, fx_level = invoice.generate_facturx_xml()
self.assertEqual(fx_level, level)
return etree.fromstring(xml_bytes)

def test_deep_customer_invoice(self):
# Bug in Basic XSD: missing CountrySubDivisionName
Expand Down Expand Up @@ -91,3 +117,52 @@ def test_deep_customer_invoice(self):
xml_root = etree.fromstring(xml_bytes)
facturx_level = get_facturx_level(xml_root)
self.assertEqual(facturx_level, level)

def test_email_uriid_has_no_schemeid(self):
xml_root = self._generate_xml_root(level="en16931")
uriid_nodes = xml_root.xpath(
"//ram:DefinedTradeContact/"
"ram:EmailURIUniversalCommunication/"
"ram:URIID",
namespaces=NSMAP,
)
self.assertTrue(uriid_nodes, "Expected seller email URIID in EN16931 XML")
self.assertEqual(uriid_nodes[0].text, "billing@example.com")
self.assertNotIn("schemeID", uriid_nodes[0].attrib)

def test_credit_transfer_uses_proprietary_id_for_non_iban_account(self):
xml_root = self._generate_xml_root(level="en16931")
proprietary_nodes = xml_root.xpath(
"//ram:SpecifiedTradeSettlementPaymentMeans/"
"ram:PayeePartyCreditorFinancialAccount/"
"ram:ProprietaryID",
namespaces=NSMAP,
)
iban_nodes = xml_root.xpath(
"//ram:SpecifiedTradeSettlementPaymentMeans/"
"ram:PayeePartyCreditorFinancialAccount/"
"ram:IBANID",
namespaces=NSMAP,
)

self.assertTrue(
proprietary_nodes,
"Expected ProprietaryID for non-IBAN creditor account",
)
self.assertEqual(
proprietary_nodes[0].text,
self.proprietary_bank.sanitized_acc_number,
)
self.assertFalse(iban_nodes, "IBANID should not be generated for non-IBAN bank")

Comment on lines +133 to +157
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add a companion regression for the IBAN branch.

This change rewires both sides of the identifier selection, but the new coverage only asserts the non-IBAN path. A small IBAN fixture/assertion would keep ram:IBANID protected too.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@account_invoice_facturx/tests/test_facturx_invoice.py` around lines 133 -
157, Add a complementary test to cover the IBAN branch: create an XML root using
the IBAN fixture (e.g., mirror
test_credit_transfer_uses_proprietary_id_for_non_iban_account but using the IBAN
bank setup such as self.iban_bank), query the same XPath expressions (ram:IBANID
and ram:ProprietaryID using NSMAP), assert that ram:IBANID nodes are present and
ram:IBANID.text equals the expected sanitized IBAN value, and assert that
ram:ProprietaryID is not present for the IBAN case (use the same variables
xml_root, iban_nodes, proprietary_nodes to keep consistency).

def test_credit_transfer_requires_account_identifier(self):
invoice = self.invoice.copy(default={"partner_bank_id": False})
invoice.action_post()
Comment on lines +159 to +160
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Clear partner_bank_id after posting as well.

action_post() can recompute stored partner_bank_id, so setting it to False only in copy(default=...) does not reliably preserve the missing-account scenario. That makes this regression prone to posting a valid bank back onto the invoice instead of exercising the new early guard.

Suggested tightening 
     invoice = self.invoice.copy(default={"partner_bank_id": False})
     invoice.action_post()
+    invoice.partner_bank_id = False

Based on learnings: In Odoo 17, partner_bank_id on account.move is a stored computed field (compute='_compute_partner_bank_id'). Calling action_post() can recompute and overwrite the value set at invoice creation time.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@account_invoice_facturx/tests/test_facturx_invoice.py` around lines 159 -
160, The test currently clears partner_bank_id only via
copy(default={"partner_bank_id": False}) but action_post() can recompute and
restore it; after creating the invoice (variable invoice) and before/after
calling invoice.action_post() explicitly ensure invoice.partner_bank_id is set
to False (i.e., assign False to the partner_bank_id field on the account.move
instance) so the missing-account scenario is preserved; locate the usage around
invoice.copy(...) and invoice.action_post() and add a direct assignment to
partner_bank_id on that invoice instance to clear it again.


self.company.write({"facturx_level": "en16931"})
with patch(
"odoo.addons.account_invoice_facturx.models.account_move.xml_check_xsd"
) as xml_check_xsd:
with self.assertRaises(UserError):
invoice.generate_facturx_xml()
xml_check_xsd.assert_not_called()
Loading