fix: emit credit notes in CreditNote-XSD element order via a typed model - #102
Open
alvarolivie wants to merge 5 commits into
Open
fix: emit credit notes in CreditNote-XSD element order via a typed model#102alvarolivie wants to merge 5 commits into
alvarolivie wants to merge 5 commits into
Conversation
UBL's Invoice and CreditNote schemas sequence their elements differently: cbc:TaxPointDate precedes the type code, there is no cbc:DueDate, the document-reference block is reordered, cac:AllowanceCharge follows the exchange rates rather than preceding them, and cac:ProjectReference / cac:PrepaidPayment / cac:WithholdingTaxTotal do not exist. A single shared struct emitted invoice-ordered XML for both, producing structurally invalid credit notes whenever those elements were present. Model the two documents as the distinct types their XSDs describe: - Extract the element runs that are identical in both schemas into embedded structs (documentHeader, documentCurrency, documentParties) so the shared ~40 fields live once and cannot drift. - Keep Invoice as the builder/parser (and order-independent parse target for both document kinds); add a CreditNote type whose fields follow the UBL-CreditNote-2.1 sequence. - Map Invoice -> CreditNote at the marshalling boundary; invoice-only fields are dropped by construction. Nothing rewrites the XML after marshalling. Requires xmlctx embedded-struct decoding support (invopop/xmlctx#3); go.mod is pinned to that branch pending its release tag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 80.91% 81.13% +0.22%
==========================================
Files 28 29 +1
Lines 2117 2163 +46
==========================================
+ Hits 1713 1755 +42
- Misses 267 271 +4
Partials 137 137 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes structurally invalid UBL CreditNote XML caused by reusing the Invoice struct field order for both document types. It introduces a dedicated CreditNote typed model (in correct XSD element sequence) and remaps Invoice → CreditNote at the marshalling boundary so encoding/xml naturally emits the right element ordering without post-processing bytes.
Changes:
- Route
Bytes/BytesCompactthrough amarshalDocumentadapter that remaps credit-noteInvoiceinstances to aCreditNotestruct before marshalling. - Refactor
Invoiceinto embedded “shared runs” (documentHeader,documentCurrency,documentParties) and add a newCreditNotetype with CreditNote-XSD field ordering. - Add an end-to-end ordering/leakage test for credit note marshalling; update existing tests impacted by the new embedded layout; bump
xmlctxto a pseudo-version.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
ubl.go |
Uses marshalDocument to ensure credit notes marshal via the new CreditNote layout. |
invoice.go |
Splits shared field runs into embedded structs and keeps Invoice as the main build/parse type. |
creditnote.go |
Adds CreditNote model, toCreditNote() projection, and marshalDocument() adapter. |
creditnote_test.go |
Adds test asserting CreditNote element ordering and absence of invoice-only fields. |
extension_test.go |
Updates initialization patterns impacted by embedded struct promotion. |
signature_test.go |
Updates initialization patterns impacted by embedded struct promotion. |
go.mod / go.sum |
Pins xmlctx to a pseudo-version needed for embedded unmarshalling behavior. |
.gitignore |
Adds go.work.sum to ignored workspace artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+102
to
+106
| type Invoice struct { | ||
| XMLName xml.Name | ||
| documentHeader | ||
|
|
||
| DueDate string `xml:"cbc:DueDate,omitempty"` |
Comment on lines
+20
to
+22
| inv := &ubl.Invoice{XMLName: xml.Name{Local: "CreditNote"}} | ||
| inv.ID = "CN-1" | ||
| inv.IssueDate = "2024-02-14" |
Comment on lines
16
to
20
| github.com/invopop/gobl.sa.zatca v0.0.2 | ||
| github.com/invopop/phive v0.6.0 | ||
| github.com/invopop/validation v0.8.0 | ||
| github.com/invopop/xmlctx v0.13.0 | ||
| github.com/invopop/xmlctx v0.13.1-0.20260702150626-9900fb876d77 | ||
| github.com/invopop/xmldsig v0.14.0 |
| // It doubles as the parse target for **both** Invoice and CreditNote XML: | ||
| // unmarshalling is order-independent, so the extra CreditNote-only fields it | ||
| // carries (CreditNoteTypeCode, CreditNoteLines) are populated when a credit note | ||
| // is parsed and simply stay empty — and therefore unmarshalled — when marshalling |
Point at the updated invopop/xmlctx#3 commit (lint fixes + tighter comments); still a branch pseudo-version pending the release tag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Export the shared embed types (DocumentHeader/DocumentCurrency/ DocumentParties) so external callers can build documents with keyed composite literals again. - Fix a pre-existing comment typo and correct the Invoice doc wording (fields are omitted, not "unmarshalled", when marshalling an invoice). - Set namespaces in the credit-note ordering test and assert the output is well-formed XML with all prefixes bound. - Re-pin xmlctx to the branch head carrying the embed-decoding support. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed the Copilot review in
|
Comment on lines
+99
to
+101
| // is parsed and simply stay empty — and therefore omitted — when marshalling | ||
| // a real invoice. Marshalling a credit note goes through CreditNote (see | ||
| // creditnote.go), which lays the divergent elements out in CreditNote-XSD order. |
| github.com/invopop/phive v0.6.0 | ||
| github.com/invopop/validation v0.8.0 | ||
| github.com/invopop/xmlctx v0.13.0 | ||
| github.com/invopop/xmlctx v0.13.1-0.20260702153213-8edb2c07e8d5 |
Add xsd_validation_test.go, which validates every convert golden against the maindoc UBL-Invoice / UBL-CreditNote schemas (guarded to skip when a Java validator isn't available), plus a differential test proving the credit note is only schema-valid in CreditNote-XSD order: the previous invoice-ordered layout is rejected because cbc:TaxPointDate lands after cbc:CreditNoteTypeCode, while the shipped path validates. Add a credit-note fixture exercising the divergent elements (TaxPointDate, document-level allowance + charge, a billing reference) and its golden. Re-pin xmlctx to the branch head carrying the embedded-decode support, the encoding/xml promotion-rule parity, and the special-field tag-matching fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the optional cbc:AccountingCostCode element to DocumentCurrency in its XSD position (between PaymentAlternativeCurrencyCode and AccountingCost) so the shared run matches the UBL 2.1 sequence exactly. - Document how to construct an Invoice now that the shared fields are promoted from embedded types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed the second review pass:
For added confidence this refactor is correct, this branch now also validates every generated golden against the official UBL 2.1 XSDs ( |
Comment on lines
+56
to
+58
| if _, err := exec.LookPath("java"); err != nil { | ||
| t.Skip("java not available, skipping XSD validation") | ||
| } |
Comment on lines
+59
to
+61
| if _, err := exec.LookPath("javac"); err != nil { | ||
| t.Skip("javac not available, skipping XSD validation") | ||
| } |
Comment on lines
+41
to
+45
| // Reference block: the credit note orders Contract/Additional ahead of | ||
| // Statement/Originator (the invoice does the reverse). | ||
| inv.ContractDocumentReference = []ubl.Reference{{ID: ubl.IDType{Value: "CONTRACT-1"}}} | ||
| inv.StatementDocumentReference = []ubl.Reference{{ID: ubl.IDType{Value: "STATEMENT-1"}}} | ||
| inv.OriginatorDocumentReference = []ubl.Reference{{ID: ubl.IDType{Value: "ORIGINATOR-1"}}} |
Comment on lines
+83
to
+85
| "<cac:ContractDocumentReference>", | ||
| "<cac:StatementDocumentReference>", | ||
| "<cac:OriginatorDocumentReference>", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
UBL's
InvoiceandCreditNoteXSDs sequence their elements differently, yet this package marshalled both from one shared struct in invoice order. Confirmed against the official UBL-2.1 schemas, the credit note diverges in several places:cbc:TaxPointDateprecedes the type code (invoice: type code first)cbc:DueDateContract/AdditionalbeforeStatement/Originator)cac:AllowanceChargefollows the exchange rates (invoice: precedes them)cac:ProjectReference,cac:PrepaidPayment,cac:WithholdingTaxTotaldo not existSince
AllowanceCharge, the document references, andProjectReference/PrepaidPaymentare all populated by the builder, a shared struct emits structurally invalid credit-note XML the moment those elements are present. This supersedes #101, which addressed only theTaxPointDatecase by rewriting the marshalled bytes.Approach — model the two documents as the distinct types they are
encoding/xmlemits fields in declaration order, so correct output falls out of a correct type. No bytes are touched after marshalling.documentHeader,documentCurrency,documentParties— so the ~40 shared fields are declared once and can't drift.Invoicekeeps all builder/parser logic and, because unmarshalling is order-independent, doubles as the parse target for both document kinds.CreditNotetype declares its divergent fields in exact UBL-CreditNote-2.1 order.toCreditNote()mapsInvoice→CreditNoteat theBytes/BytesCompactboundary; invoice-only fields are dropped by construction.Tests
TestCreditNoteMarshalOrderingasserts every divergence end-to-end:TaxPointDatebefore the type code, the reordered reference block,AllowanceChargeafter the exchange rates, and thatDueDate/InvoiceTypeCode/ProjectReference/PrepaidPayment/WithholdingTaxTotal/InvoiceLinenever appear in a credit note.golangci-lint0 issues.Dependency⚠️
Parsing into embedded structs needs anonymous-embed decoding support in xmlctx: invopop/xmlctx#3.
go.modis currently pinned to that branch's pseudo-version so CI is green.Before merge: merge xmlctx#3, tag it (e.g.
v0.14.0), then `go get github.com/invopop/xmlctx@v0.14.0` and updatego.mod/go.sum.Not included (deliberate follow-up)
The
if CreditNoteTypeCode != nilbehaviour branches in build/parse remain. Retiring them means a real behaviour split (separate builders/parsers) and is better as its own PR; this change is purely the structural/ordering fix.🤖 Generated with Claude Code