feat(ksef): emit P_6 / P_8A / P_9A in FA(3) documents#1529
Conversation
Emit the three FA(3) fields required by art. 106e ust. 1 for goods sales that were previously omitted (#1525): - P_6 (sale date): new neutral IssueInvoiceCommand.saleDate filled from Order.placedAt (never createdAt - that is OL's ingestion clock); threaded through Fa3BuilderInput and emitted after P_2 in the Fa sequence whenever known, including when equal to P_1. - P_9A (net unit price): computed per FaWiersz as money(lineNet / quantity) from the same lineNet source as P_11/P_13_x; 2 decimals; documented rounding drift for quantity > 1 accepted; KOR before/after rows use the same formula. - P_8A (unit of measure): new neutral InvoiceLine.unit plus per-connection default at KsefConnectionConfig.invoiceDefaults.lineUnit (new top-level section per the payment #1311 precedent); precedence line unit -> connection default -> omit; config-shape validator gates a 20-char cap (empty treated as absent); FE adds a clearable 'Default line unit' field (datalist suggestions, prefilled 'szt.' on creation) to the create wizard and the edit-connection structured section. The structural FA(3) validator gains a FaWiersz child-order check (FA3_FA_WIERSZ_CHILD_ORDER) so an ordering regression fails at build time. Closes #1525 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…load, guard P_9A NaN Address PR #1529 round-1 review (2 BLOCKING, 1 IMPORTANT, 4 SUGGESTION): - BLOCKING-1: orderFromReadySnapshot now rehydrates snapshot.placedAt into Order.placedAt (invalid/absent stays undefined - no fallback substitution), so every controller flow that rebuilds the Order from the persisted snapshot carries the sale date into P_6. - BLOCKING-2: InvoicingIssuePayloadV1 gains an optional additive saleDate (no schemaVersion bump); AutoIssueTriggerService.composePayload threads it and the worker invoicing-issue handler restores it into the command and validates it when present, so the auto-issue path emits P_6 too. - IMPORTANT-1: the FA(3) builder emits P_9A only for quantity > 0 (a zero-quantity KOR after-row or malformed-snapshot default would divide to a literal NaN that KSeF rejects at clearance), and the command composer rejects non-positive/non-finite item quantities up front with the new PII-clean InvalidInvoiceLineError (mapped to 400 in the controller and allow-listed in the auto-issue PII-safe log envelope). - SUGGESTION-1: new FA3_FA_CHILD_ORDER constant + Fa-level child-order check in the structural validator so a P_6 position regression fails locally. - SUGGESTION-2: documented the accepted UTC-day trade-off in the mapper's toIsoDate helper (mirrors the controller toIsoDateOnly precedent). - SUGGESTION-3: IssuedLineSnapshot.lines doc now names the unit field. - SUGGESTION-4: line-unit suggestion list uses the proper Polish 'usl.' spelling with the l-stroke character. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…able asDate variant PR #1529 round-2 polish: the placedAt rehydration branch accepted only an ISO string while the sibling createdAt/updatedAt path (asDate) also accepts a Date instance. Extract asOptionalDate (string | Date -> valid Date, else undefined - the no-fallback semantics optional fields need) and delegate asDate to it; placedAt now shares the same acceptance. Spec adds a Date-instance input case alongside the existing string/absent/invalid ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
piotrswierzy
left a comment
There was a problem hiding this comment.
Review: feat(ksef): emit P_6 / P_8A / P_9A in FA(3) documents
Verdict: Approve. Correct, boundary-clean, and unusually thorough. No blockers.
What it does
Emits three art. 106e VAT-act fields the KSeF FA(3) builder was omitting: P_6 (sale date), P_8A (unit of measure), P_9A (net unit price). Threads a neutral saleDate (from Order.placedAt) and a neutral InvoiceLine.unit through the whole issue pipeline (mapper → job payload → worker handler → adapter → builder), adds a connection-level invoiceDefaults.lineUnit default with FE form + shape validator, and hardens the structural validator with Fa/FaWiersz child-order guards.
Assessment
- Architecture / ADR-026 — clean. Core gains only country-agnostic concepts (
saleDate: YYYY-MM-DD,InvoiceLine.unit); no FA/P_/KSeF/NIP vocabulary leaks intolibs/core. The mapper comments deliberately avoid naming P_6. All FA(3) specifics stay in the KSeF plugin. CORE↔Integration boundary respected. - Correctness — verified against the FA(3) structure.
P_9A = money(lineNet / quantity)reuses the samelineNetthat feedsP_11/P_13_x, so values can't drift;quantity > 0guard prevents aNaNliteral on zero-qty KOR rows. The accepted 2dp drift is safe (KSeF doesn't cross-validate P_9A×P_8B=P_11).P_8Aprecedence (line.unit→invoiceDefaults.lineUnit→ omit), with empty/whitespace treated as absent, applied uniformly to plain and correction lines.P_6emitted right afterP_2, including when equal toP_1; sourced only fromplacedAt, nevercreatedAt.- Element ordering (P_8A before P_8B, P_9A after P_8B; P_6 before the P_13_x aggregates) matches the XSD and is now enforced at build time by tag-boundary-based order guards.
- The
orderFromReadySnapshotrehydration completes theplacedAtsnapshot round-trip that the writer already persisted — without it the controller manual-issue path would have lost P_6 (real fix, not just plumbing).
- Naming / layers / types — compliant. No
anyin runtime code, explicit types, neutral PII-cleanInvalidInvoiceLineError(cites only order id), mapped to 400 at the controller boundary and added to the auto-issue PII-safe error set. No console.log. - Tests — excellent. AAA, edge cases covered end-to-end: qty-1 P_9A==P_11, documented qty-3 drift (33.33), zero-qty KOR omission (no NaN), P_6 present/absent/equals-P_1, unit precedence incl. correction lines, validator order-regression guards for both P_6 and P_8A, config-shape validator bounds (20-char/empty/whitespace/non-string), and FE prefill/clear/hydrate/merge semantics.
- Security / migrations — none applicable (config is JSONB on the existing connection row; no schema change).
BLOCKING
None.
IMPORTANT
None.
SUGGESTION (optional, non-blocking)
P_9B(gross unit price) remains unemitted — correctly out of scope for #1525, but worth a follow-up note if the PDF visualization later wants both unit prices side by side.- The 2dp
P_9Aprecision is a deliberate, documented trade-off (issue #1525 binding decision). If a future customer's own visualization does reconcile P_9A×P_8B against P_11, computing P_9A atTKwotowy2's full precision (up to 8dp) would remove the cent drift — but there's no correctness need today.
Positives
- Genuine end-to-end threading with matching test coverage at every hop, rather than emitting the fields at the builder only.
- Defense-in-depth on the divide-by-zero / NaN path (command-composer gate and builder guard).
- Turned a latent bug (snapshot persisted
placedAtbut rehydrator dropped it) into a fix. - Validator hardening converts an ordering regression from a KSeF-clearance failure into a build-time failure.
- Careful FE details: distinct datalist ids for create vs edit forms, delete-on-empty at both leaf and object level so a hollow
invoiceDefaultsnever reaches the wire.
| // catch but KSeF rejects at clearance - the optional element is omitted | ||
| // instead (defense-in-depth with the command composer's positive-quantity | ||
| // gate, which covers only the plain-issue path). | ||
| ...(line.quantity > 0 ? { P_9A: money(lineNet(line) / line.quantity) } : {}), |
There was a problem hiding this comment.
Correctness confirmed (positive). P_9A = money(lineNet(line) / line.quantity) reuses the exact lineNet helper that feeds P_11 and the P_13_x band aggregation, so the net unit price can never diverge from the authoritative net line value — the right call. The line.quantity > 0 guard is the correct defense: it covers the zero-quantity KOR "after" row (full return) that the command-composer's positive-quantity gate does not reach, and omits the optional element rather than emitting a literal NaN that the local structural validator can't catch but KSeF would reject at clearance.
Note on the accepted 2dp drift (documented in the comment and issue #1525): this is safe against KSeF clearance — KSeF does not cross-validate P_9A × P_8B = P_11 (P_9A is informational; P_11/P_13_x/P_15 are authoritative), so the cent-level drift for quantity > 1 will not trigger a rejection. No change requested.
| // placedAt is optional on Order (unlike createdAt/updatedAt) so there is no | ||
| // record-level fallback to substitute - a snapshot without it stays without | ||
| // it (invoicing then omits the sale date rather than guessing, #1525). | ||
| const placedAt = asOptionalDate(snapshot.placedAt); |
There was a problem hiding this comment.
Good catch threading this through the snapshot rehydrator (positive/context). order-record.service.ts already persisted placedAt into the snapshot (...(order.placedAt !== undefined && { placedAt: order.placedAt.toISOString() })), but orderFromReadySnapshot previously dropped it on the way back — so without this the controller's manual-issue path (invoicing.controller.ts:1020 → orderFromReadySnapshot → toIssueInvoiceCommand) would have silently lost the sale date and never emitted P_6. This completes the round-trip. The asOptionalDate refactor (no fallback for the optional field, invalid Date instances now correctly resolve to undefined) is a clean, behavior-preserving extraction for the mandatory createdAt/updatedAt paths.
What
FA(3) documents issued by the KSeF adapter omitted three fields required by art. 106e ust. 1 of the Polish VAT act for goods sales: P_6 (sale date), P_8A (unit of measure) and P_9A (net unit price). KSeF accepted the documents (the XSD marks them optional), but they were legally incomplete and the official PDF visualization could not state the price basis. This PR emits all three per the binding design decisions in #1525 / epic #1524.
How
P_6 (sale date)
IssueInvoiceCommand.saleDate?: string(ISOYYYY-MM-DD) inlibs/core/src/invoicing/domain/types/invoicing.types.ts.toIssueInvoiceCommandfills it fromOrder.placedAt; whenplacedAtis absent the field stays undefined -Order.createdAt(OL's ingestion clock) is never substituted.mapToFa3BuilderInputintoFa3BuilderInput.saleDateand emitted right afterP_2in theFasequence (the optional choice at XSD line ~2471), whenever known - including when equal toP_1.P_9A (net unit price)
FaWierszthe builder emitsmoney(lineNet(line) / line.quantity)- the SAMElineNethelper that feedsP_11and theP_13_xband aggregation, so the values can never drift from each other. 2 decimal places.P_9A x P_8Bmay differ fromP_11by cents) is accepted and documented in a code comment;P_11stays authoritative. KOR before/after rows use the identical formula (no correction special-casing).P_8A (unit of measure)
InvoiceLine.unit?: string(country-agnostic; the order mapper does not set it - marketplace orders carry no unit).KsefConnectionConfig.invoiceDefaults?: { lineUnit?: string }section (follows thepayment[FEATURE] Integration — emit FA(3) Platnosc (payment method, bank account, payment term, skonto) in KSeF invoices #1311 precedent, NOT onKsefSellerConfig).defaultTaxRate):line.unit->invoiceDefaults.lineUnit-> omitP_8A. Empty/whitespace counts as absent.ksef-connection-config-shape-validator.adapter.ts:lineUnitoptional string, 1-20 chars after trim (empty/whitespace treated as absent, not an error).connectionConfigcontribution per web: move plugin-specific connection-config assembly behind a plugin slot #1330) - free text with datalist suggestions (szt.,kg,godz.,usl.,kpl.,m), prefilledszt.on creation, clearable (empty = P_8A not emitted, hollowinvoiceDefaultsdropped from config).Validator hardening
FA3_FA_WIERSZ_CHILD_ORDERconstant + per-row child-order check infa3-xsd.validator.ts(P_8A immediately before P_8B, P_9A immediately after), so a builder ordering regression fails at build time instead of at KSeF clearance.How to test
Scoped runs (all green):
libs/core:npx jest --testPathPattern=invoicing- 17 suites, 185 tests passed (incl. new saleDate mapper cases).libs/integrations/ksef:npx jest- 29 suites, 378 tests passed (builder P_6/P_8A/P_9A cases incl. the qty-3 drift casenet 100.00 -> P_9A 33.33, mapper unit-precedence cases, config-shape validator lineUnit cases, FaWiersz order-guard case).apps/web:npx vitest run ksef- 10 files, 115 tests passed (schema cap, hydration, merge/clear semantics, wizard prefill + omission).tsc --noEmit/tsc -bclean for all three packages; ESLint clean on touched files.Manual: create/edit a KSeF connection - the "Default line unit" field is prefilled
szt.and clearable; issue an invoice for an order withplacedAtand inspect the persisted source XML:<P_6>,<P_8A>,<P_9A>present in XSD order.Closes #1525
🤖 Generated with Claude Code