Append non-grant advances to PaymentsOnAccount#19
Open
methodofaction wants to merge 3 commits into
Open
Conversation
setAdvances constructed a PaymentOnAccount struct for each non-grant advance but never appended it to the regular slice, so the PaymentsOnAccount block was always omitted from the XML even when advances existed. Add the missing append. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Golden-file test that exercises a non-grant advance so the PaymentsOnAccount block is emitted. Without the append fix in setAdvances, the generated XML omits the block and this test fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes Facturae XML generation for invoices that include non-grant advances by ensuring <PaymentsOnAccount> is emitted when appropriate, and adds a regression fixture pair to cover the scenario end-to-end.
Changes:
- Append non-grant
PaymentOnAccountentries inInvoiceTotals.setAdvancesso<PaymentsOnAccount>is included in generated XML. - Add a new GOBL input fixture (
invoice-with-advance.json) exercising advances + due dates. - Add the corresponding expected XML output fixture (
out/invoice-with-advance.xml) containing<PaymentsOnAccount>.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
invoice_totals.go |
Fixes omission of non-grant advances from the PaymentsOnAccount slice, enabling correct XML output. |
test/data/invoice-with-advance.json |
New test fixture input that includes an advance payment and explicit payment terms. |
test/data/out/invoice-with-advance.xml |
New expected XML output verifying <PaymentsOnAccount> appears and totals reflect the advance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Facturae's XSD requires PaymentOnAccountDate (no minOccurs=0). When a non-grant advance has no date set, the previous code left the zero cal.Date value, which would marshal as 0001-01-01 and fail XSD validation. Default PaymentOnAccountDate to the invoice's IssueDate and override with the advance's date when present — an advance has necessarily been paid by the time the invoice is issued, so this is a safe upper bound. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
copilot complained about the date being set to years zero if an advance date did not exist. I'm not sure if this guard is necessary as we may require it upstream. |
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.
Summary
setAdvancesbuilt eachPaymentOnAccountstruct but never appended it to the slice, so the<PaymentsOnAccount>block was always omitted from the XML even when non-grant advances existed on the invoice. Add the missingappend.Extracted from #16 (issue #1 of that PR's three bundled fixes).
Test plan
go test .passes with the newinvoice-with-advancefixturego test -tags xsdvalidate .passes (XML is XSD-valid against Facturae v3.2.2)appendmakes the fixture test fail (missing<PaymentsOnAccount>block)🤖 Generated with Claude Code