Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
==========================================
+ Coverage 93.38% 93.39% +0.01%
==========================================
Files 369 369
Lines 20048 20103 +55
==========================================
+ Hits 18721 18776 +55
Misses 888 888
Partials 439 439 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR extends the Spanish VERIFACTU addon validation rules to better ensure invoices can always be rendered into schema-valid VERIFACTU XML, and updates unit tests + changelog to match.
Changes:
- Added new invoice validation rules (party name length limits, series+code combined length, non-ES tax ID code length, and max number of non-retained tax rates).
- Updated verifactu invoice tests to assert rule IDs/messages and added coverage for the new rules.
- Documented the change in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
data/rules/es-verifactu.json |
Adds the generated rule definitions for the new VERI*FACTU invoice constraints. |
addons/es/verifactu/bill.go |
Implements the new rule checks in Go (series+code length, party name length, non-ES tax ID code length, max non-retained rates). |
addons/es/verifactu/bill_test.go |
Updates existing assertions to include rule IDs and adds tests for the new constraints. |
CHANGELOG.md |
Notes the addition of extra VERI*FACTU schema-validity rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func seriesCodeFits(series, code cbc.Code) bool { | ||
| joined := series.Join(code) | ||
| return len(joined) <= 60 | ||
| } |
There was a problem hiding this comment.
seriesCodeFits uses len(joined) which measures UTF-8 bytes, not characters. Since cbc.Code permits non-ASCII characters (e.g., Ñ), this can reject values that are within a 60-character XSD limit but exceed 60 bytes. Consider counting runes (e.g., utf8.RuneCountInString(joined.String())) to align with schema maxLength semantics.
| t.Run("invoice series and code fit within 60 chars", func(t *testing.T) { | ||
| inv := testInvoiceStandard(t) | ||
| inv.Series = cbc.Code(strings.Repeat("S", 30)) | ||
| inv.Code = cbc.Code(strings.Repeat("C", 30)) | ||
| assertValidationError(t, inv, "[GOBL-ES-VERIFACTU-BILL-INVOICE-20] invoice series and code combined must be 60 characters or less") | ||
| }) | ||
|
|
||
| t.Run("preceding series and code fit within 60 chars", func(t *testing.T) { |
There was a problem hiding this comment.
These test names say the series+code "fit within 60 chars" but the setup expects a validation error. Given cbc.Code.Join inserts a -, the constructed ID is 61 chars (30+1+30) and should fail; please rename the tests to reflect that they exceed the limit (or adjust the lengths and assert success for the boundary case).
| t.Run("invoice series and code fit within 60 chars", func(t *testing.T) { | |
| inv := testInvoiceStandard(t) | |
| inv.Series = cbc.Code(strings.Repeat("S", 30)) | |
| inv.Code = cbc.Code(strings.Repeat("C", 30)) | |
| assertValidationError(t, inv, "[GOBL-ES-VERIFACTU-BILL-INVOICE-20] invoice series and code combined must be 60 characters or less") | |
| }) | |
| t.Run("preceding series and code fit within 60 chars", func(t *testing.T) { | |
| t.Run("invoice series and code exceed 60 chars combined", func(t *testing.T) { | |
| inv := testInvoiceStandard(t) | |
| inv.Series = cbc.Code(strings.Repeat("S", 30)) | |
| inv.Code = cbc.Code(strings.Repeat("C", 30)) | |
| assertValidationError(t, inv, "[GOBL-ES-VERIFACTU-BILL-INVOICE-20] invoice series and code combined must be 60 characters or less") | |
| }) | |
| t.Run("preceding series and code exceed 60 chars combined", func(t *testing.T) { |
| t.Run("preceding series and code fit within 60 chars", func(t *testing.T) { | ||
| inv := testInvoiceStandard(t) | ||
| inv.Type = bill.InvoiceTypeCorrective | ||
| inv.Preceding = []*org.DocumentRef{ | ||
| { | ||
| Series: cbc.Code(strings.Repeat("S", 30)), | ||
| Code: cbc.Code(strings.Repeat("C", 30)), | ||
| }, | ||
| } | ||
| assertValidationError(t, inv, "[GOBL-ES-VERIFACTU-BILL-INVOICE-21] ($.preceding[0]) preceding series and code combined must be 60 characters or less") | ||
| }) |
There was a problem hiding this comment.
This preceding series+code length test triggers other preceding validations too (missing issue_date and, for corrective invoices, tax). That makes the failure less focused and could mask regressions. Consider populating the minimal required preceding fields so the test isolates rule 21.
| func assertValidationError(t *testing.T, inv *bill.Invoice, expected ...string) { | ||
| t.Helper() | ||
| require.NoError(t, inv.Calculate()) | ||
| err := rules.Validate(inv) | ||
| require.ErrorContains(t, err, expected) | ||
| for _, e := range expected { | ||
| require.ErrorContains(t, err, e) | ||
| } | ||
| } |
There was a problem hiding this comment.
assertValidationError is now variadic; if it's ever called with no expected strings, it will pass even when rules.Validate(inv) returns nil. Add an explicit require.Error(t, err) (or guard against empty expected) to ensure the helper always asserts that validation failed.
| rules.Field("supplier", | ||
| rules.Field("name", | ||
| rules.AssertIfPresent("18", "supplier name must be 120 characters or less", is.Length(0, 120)), | ||
| ), | ||
| ), | ||
| // Customer - universal | ||
| // Code 19: customer name max 120 chars | ||
| // Code 22: non-ES customer tax ID code max 18 chars | ||
| rules.Field("customer", | ||
| rules.Field("name", | ||
| rules.AssertIfPresent("19", "customer name must be 120 characters or less", is.Length(0, 120)), | ||
| ), |
There was a problem hiding this comment.
The new 120-character limit for party names uses is.Length(0, 120), which counts bytes for strings. XML Schema maxLength constraints are defined in characters (runes), so names containing multi-byte characters (e.g., accented letters) could be incorrectly rejected. Consider switching these name validations to is.RuneLength(0, 120) (and update generated rules JSON accordingly).
Pre-Review Checklist
go generate .to ensure that the Schemas and Regime data are up to date.And if you are part of the org: