UBL fixes #852

stephanstapel · 2025-12-16T22:57:52Z

Summary

Some fixes to UBL reading and writing

Checklist

Unit tests added/updated

Breaking changes?

No
Yes (explain impact & migration)

closes #849

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-12-16T23:01:48Z

ZUGFeRD/InvoiceDescriptor22UBLWriter.cs

+                if ((party.SpecifiedLegalOrganization != null) || !String.IsNullOrWhiteSpace(party.Description))
                {
-                    //Party legal registration identifier (BT-30)
-                    _Writer.WriteElementString("cbc", "CompanyID", party.GlobalID.ID);
-                }
+                    writer.WriteStartElement("cac", "PartyLegalEntity");

-                if (party.Description != null)
-                {
-                    //Party additional legal information (BT-33)
-                    _Writer.WriteElementString("cbc", "CompanyLegalForm", party.Description);
-                }
+                    if (party.SpecifiedLegalOrganization != null)


Do not drop PartyLegalEntity when no legal org is set

The UBL writer now wraps the entire PartyLegalEntity block in a check for SpecifiedLegalOrganization or Description only. Invoices that only populate the party name (and possibly a GlobalID, which was previously written here) now emit no cac:PartyLegalEntity/cbc:RegistrationName at all, even though EN16931 requires the registration name for accounting parties. Such invoices will produce non-compliant UBL output and omit seller/buyer identifiers whenever the descriptor doesn’t also set SpecifiedLegalOrganization or Description.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This pull request implements several fixes for UBL (Universal Business Language) reading and writing functionality, focusing on improving null safety, implementing business rules, and enhancing party identification handling.

Fixed null reference handling in SpecifiedLineTradeSettlement processing
Implemented business rule BR-O-05 for tax category code 'O'
Enhanced party identification with scheme ID support for buyer/seller organizations

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

File	Description
ZUGFeRD/InvoiceDescriptor23CIIReader.cs	Added null check for `SpecifiedLineTradeSettlement` to prevent processing when the node doesn't exist; improved code formatting
ZUGFeRD/InvoiceDescriptor22UBLWriter.cs	Implemented BR-O-05 rule for tax category 'O', added scheme ID attributes for party identifications, refactored `PartyLegalEntity` writing to use `SpecifiedLegalOrganization`
ZUGFeRD.Test/ZUGFeRD22Tests.cs	Updated test data file path to use newer EXTENDED example
ZUGFeRD.Test/XRechnungUBLTests.cs	Added comprehensive test for buyer/seller scheme IDs in UBL format, verifying round-trip serialization/deserialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ZUGFeRD/InvoiceDescriptor22UBLWriter.cs

ZUGFeRD.Test/XRechnungUBLTests.cs

Copilot · 2025-12-16T23:05:00Z

ZUGFeRD.Test/XRechnungUBLTests.cs

+
+            desc.Buyer.SpecifiedLegalOrganization = new LegalOrganization(
+                GlobalIDSchemeIdentifiers.DUNS,
+                "123456789",
+                "Buyer Company Ltd.");            
+            desc.Seller.SpecifiedLegalOrganization = new LegalOrganization(
+                GlobalIDSchemeIdentifiers.AbnScheme,
+                "987654321",
+                "Seller Company Ltd.");
+
+
+            MemoryStream ms = new MemoryStream();
+            desc.Save(ms, ZUGFeRDVersion.Version23, Profile.XRechnung, ZUGFeRDFormats.UBL);
+            ms.Seek(0, SeekOrigin.Begin);
+            InvoiceDescriptor loadedInvoice = InvoiceDescriptor.Load(ms);
+
+            Assert.AreEqual(loadedInvoice.Buyer.ID.SchemeID, GlobalIDSchemeIdentifiers.DUNS);
+            Assert.AreEqual(loadedInvoice.Buyer.ID.ID, "123456789");            
+            Assert.AreEqual(loadedInvoice.Seller.ID.SchemeID, GlobalIDSchemeIdentifiers.AbnScheme);
+            Assert.AreEqual(loadedInvoice.Seller.ID.ID, "987654321");
+


This line contains trailing whitespace. Consider removing it to maintain code cleanliness.

