fix!: improve peppol specification compliance#10
Merged
Conversation
…ssLine for party postal addresses and delivery locations Legal Entity schemeID (BT-30) - Changed companyId from string to object with id and optional schemeID, updated schema, builder, parser, and test data Tax Scheme Limitations - Changed taxSchemeCompanyID to taxSchemes array supporting 0..2 tax schemes with companyId and optional schemeID Line-level DocumentTypeCode - Changed documentReference from string to object with id and optional documentTypeCode, defaulting to '130' when not specified Line-level Tax Category Exemption - Added exemptionReason and exemptionReasonCode to line-level taxCategory schema, builder, and parser German-specific Note Repetition - Changed note from single string to array of objects with content and optional languageID for invoices, credit notes, and lines
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ICD code constants and new payee/tax-representative schemas, converts notes to structured multilingual arrays, extends payment means with card/mandate fields, updates party/tax identification shapes, normalizes parsing of notes/address/tax-schemes, and updates the XML builder to emit new parties, multilingual notes, and card/mandate elements. ChangesPeppol Party and Payment Structure Enhancements
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 9 high |
🟢 Metrics 0 complexity · 12 duplication
Metric Results Complexity 0 Duplication 12
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/builder/DocumentBuilder.ts`:
- Around line 559-560: The code emits a non-standard <cbc:AddressLine> scalar
and reads the wrong key in the parser; update the builders and parser to use the
UBL aggregate structure: in DocumentBuilder.ts replace the scalar
'cbc:AddressLine' entries in the Address construction (including in
__buildDelivery and __buildTaxRepresentativeParty) with a cac:AddressLine
aggregate containing a child cbc:Line holding party.address.addressLine, and in
DocumentParser.ts stop reading postal['cbc:AddressLine'] — instead read the
cac:AddressLine aggregate and extract its child cbc:Line (pass that value to
this.__str). Ensure the exact symbol names (__buildDelivery,
__buildTaxRepresentativeParty, cac:AddressLine, cbc:Line, and the
this.__str(...) call) are used so the changes target the correct locations.
- Around line 443-520: Both helpers incorrectly wrap party sub-elements in an
extra 'cac:Party' object causing invalid nested <cac:Party>; update
__buildPayeeParty and __buildTaxRepresentativeParty to return the inner party
fields directly (i.e., remove the top-level 'cac:Party' key so they return the
object containing
'cac:PartyIdentification'/'cac:PartyName'/'cac:PartyLegalEntity' and
'cac:PartyName'/'cac:PostalAddress' respectively). Then update DocumentParser
access paths (previously expecting ['cac:Party'] at the cited locations) to read
the party sub-elements directly (adjust references in DocumentParser.ts around
the mentioned ranges 83–88 and 172–177 to drop the extra ['cac:Party']
dereference). Ensure callers that assemble PayeeParty and TaxRepresentativeParty
now embed the returned object directly under 'cac:PayeeParty' and
'cac:TaxRepresentativeParty'.
- Around line 498-520: The __buildTaxRepresentativeParty method currently emits
only cac:PartyName and cac:PostalAddress but must include a required
cac:PartyTaxScheme/cbc:CompanyID (VAT identifier) per PEPPOL BIS Billing 3.0;
update the TaxRepresentativeParty schema
(src/documents/common/TaxRepresentativeParty.ts) to add a vatId (or companyId)
property, adjust the parser to read that field, and modify
__buildTaxRepresentativeParty to emit a cac:PartyTaxScheme object containing
cbc:CompanyID set to taxRep.vatId (ensuring it exists for non-nullable
taxRepresentativeParty).
- Around line 69-82: The note-building map is duplicated in DocumentBuilder.ts
for invoice, credit note, and line items; extract a private helper (e.g.,
__buildNotes(notes: Note[])) inside DocumentBuilder that implements the existing
logic (for each note return { '#text': content, ...XMLAttributes({ languageID })
} when languageID exists else the raw content) and then replace the three spread
blocks (the invoice block around invoice.note, the credit note block, and the
line-item block) with a spread of that helper's result to keep behavior
identical and avoid divergence.
In `@src/documents/common/ICDCodes.ts`:
- Around line 126-127: The isValidICDCode function incorrectly uses the "in"
operator which checks the prototype chain and can return true for inherited
properties (e.g., "toString"); replace that check with an own-property check
against the ICDCodes map (use Object.prototype.hasOwnProperty.call(ICDCodes,
code) or Object.hasOwn(ICDCodes, code)) so only actual ICDCodes keys validate;
update the implementation inside isValidICDCode to perform this direct
own-property lookup.
In `@src/documents/common/Line.ts`:
- Around line 29-33: The schema for documentReference.documentTypeCode currently
allows empty strings (z.string().optional()), so empty "" bypasses the fallback
in DocumentBuilder (where line.documentReference.documentTypeCode ?? '130');
update the Line.ts schema to disallow empty strings by requiring at least one
character (e.g., change to z.string().min(1).optional()) so only undefined/null
triggers the builder's '130' default; verify the field name
documentReference.documentTypeCode and the DocumentBuilder usage remain
consistent after the change.
In `@src/parser/DocumentParser.ts`:
- Around line 315-336: In __parsePayeeParty (and mirror the change in
__parseTaxRepresentativeParty) avoid the non-null assertion on name: call
this.__str(partyNameEl?.['cbc:Name']) without '!' and validate the result; if
it's undefined, throw a clear, descriptive error (e.g., "Missing party name in
__parsePayeeParty" / "__parseTaxRepresentativeParty") or handle a sensible
fallback so the runtime doesn't silently propagate undefined into schema
validation; update the returned object to use the validated value.
- Around line 83-88: The parser is incorrectly assuming a nested cac:Party
element; update the calls to __parsePayeeParty and __parseTaxRepresentativeParty
to pass the cac:PayeeParty and cac:TaxRepresentativeParty nodes directly (e.g.,
use inv['cac:PayeeParty'] and inv['cac:TaxRepresentativeParty'] instead of
inv['cac:PayeeParty']?.['cac:Party'] /
inv['cac:TaxRepresentativeParty']?.['cac:Party']), and apply the same change in
parseCreditNote (the credit-note branch that calls the same parser helpers) so
the parser consumes the PEPPOL-standard structure without expecting an extra
wrapper.
- Around line 258-272: The non-null assertion on companyId in
DocumentParser.taxSchemes can yield undefined at runtime and cause obscure Zod
errors; replace the `companyId: this.__str(ts['cbc:CompanyID'])!` pattern with
an explicit check: call `this.__str(ts['cbc:CompanyID'])`, and if it returns
undefined throw a clear, descriptive Error (e.g., "Missing cbc:CompanyID in
cac:PartyTaxScheme" with any available context like party identifier) so
failures surface with intent, or alternatively omit that entry if you want to
tolerate missing IDs—do this inside the taxSchemes mapping to avoid propagating
undefined into the z.string().min(1) validation.
- Around line 33-45: Extract the duplicated IIFE into a private helper on
DocumentParser named something like __parseNotes(notesNode) that accepts the raw
notes value (e.g., inv['cbc:Note']), returns undefined when there are no notes,
normalizes a single item into an array, and maps each note to { content: noteStr
?? '', languageID: this.__attr(noteObj, 'languageID') } where noteStr is
obtained via this.__str(n) (avoid the non-null assertion). Replace the three
IIFEs (the blocks that reference inv['cbc:Note'] in invoice, credit note, and
the line-item section) with calls to this.__parseNotes(...) so all parsing and
the noteStr fallback are handled in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 442de873-f577-4992-a18a-f3ab051ebe82
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
src/builder/DocumentBuilder.tssrc/data/basic-creditNote.tssrc/data/basic-invoice.tssrc/documents/common/ICDCodes.tssrc/documents/common/Line.tssrc/documents/common/Parties.tssrc/documents/common/PayeeParty.tssrc/documents/common/PaymentMeans.tssrc/documents/common/TaxRepresentativeParty.tssrc/documents/common/index.tssrc/documents/invoices/Invoice.tssrc/parser/DocumentParser.ts
Member
|
Hi @costikid! Thank you very much for the contribution. I’ll review and test the PR tomorrow — sorry for the delay. CodeRabbit seems to be correct regarding the Party and address line issues, so I’ll make the necessary changes during the review. The comments about the parser can probably be ignored for now, since the output is already validated by the Zod schema. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/parser/DocumentParser.ts (2)
256-270:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid non-null assertions for required XML children in party/payment parsing.
These
!assertions can silently passundefinedinto typed output (companyId,name,cardAccountfields), then fail later with less actionable errors. Validate and throw explicit parse errors at extraction points.Also applies to: 313-334, 357-375, 636-644
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser/DocumentParser.ts` around lines 256 - 270, In DocumentParser, replace the non-null assertions used when extracting required XML children (e.g., in the taxSchemes IIFE that sets companyId via this.__str(ts['cbc:CompanyID'])!, and similarly where name and cardAccount are forced) with explicit validation: check the presence and non-empty string result of this.__str(...) and throw a descriptive parse error (including context like element name and parent id) when missing, rather than allowing undefined to flow into the output; update the taxSchemes mapping and the other affected extraction sites (the party/payment parsing blocks that populate companyId, name, cardAccount) to perform these checks before building the returned object so consumers receive immediate, actionable errors.
34-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
noteStr!and centralize note parsing.
content: noteStr!can produce runtimeundefinedwhen a note node has attributes but no text. This is repeated in three places; extract a single helper and handle empty text safely.Proposed fix
+ private __parseNotes( + notes: unknown + ): Array<{ content: string; languageID?: string }> | undefined { + if (!notes) return undefined; + const notesArray = Array.isArray(notes) ? notes : [notes]; + return notesArray.map((n) => { + const noteObj = n as Record<string, unknown>; + return { + content: this.__str(n) ?? '', + languageID: this.__attr(noteObj, 'languageID'), + }; + }); + }- note: (() => { ... })(), + note: this.__parseNotes(inv['cbc:Note']),- note: (() => { ... })(), + note: this.__parseNotes(cn['cbc:Note']),- note: (() => { ... })(), + note: this.__parseNotes(l['cbc:Note']),Also applies to: 126-138, 793-805
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser/DocumentParser.ts` around lines 34 - 46, The current inline note mapping in DocumentParser uses a non-null assertion (noteStr!) that can throw if a note element has attributes but no text; extract a single helper method (e.g., DocumentParser.parseNote or parseNotes) that accepts a note node (Record<string, unknown> or unknown) and returns a safe object with content set to either the string from this.__str(node) or an empty string/undefined (choose consistent behavior) and languageID from this.__attr(node,'languageID'); then replace the three duplicate inline mappings (the anonymous IIFE here and the occurrences around the ranges mentioned) to call that helper, removing all non-null assertions and centralizing the parsing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/parser/DocumentParser.ts`:
- Around line 383-388: The current logic only handles cac:AddressLine when it's
an object and ignores the array form; update the address extraction in
DocumentParser to detect if addressLine is an array and handle it by extracting
each element's 'cbc:Line' (using the same __str conversion) and either join them
(e.g., with a space or newline) or return the first non-empty line; keep the
existing object branch for single objects and ensure you reference the
addressLine variable, the 'cbc:Line' field, and the __str method when
implementing this change.
---
Duplicate comments:
In `@src/parser/DocumentParser.ts`:
- Around line 256-270: In DocumentParser, replace the non-null assertions used
when extracting required XML children (e.g., in the taxSchemes IIFE that sets
companyId via this.__str(ts['cbc:CompanyID'])!, and similarly where name and
cardAccount are forced) with explicit validation: check the presence and
non-empty string result of this.__str(...) and throw a descriptive parse error
(including context like element name and parent id) when missing, rather than
allowing undefined to flow into the output; update the taxSchemes mapping and
the other affected extraction sites (the party/payment parsing blocks that
populate companyId, name, cardAccount) to perform these checks before building
the returned object so consumers receive immediate, actionable errors.
- Around line 34-46: The current inline note mapping in DocumentParser uses a
non-null assertion (noteStr!) that can throw if a note element has attributes
but no text; extract a single helper method (e.g., DocumentParser.parseNote or
parseNotes) that accepts a note node (Record<string, unknown> or unknown) and
returns a safe object with content set to either the string from
this.__str(node) or an empty string/undefined (choose consistent behavior) and
languageID from this.__attr(node,'languageID'); then replace the three duplicate
inline mappings (the anonymous IIFE here and the occurrences around the ranges
mentioned) to call that helper, removing all non-null assertions and
centralizing the parsing logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f951330-4f15-41c2-9cd9-71b77ba908ce
📒 Files selected for processing (6)
src/builder/DocumentBuilder.tssrc/documents/common/Parties.tssrc/documents/common/TaxRepresentativeParty.tssrc/parser/DocumentParser.tstests/builder-creditnote.test.tstests/builder-invoice.test.ts
Member
|
@costikid any reason why you added line level tax exemption ? It does not seem to follow the specification https://docs.peppol.eu/poacc/billing/3.0/syntax/ubl-invoice/cac-InvoiceLine/cac-Item/cac-ClassifiedTaxCategory/ |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/builder/DocumentBuilder.ts (1)
611-680:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
cac:PaymentMeanschild elements emitted in wrong XSD sequence order.UBL 2.1
cac:PaymentMeansTyperequires this element sequence:cbc:PaymentMeansCode,cbc:PaymentDueDate,cbc:PaymentID,cac:CardAccount,cac:PayerFinancialAccount,cac:PayeeFinancialAccount,cac:PaymentMandate. The current builder emitscac:PayeeFinancialAccountbeforecac:CardAccount(lines 629–643 before 644–658), which violates XSD validation whenever both are present. Reorder the spread operators soCardAccountprecedesPayeeFinancialAccount.Proposed fix
'cbc:PaymentID': m.paymentId, - ...(financialAccount - ? { - 'cac:PayeeFinancialAccount': { - 'cbc:ID': financialAccount.id, - 'cbc:Name': financialAccount.name, - 'cac:FinancialInstitutionBranch': - financialAccount.financialInstitutionBranch - ? { - 'cbc:ID': - financialAccount.financialInstitutionBranch, - } - : undefined, - }, - } - : {}), ...(cardAccount ? { 'cac:CardAccount': { 'cbc:PrimaryAccountNumberID': cardAccount.primaryAccountNumberId, 'cbc:NetworkID': cardAccount.networkId, ...(cardAccount.holderName ? { 'cbc:HolderName': cardAccount.holderName, } : {}), }, } : {}), + ...(financialAccount + ? { + 'cac:PayeeFinancialAccount': { + 'cbc:ID': financialAccount.id, + 'cbc:Name': financialAccount.name, + 'cac:FinancialInstitutionBranch': + financialAccount.financialInstitutionBranch + ? { + 'cbc:ID': + financialAccount.financialInstitutionBranch, + } + : undefined, + }, + } + : {}), ...(paymentMandate🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/builder/DocumentBuilder.ts` around lines 611 - 680, The cac:PaymentMeans child element order in __buildPaymentMeans is incorrect; move the cardAccount spread (producing 'cac:CardAccount') so it is added before the financialAccount spread (producing 'cac:PayeeFinancialAccount') and ensure the paymentMandate spread ('cac:PaymentMandate') remains last, so the emitted sequence matches UBL 2.1: 'cbc:PaymentMeansCode', 'cbc:PaymentDueDate', 'cbc:PaymentID', 'cac:CardAccount', 'cac:PayerFinancialAccount' (if any), 'cac:PayeeFinancialAccount', 'cac:PaymentMandate'.
♻️ Duplicate comments (3)
src/documents/common/Line.ts (1)
29-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestrict empty strings for
documentTypeCode.
documentTypeCode: z.string().optional()accepts empty strings, which bypass the builder's?? '130'fallback atDocumentBuilder.ts:1050(since''is not nullish) and produce an empty<cbc:DocumentTypeCode/>element instead of the BT-17/130default.🛡️ Proposed fix
documentReference: z .object({ id: z.string().min(1), - documentTypeCode: z.string().optional(), + documentTypeCode: z.string().min(1).optional(), }) .optional(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/documents/common/Line.ts` around lines 29 - 34, The schema for documentReference.documentTypeCode allows empty strings which bypass the fallback in DocumentBuilder (see DocumentBuilder.ts:1050); update the zod definition for documentTypeCode in Line.ts (documentReference) to disallow empty strings, e.g., change z.string().optional() to z.string().min(1).optional() (or an equivalent non-empty string refinement) so the builder's nullish-coalescing default ('130') is applied when no value is provided.src/builder/DocumentBuilder.ts (1)
1045-1053:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
?? '130'fallback can still be bypassed by an empty string.
line.documentReference.documentTypeCode ?? '130'only defaults when the value isnull/undefined. The companion schema (src/documents/common/Line.ts:32) currently allows empty strings, which would emit<cbc:DocumentTypeCode></cbc:DocumentTypeCode>. Tightening the schema toz.string().min(1).optional()(see the comment onLine.ts) is the simplest fix; alternatively guard here:🛡️ Defensive fallback at the builder
- 'cbc:DocumentTypeCode': - line.documentReference.documentTypeCode ?? '130', + 'cbc:DocumentTypeCode': + line.documentReference.documentTypeCode || '130',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/builder/DocumentBuilder.ts` around lines 1045 - 1053, The builder emits an empty <cbc:DocumentTypeCode> when documentTypeCode is an empty string because the nullish-coalescing fallback only handles null/undefined; update the code that constructs the documentReference in DocumentBuilder (the expression using line.documentReference and line.documentReference.documentTypeCode) to defensively treat empty strings as absent (e.g., normalize/trim and use a logical OR fallback to '130' or check length) or, alternatively, tighten the Line schema (in src/documents/common/Line.ts) to z.string().min(1).optional() so documentTypeCode cannot be an empty string; pick one approach and apply consistently to avoid emitting empty tags.src/parser/DocumentParser.ts (1)
355-374:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
__parseAddressLinestill drops content whencac:AddressLineis parsed as an array.
fast-xml-parserreturns an array forcac:AddressLinewhen multiple<cac:AddressLine>elements exist (a valid 0..unbounded structure in UBL). The current checktypeof addressLine === 'object'matches arrays too, and thenaddressLine['cbc:Line']silently resolves toundefined, dropping every line. Unlike a missing required field, Zod won't catch this sinceaddressLineis optional — it's silent data loss on round-trip.🛡️ Proposed fix
private __parseAddressLine( address: Record<string, unknown> ): string | undefined { // Prefer the current cac:AddressLine/cbc:Line structure. const addressLine = address['cac:AddressLine']; + if (Array.isArray(addressLine)) { + const lines = addressLine + .map((al) => + this.__str( + (al as Record<string, unknown>)['cbc:Line'] + ) + ) + .filter((l): l is string => Boolean(l)); + if (lines.length > 0) return lines.join('\n'); + } if (addressLine && typeof addressLine === 'object') { return this.__str( (addressLine as Record<string, unknown>)['cbc:Line'] ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser/DocumentParser.ts` around lines 355 - 374, The __parseAddressLine function currently treats address['cac:AddressLine'] as an object which erroneously matches arrays and drops content; change the logic to detect Array.isArray(addressLine) and handle three cases: if it's an array, map each element to its 'cbc:Line' using __str and join them (e.g., with '\n' or ', '), if it's a single object extract its 'cbc:Line' via __str, otherwise fall back to the legacy cbc:AddressLine path; keep the deprecation warning behavior tied to __hasWarnedLegacyAddressLine and reuse __str for normalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/builder/DocumentBuilder.ts`:
- Around line 486-528: The __buildPartyTaxScheme currently defaults
cac:TaxScheme/cbc:ID to "VAT" when ts.schemeID is missing, which can produce
incorrect PEPPOL semantics for non-VAT tax registrations; change the logic in
__buildPartyTaxScheme (which accepts ts:
NonNullable<Invoice['taxRepresentativeParty']>['taxScheme']) to only emit the
cac:TaxScheme object when ts.schemeID is provided (or require callers to supply
schemeID for non-VAT entries) instead of silently inserting "VAT", and simplify
the duplicated if/else by building the cac:TaxScheme only once conditionally
when ts.schemeID exists.
In `@src/parser/DocumentParser.ts`:
- Around line 952-961: Change the return type from Array<{ content: string;
languageID?: string }> | undefined to { content: string; languageID?: string }[]
| undefined to satisfy `@typescript-eslint/array-type`; in __parseNotes, avoid the
non-null assertion on this.__str(note) — capture the result (e.g. const text =
this.__str(note)) and if it's null/undefined throw a clear error (or otherwise
handle as a validation failure) so missing cbc:Note text surfaces
deterministically; keep languageID from this.__attr(note, 'languageID') and
preserve the notesArray mapping logic in __parseNotes.
---
Outside diff comments:
In `@src/builder/DocumentBuilder.ts`:
- Around line 611-680: The cac:PaymentMeans child element order in
__buildPaymentMeans is incorrect; move the cardAccount spread (producing
'cac:CardAccount') so it is added before the financialAccount spread (producing
'cac:PayeeFinancialAccount') and ensure the paymentMandate spread
('cac:PaymentMandate') remains last, so the emitted sequence matches UBL 2.1:
'cbc:PaymentMeansCode', 'cbc:PaymentDueDate', 'cbc:PaymentID',
'cac:CardAccount', 'cac:PayerFinancialAccount' (if any),
'cac:PayeeFinancialAccount', 'cac:PaymentMandate'.
---
Duplicate comments:
In `@src/builder/DocumentBuilder.ts`:
- Around line 1045-1053: The builder emits an empty <cbc:DocumentTypeCode> when
documentTypeCode is an empty string because the nullish-coalescing fallback only
handles null/undefined; update the code that constructs the documentReference in
DocumentBuilder (the expression using line.documentReference and
line.documentReference.documentTypeCode) to defensively treat empty strings as
absent (e.g., normalize/trim and use a logical OR fallback to '130' or check
length) or, alternatively, tighten the Line schema (in
src/documents/common/Line.ts) to z.string().min(1).optional() so
documentTypeCode cannot be an empty string; pick one approach and apply
consistently to avoid emitting empty tags.
In `@src/documents/common/Line.ts`:
- Around line 29-34: The schema for documentReference.documentTypeCode allows
empty strings which bypass the fallback in DocumentBuilder (see
DocumentBuilder.ts:1050); update the zod definition for documentTypeCode in
Line.ts (documentReference) to disallow empty strings, e.g., change
z.string().optional() to z.string().min(1).optional() (or an equivalent
non-empty string refinement) so the builder's nullish-coalescing default ('130')
is applied when no value is provided.
In `@src/parser/DocumentParser.ts`:
- Around line 355-374: The __parseAddressLine function currently treats
address['cac:AddressLine'] as an object which erroneously matches arrays and
drops content; change the logic to detect Array.isArray(addressLine) and handle
three cases: if it's an array, map each element to its 'cbc:Line' using __str
and join them (e.g., with '\n' or ', '), if it's a single object extract its
'cbc:Line' via __str, otherwise fall back to the legacy cbc:AddressLine path;
keep the deprecation warning behavior tied to __hasWarnedLegacyAddressLine and
reuse __str for normalization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 68ad6916-65ce-43d7-8d92-2214e36df9eb
📒 Files selected for processing (3)
src/builder/DocumentBuilder.tssrc/documents/common/Line.tssrc/parser/DocumentParser.ts
Contributor
Author
Yes, you are right about that. Thank you. |
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
2 participants
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.
Address Fields - Builder now emits cbc:CountrySubentity and cbc:AddressLine for party postal addresses and delivery locations
Legal Entity schemeID (BT-30) - Changed companyId from string to object with id and optional schemeID, updated schema, builder, parser, and test data
Tax Scheme Limitations - Changed taxSchemeCompanyID to taxSchemes array supporting 0..2 tax schemes with companyId and optional schemeID
Line-level DocumentTypeCode - Changed documentReference from string to object with id and optional documentTypeCode, defaulting to '130' when not specified
Line-level Tax Category Exemption - Added exemptionReason and exemptionReasonCode to line-level taxCategory schema, builder, and parser
German-specific Note Repetition - Changed note from single string to array of objects with content and optional languageID for invoices, credit notes, and lines