Re-enable BR-CO-19 and BR-CO-20 validation with proper implementation

[fank]

Background

PR #43 disabled BR-CO-19 and BR-CO-20 validation checks. This issue tracks the work needed to properly re-implement these validations.

Why Were They Disabled?

The original validation code contained logically impossible conditions that could never trigger violations:

// Original code in check.go:159-163
if !inv.BillingSpecifiedPeriodStart.IsZero() || !inv.BillingSpecifiedPeriodEnd.IsZero() {
    if inv.BillingSpecifiedPeriodStart.IsZero() && inv.BillingSpecifiedPeriodEnd.IsZero() {
        inv.addViolation(rules.BRCO19, "If invoicing period is used, either start date or end date must be filled")
    }
}

Problem: If the outer if is true (at least one date is non-zero), the inner if can NEVER be true (both dates are zero). The validation was already non-functional.

PR #43 removed this broken code to eliminate dead code and make the non-enforcement explicit.

What These Rules Require

BR-CO-19 (Invoice Period)

Spec: If INVOICING PERIOD (BG-14) is used, either "Invoicing period start date" (BT-73) OR "Invoicing period end date" (BT-74) OR both must be filled.

BR-CO-20 (Invoice Line Period)

Spec: If INVOICE LINE PERIOD (BG-26) is used, either "Invoice line period start date" (BT-134) OR "Invoice line period end date" (BT-135) OR both must be filled.

The Core Problem

The Go struct cannot distinguish between:

1. BG-14/BG-26 not present in source XML (valid, no violation)
2. BG-14/BG-26 present with empty dates in source XML (INVALID, should violate)

Both cases result in time.Time{} (zero value) in the Invoice struct.

Example XML that should violate but doesn't:

<rsm:SpecifiedSupplyChainTradeTransaction>
    <ram:ApplicableHeaderTradeSettlement>
        <ram:BillingSpecifiedPeriod>
            <!-- BG-14 is present but has no dates - violation! -->
        </ram:BillingSpecifiedPeriod>
    </ram:ApplicableHeaderTradeSettlement>
</rsm:SpecifiedSupplyChainTradeTransaction>

Usage Scenarios

The library supports three ways of working with invoices, each requiring different handling:

1. Parsing XML (Primary Use Case)

inv, err := einvoice.ParseXMLFile("invoice.xml")
if err != nil {
    return err
}
err = inv.Validate()  // Should catch empty BG-14/BG-26 elements

Current behavior: Cannot detect empty BG-14/BG-26 elements because parser doesn't track element presence.

Required fix: Track when BG-14/BG-26 elements exist in source XML.

2. Writing XML (Already Correct)

inv.Write(os.Stdout)

Current behavior: Writer only outputs BG-14/BG-26 when at least one date is non-zero (writer.go:437, writer.go:112). This automatically satisfies BR-CO-19/20.

No changes needed - writer behavior is correct.

3. Programmatic Building (NOT covered in original issue!)

inv := &einvoice.Invoice{
    InvoiceNumber: "INV-001",
    BillingSpecifiedPeriodStart: periodStart,  // Setting only start date
    BillingSpecifiedPeriodEnd:   time.Time{},  // Zero value
    // ... other fields
}

if err := inv.Validate(); err != nil {
    // Should this validate BR-CO-19?
}

inv.Write(os.Stdout)  // Will output BG-14 with only start date (valid per BR-CO-19)

Current tests (writer_test.go:357-581) show programmatic building with:

• Only end date set → Writer outputs BG-14 (valid)
• Only start date set → Writer outputs BG-14 (valid)
• Both dates set → Writer outputs BG-14 (valid)
• Neither date set → Writer omits BG-14 (valid, not "used")

Analysis: When building programmatically:

• Setting neither date → BG-14 not written → not "used" → no violation
• Setting at least one date → BG-14 written → "used" AND has date → no violation

The rule cannot be violated when building programmatically because the writer behavior ensures correctness.

Proposed Solution

Option 1: Add Presence Tracking (Recommended)

Add boolean fields to track if the groups were present in source XML:

type Invoice struct {
    // ... existing fields ...

    BillingSpecifiedPeriodStart time.Time
    BillingSpecifiedPeriodEnd   time.Time

    // Add this (unexported to prevent confusion):
    billingPeriodPresent bool  // true if BG-14 was in source XML

    // ... rest of fields ...
}

type InvoiceLine struct {
    // ... existing fields ...

    BillingSpecifiedPeriodStart time.Time
    BillingSpecifiedPeriodEnd   time.Time

    // Add this (unexported to prevent confusion):
    linePeriodPresent bool  // true if BG-26 was in source XML

    // ... rest of fields ...
}

Note: Fields are unexported because they are only meaningful for parsed invoices. Programmatic builders should not need to set these.

Validation logic:

// BR-CO-19 - Only check if element was present in parsed XML
if inv.billingPeriodPresent {
    if inv.BillingSpecifiedPeriodStart.IsZero() && inv.BillingSpecifiedPeriodEnd.IsZero() {
        inv.addViolation(rules.BRCO19, "If invoicing period is used, either start date or end date must be filled")
    }
}

// BR-CO-20
for i, line := range inv.InvoiceLines {
    if line.linePeriodPresent {
        if line.BillingSpecifiedPeriodStart.IsZero() && line.BillingSpecifiedPeriodEnd.IsZero() {
            inv.addViolation(rules.BRCO20, fmt.Sprintf("Invoice line %d: if line period is used, either start date or end date must be filled", i+1))
        }
    }
}

Parser changes:

// In parser.go, when parsing BillingSpecifiedPeriod
billingPeriodNode := applicableHeaderTradeSettlement.SelectElement("ram:BillingSpecifiedPeriod")
if billingPeriodNode != nil {
    inv.billingPeriodPresent = true
    inv.BillingSpecifiedPeriodStart, _ = parseTime(applicableHeaderTradeSettlement, "ram:BillingSpecifiedPeriod/ram:StartDateTime/udt:DateTimeString")
    inv.BillingSpecifiedPeriodEnd, _ = parseTime(applicableHeaderTradeSettlement, "ram:BillingSpecifiedPeriod/ram:EndDateTime/udt:DateTimeString")
}

// Similar for line periods in parseInvoiceLines()
linePeriodNode := lineTradeSettlement.SelectElement("ram:BillingSpecifiedPeriod")
if linePeriodNode != nil {
    invoiceLine.linePeriodPresent = true
    invoiceLine.BillingSpecifiedPeriodStart, _ = parseTime(lineItem, "ram:SpecifiedLineTradeSettlement/ram:BillingSpecifiedPeriod/ram:StartDateTime/udt:DateTimeString")
    invoiceLine.BillingSpecifiedPeriodEnd, _ = parseTime(lineItem, "ram:SpecifiedLineTradeSettlement/ram:BillingSpecifiedPeriod/ram:EndDateTime/udt:DateTimeString")
}

Writer changes:
No changes needed - writer already only outputs BG-14/BG-26 when at least one date exists.

Programmatic building behavior:

• Fields default to false → validation skipped
• Writer behavior ensures valid output
• No breaking changes for existing code

Option 2: Document as Known Limitation

If adding fields is not desired:

1. Document in code comments that these rules cannot be validated
2. Add to README/documentation as a known limitation
3. Accept that the library cannot catch this specific violation when parsing

Implementation Checklist

Option 1 (Presence Tracking):

• Add billingPeriodPresent bool to Invoice struct (unexported)
• Add linePeriodPresent bool to InvoiceLine struct (unexported)
• Update parser to set presence flags when parsing BG-14/BG-26 elements
• Implement proper validation logic for BR-CO-19
• Implement proper validation logic for BR-CO-20
• Add test cases for parsing:
  • BG-14 present with no dates (should violate)
  • BG-14 present with only start date (valid)
  • BG-14 present with only end date (valid)
  • BG-14 present with both dates (valid)
  • BG-14 not present (valid, no violation)
  • Same test coverage for BG-26 (invoice line periods)
• Verify existing programmatic building tests still pass (writer_test.go:357-881)
• Verify writer behavior remains unchanged
• Update documentation to clarify validation only applies to parsed invoices

Option 2 (Document Limitation):

• Add comments to BR-CO-19/BR-CO-20 sections explaining limitation
• Update README with known limitation
• Document that writer ensures correct output regardless

Code Locations

Affected files:

• model.go:315-316 - Invoice.BillingSpecifiedPeriod fields
• model.go:195-196 - InvoiceLine.BillingSpecifiedPeriod fields
• parser.go:150-151 - Invoice period parsing
• parser.go:429-430 - Line period parsing
• check.go:156-166 - Disabled validation (to be re-enabled)
• writer.go:437-448 - Invoice period writing (already correct)
• writer.go:112-118 - Line period writing (already correct)
• writer_test.go:357-881 - Programmatic building tests (verify these still pass)

Priority

Medium - These rules are currently not enforced, but:

• Writer already ensures correct output for programmatically built invoices
• Most invoicing systems would populate at least one date when using these groups
• This is an edge case validation that would catch malformed XML input during parsing
• Programmatic building is not affected - the rule is satisfied by design

However, full spec compliance requires implementing these checks for the parsing scenario.

References

• PR Fix VAT calculation and validation issues #43: "Fix critical VAT calculation and validation issues (C1, C2)"
• EN 16931 specification sections on BG-14 and BG-26
• Related rules: BR-CO-19, BR-CO-20
• Existing tests: writer_test.go lines 357-881 (programmatic building scenarios)