Suggested change

desc.Buyer.SpecifiedLegalOrganization = new LegalOrganization(

GlobalIDSchemeIdentifiers.DUNS,

"123456789",

"Buyer Company Ltd.");

desc.Seller.SpecifiedLegalOrganization = new LegalOrganization(

GlobalIDSchemeIdentifiers.AbnScheme,

"987654321",

"Seller Company Ltd.");

MemoryStream ms = new MemoryStream();

desc.Save(ms, ZUGFeRDVersion.Version23, Profile.XRechnung, ZUGFeRDFormats.UBL);

ms.Seek(0, SeekOrigin.Begin);

InvoiceDescriptor loadedInvoice = InvoiceDescriptor.Load(ms);

Assert.AreEqual(loadedInvoice.Buyer.ID.SchemeID, GlobalIDSchemeIdentifiers.DUNS);

Assert.AreEqual(loadedInvoice.Buyer.ID.ID, "123456789");

Assert.AreEqual(loadedInvoice.Seller.ID.SchemeID, GlobalIDSchemeIdentifiers.AbnScheme);

Assert.AreEqual(loadedInvoice.Seller.ID.ID, "987654321");

desc.Buyer.SpecifiedLegalOrganization = new LegalOrganization(

GlobalIDSchemeIdentifiers.DUNS,

"123456789",

"Buyer Company Ltd.");

desc.Seller.SpecifiedLegalOrganization = new LegalOrganization(

GlobalIDSchemeIdentifiers.AbnScheme,

"987654321",

"Seller Company Ltd.");

MemoryStream ms = new MemoryStream();

desc.Save(ms, ZUGFeRDVersion.Version23, Profile.XRechnung, ZUGFeRDFormats.UBL);

ms.Seek(0, SeekOrigin.Begin);

InvoiceDescriptor loadedInvoice = InvoiceDescriptor.Load(ms);

Assert.AreEqual(loadedInvoice.Buyer.ID.SchemeID, GlobalIDSchemeIdentifiers.DUNS);

Assert.AreEqual(loadedInvoice.Buyer.ID.ID, "123456789");

Assert.AreEqual(loadedInvoice.Seller.ID.SchemeID, GlobalIDSchemeIdentifiers.AbnScheme);

Assert.AreEqual(loadedInvoice.Seller.ID.ID, "987654321");

Copilot · 2025-12-16T23:05:01Z

ZUGFeRD/InvoiceDescriptor22UBLWriter.cs

+                        _Writer.WriteStartElement("cbc", "CompanyID");

-                writer.WriteEndElement(); //!PartyLegalEntity
+                        if (party.SpecifiedLegalOrganization.ID.SchemeID.HasValue)
+                        {
+                            _Writer.WriteAttributeString("schemeID", party.SpecifiedLegalOrganization.ID.SchemeID.Value.EnumToString());
+                        }
+                        _Writer.WriteValue(party.SpecifiedLegalOrganization.ID.ID);
+                        _Writer.WriteEndElement(); // !CompanyID


Potential null reference exception when accessing party.SpecifiedLegalOrganization.ID. The code checks if party.SpecifiedLegalOrganization is not null, but doesn't verify that party.SpecifiedLegalOrganization.ID is not null before accessing ID.SchemeID and ID.ID. According to the LegalOrganization class, the ID property can be null (initialized to null in the default constructor). Consider adding a null check for party.SpecifiedLegalOrganization.ID before accessing its properties.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-12-16T23:05:02Z

ZUGFeRD/InvoiceDescriptor23CIIReader.cs

+                foreach (XmlNode invoiceReferencedDocumentNode in tradeLineItem.SelectNodes(".//ram:SpecifiedLineTradeSettlement/ram:InvoiceReferencedDocument", nsmgr))
+                {
+                    // TODO: process
+                }


This assignment to invoiceReferencedDocumentNode is useless, since its value is never read.

Suggested change

foreach (XmlNode invoiceReferencedDocumentNode in tradeLineItem.SelectNodes(".//ram:SpecifiedLineTradeSettlement/ram:InvoiceReferencedDocument", nsmgr))

{

// TODO: process

}

// TODO: process InvoiceReferencedDocument nodes when implementation is defined

Copilot · 2025-12-16T23:05:02Z

ZUGFeRD/InvoiceDescriptor23CIIReader.cs

+                XmlNode applicableTradeTaxNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:ApplicableTradeTax", nsmgr);
+                // TODO: process


This assignment to applicableTradeTaxNode is useless, since its value is never read.

Suggested change

XmlNode applicableTradeTaxNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:ApplicableTradeTax", nsmgr);

// TODO: process

// TODO: process ApplicableTradeTax when requirements are defined

Copilot · 2025-12-16T23:05:02Z

ZUGFeRD/InvoiceDescriptor23CIIReader.cs

-                                                 EnumExtensions.StringToEnum<ChargeReasonCodes>(reasonCode));
-                }
-                else // allowance
+                XmlNode billingSpecifiedPeriodNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:BillingSpecifiedPeriod", nsmgr);


This assignment to billingSpecifiedPeriodNode is useless, since its value is never read.

Copilot · 2025-12-16T23:05:02Z

ZUGFeRD/InvoiceDescriptor23CIIReader.cs

+                XmlNode specifiedTradeSettlementLineMonetarySummationNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:SpecifiedTradeSettlementLineMonetarySummation", nsmgr);
                // TODO: process


This assignment to specifiedTradeSettlementLineMonetarySummationNode is useless, since its value is never read.

Suggested change

XmlNode specifiedTradeSettlementLineMonetarySummationNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:SpecifiedTradeSettlementLineMonetarySummation", nsmgr);

// TODO: process

// TODO: process ram:SpecifiedTradeSettlementLineMonetarySummation if needed

Co-authored-by: Copilot <[email protected]>

Copilot · 2025-12-16T23:19:45Z

@stephanstapel I've opened a new pull request, #853, to work on those changes. Once the pull request is ready, I'll request review from you.

stephanstapel added 2 commits December 16, 2025 23:56

UBL fixes

f355d53

closes #849

test against new test data

6bac71c

Copilot AI review requested due to automatic review settings December 16, 2025 22:57

Copilot started reviewing on behalf of stephanstapel December 16, 2025 22:58 View session

chatgpt-codex-connector bot reviewed Dec 16, 2025

View reviewed changes

Copilot AI reviewed Dec 16, 2025

View reviewed changes

stephanstapel and others added 4 commits December 17, 2025 00:18

Update ZUGFeRD/InvoiceDescriptor22UBLWriter.cs

b15a39d

Co-authored-by: Copilot <[email protected]>

Update ZUGFeRD.Test/XRechnungUBLTests.cs

0e2d448

Co-authored-by: Copilot <[email protected]>

Update ZUGFeRD.Test/XRechnungUBLTests.cs

97d4baf

Co-authored-by: Copilot <[email protected]>

Update ZUGFeRD.Test/XRechnungUBLTests.cs

f80b961

Co-authored-by: Copilot <[email protected]>

Copilot AI mentioned this pull request Dec 16, 2025

[WIP] WIP address feedback from review on UBL fixes PR #853

Closed

6 tasks

stephanstapel merged commit cf5f0ab into master Dec 16, 2025
2 checks passed

		XmlNode applicableTradeTaxNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:ApplicableTradeTax", nsmgr);
		// TODO: process

	XmlNode applicableTradeTaxNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:ApplicableTradeTax", nsmgr);
	// TODO: process
	// TODO: process ApplicableTradeTax when requirements are defined

		XmlNode specifiedTradeSettlementLineMonetarySummationNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:SpecifiedTradeSettlementLineMonetarySummation", nsmgr);
		// TODO: process

	XmlNode specifiedTradeSettlementLineMonetarySummationNode = tradeLineItem.SelectSingleNode(".//ram:SpecifiedLineTradeSettlement/ram:SpecifiedTradeSettlementLineMonetarySummation", nsmgr);
	// TODO: process
	// TODO: process ram:SpecifiedTradeSettlementLineMonetarySummation if needed

Uh oh!

UBL fixes #852

UBL fixes #852

Uh oh!

Conversation

stephanstapel commented Dec 16, 2025

Summary

Checklist

Breaking changes?

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

stephanstapel Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants