Skip to content

Recover the BIC when UBL nests it inside the financial institution - #124

Closed
rilla wants to merge 2 commits into
mainfrom
gbl-82-review-followups
Closed

Recover the BIC when UBL nests it inside the financial institution#124
rilla wants to merge 2 commits into
mainfrom
gbl-82-review-followups

Conversation

@rilla

@rilla rilla commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Context

UBL can carry the payee's BIC in two places: on cac:FinancialInstitutionBranch/cbc:ID (BT-86), or one level deeper under cac:FinancialInstitution/cbc:ID. The parser only read the first, so a document using the nested form lost the BIC entirely.

test/data/parse/peppol/nbio-stuck-ubl.xml is one of those documents — its GKCCBEBB never reached the parsed invoice, as the golden output shows. The wire model already declared FinancialInstitution for this element, but nothing read it.

Changes

  • Fall back to the nested FinancialInstitution/ID when the branch carries no ID of its own.
  • Assert the recovered BIC on the Peppol fixture that exercises the nested form.
  • Use cbc.Code's IsEmpty in newCreditTransferAccount, now that those fields are codes.

Outbound is unchanged: the BIC still goes to BT-86. The fallback only fires where the parser previously produced nothing, so no existing document parses differently.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.46%. Comparing base (58fb61a) to head (73604ce).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   81.40%   81.46%   +0.05%     
==========================================
  Files          28       28              
  Lines        2167     2174       +7     
==========================================
+ Hits         1764     1771       +7     
  Misses        265      265              
  Partials      138      138              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rilla

rilla commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing this. It came out of reading around the v0.504.0 payment change rather than from a reported problem, and the case for it rests on the nested cac:FinancialInstitution form being something we should accept — which I never confirmed against EN 16931. If the form shows up in real traffic, the branch keeps the fix and its test.

@rilla rilla closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants