feat(web/ksef): rebuild FA(3) preview to mirror the official KSeF visualization#1528
Conversation
piotrswierzy
left a comment
There was a problem hiding this comment.
Tech Lead Review — PR #1528
Verdict: APPROVE. A clean, well-scoped, thoroughly-tested FE rebuild of the KSeF FA(3) preview that fixes two real correctness bugs and respects every architectural boundary. No blockers.
What it does
Rebuilds KsefFa3View (parser + render) to mirror the official KSeF/MF invoice visualization instead of the old flat slot-row list, and fixes two parser bugs: the swapped P_1/P_2 (issue-date ↔ invoice-number) mapping and the wrong name element (NazwaSkrocona → Nazwa with a foreign-doc fallback). Extends the parser to party addresses, currency, sale date, unit/net-unit-price, RodzajFaktury, Platnosc, and KOR correction metadata; adds a document-styled, theme-aware, tokens-only layout in a bounded index.css section.
Assessment
- Plugin seam — correct. The FA(3) preview renders through the KSeF plugin's
invoiceDetailSectioncontribution (ksef-invoice-detail-section.tsx→KsefFa3View), consistent with the content-onlyPlatformContributioncontract infrontend-architecture.md. ZeroplatformType ===literals; all cross-boundary imports go throughshared/*andfeatures/invoicingpublic barrels — no deep subpaths, no host internals. - State ownership — correct. Server state stays in the
features/invoicingTanStack-Query hooks; the component only transforms already-fetched XML client-side viaDOMParser(memoized onxmlText). No rawfetch(). - Types — clean. No
any; explicit return types throughout; every optional FA(3) field modeled as| null(no zod snapshot.optional()/.nullish()trap — there is no zod schema here, the XML projection is hand-typed correctly). - UI/style — compliant. Tokens only (
--space-*,--text-*,--border-*,--status-*,--radius-*,--shadow-focus), no raw hex; responsive (overflow-x:autotable wrap,auto-fitparty grid,flex-wrap);mono-textfor ids/NIP/bank/KSeF number,tabularfor amounts/dates; native<details>/<summary>with a:focus-visiblering for the KOR before-lines.check-design-tokens.mjsreported passing. - Naming/tests — compliant. kebab-case files / PascalCase exports,
*.test.tsx. 29 tests cover loading/placeholder path plus full invoice, minimal legacy doc, KOR before/after separation (#1364 preserved), payment mapping, namespace-prefixed (tns:) docs, and the parse-failurenullcontract.
BLOCKING
None.
IMPORTANT
None.
SUGGESTION
ksef-fa3-view.tsx:304(+ VAT table at:521) — addscope="col"to<th>cells for stronger screen-reader header association (a11y,frontend-ui-style-guide.md).ksef-invoice-detail-section.tsx:248— fetched-but-unparseable XML yields an empty preview (chip only) instead of the placeholder/error copy, since the branch mountsKsefFa3ViewwheneverviewText !== nulland the view self-nulls on parse failure. Unlikely edge given the robust parser; pre-existing wiring (only theksefNumberprop was added).
Positives
- Fixes two genuine data-fidelity bugs (P_1/P_2 swap,
Nazwa) each with a targeted regression test. - Namespace-agnostic
localNamelookup handlestns:-prefixed real-world FA(3) documents wheregetElementsByTagNamewould silently return nothing — well reasoned and tested. - Compile-time exhaustiveness on
FormaPlatnoscivia aRecord<KsefFormaPlatnosci, …>keyed off the guarded #1311 vocabulary, with graceful raw-code fallback for future codes. - Per-table conditional Unit/Net-unit-price columns avoid dash-only columns on legacy documents — a thoughtful touch, tested on both plain and KOR tables.
| <table className="ksef-fa3-view__table"> | ||
| <thead> | ||
| <tr> | ||
| <th>{t('invoice.ksef.fa3LineNo', '#')}</th> |
There was a problem hiding this comment.
SUGGESTION (a11y, frontend-ui-style-guide.md — a11y): the <th> column headers here (and the VAT-summary table at line 521) omit scope="col". For data tables, adding scope="col" gives screen readers an explicit header-to-cell association. Cheap and improves the semantics of both tables. Non-blocking.
| <span>{t('invoice.ksef.fa3ViewError', "Preview failed. Click 'View' to retry.")}</span> | ||
| ) : fa3.viewText !== null ? ( | ||
| <KsefFa3View xmlText={fa3.viewText} /> | ||
| <KsefFa3View xmlText={fa3.viewText} ksefNumber={ksefNumber} /> |
There was a problem hiding this comment.
SUGGESTION (UX edge case): KsefFa3View returns null when the fetched XML can't be parsed, but this branch renders <KsefFa3View> whenever viewText !== null. So a fetched-but-unparseable document shows an empty preview area (just the doc-preview__chip) rather than falling through to the fa3PreviewPlaceholder/error copy — which contradicts the "parent falls through to the placeholder" contract described in the KsefFa3View header doc. Given the parser is namespace-tolerant and well-tested against the builder's shapes this is an unlikely edge, but consider surfacing the placeholder/error text when viewText is present yet unparseable. Non-blocking, and the parse-fallback wiring predates this PR (only the ksefNumber prop was added on this line).
…view notes Addresses piotrswierzy's two non-blocking review notes on #1528: - Add scope="col" to the line-items and VAT-summary table headers for proper screen-reader header association. - KsefFa3View now calls an optional onParseError callback when the XML can't be parsed, and KsefInvoiceDetailSection uses it to fall through to the "Preview failed" copy instead of rendering an empty preview area for a fetched-but-unparseable document. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
|
Addressed both SUGGESTIONs in 52bdf8f:
Not rebasing onto |
…ualization Fixes two parser bugs: P_1/P_2 were read swapped (FA(3) defines P_1 = issue date, P_2 = invoice number) and party names were read from NazwaSkrocona while the builder emits Nazwa (NazwaSkrocona kept as a fallback for foreign documents). Extends the parser with party addresses (AdresL1/AdresL2/KodKraju), KodWaluty, P_6, P_8A, P_9A, RodzajFaktury, Platnosc (TerminPlatnosci, FormaPlatnosci, RachunekBankowy/NrRB), PrzyczynaKorekty, and the corrected-invoice number from DaneFaKorygowanej. Rebuilds the layout as a theme-aware document: header with invoice number + type subtitle + KSeF number, two-column Seller/Buyer blocks, details row, line-items table with render-derived gross and conditional Unit / Net-unit-price columns (hidden on pre-backend-change documents), per-band VAT summary with gross, total due with currency, payment section reusing the 1-7 FormaPlatnosci vocabulary (#1311), and the KOR before-lines collapsed section kept from #1364. Closes #1526 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
- Match FA(3) elements by localName (wildcard scan) instead of qualified tag names - prefixed documents (tns:) parsed to null before; happy-dom also mis-implements getElementsByTagNameNS so the NS API was not usable - Key PAYMENT_FORM_LABELS by KsefFormaPlatnosci from ksef-setup.schema (compile-time exhaustiveness with the #1311 vocabulary, guarded by the forma-platnosci drift script); unmapped codes render the raw code instead of dropping the Payment-form row - Accept the KSeF number as a prop from the invoice record (the builder never embeds it in the document); keep the XML parse as a fallback for foreign documents only - Normalize 0 KR / 0 WDT / 0 EX line-table rates to the VAT-summary band labels - Parse and render Platnosc/Skonto (early-payment discount) - Compute the conditional Unit / Net-unit-price columns per table so a KOR's before rows keep their values independently of the after rows - Tests: prefixed-document fixture, TerminOpis descriptive payment term (the only branch the OL builder emits), skonto row, unmapped payment code, per-table column visibility Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
- Memoize parseFa3Xml on xmlText - the parse is a full DOMParser pass plus ~35 subtree scans and the parent re-renders on unrelated state - Fix React key collisions: prefix the index fallback for line rows (lineNo '2' vs index 2 stringify identically) and key address lines by position (AdresL1 may equal AdresL2) - Parse DaneFaKorygowanej/NrKSeFFaKorygowanej and render the corrected original's KSeF number in the correction block (the builder emits it whenever the corrected original was cleared - the normal OL case) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…view notes Addresses piotrswierzy's two non-blocking review notes on #1528: - Add scope="col" to the line-items and VAT-summary table headers for proper screen-reader header association. - KsefFa3View now calls an optional onParseError callback when the XML can't be parsed, and KsefInvoiceDetailSection uses it to fall through to the "Preview failed" copy instead of rendering an empty preview area for a fetched-but-unparseable document. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
52bdf8f to
4c04413
Compare
What
Rebuilds the FA(3) preview component (
apps/web/src/plugins/ksef/components/ksef-fa3-view.tsx) so it mirrors the official KSeF/MF invoice visualization instead of the old flatslot-rowlist.Two parser bugs fixed
invoiceNumber = P_1andissueDate = P_2, while FA(3) definesP_1= issue date andP_2= invoice number (the builder emits them correctly). The preview showed the date as the invoice number and vice versa. Regression-tested with a builder-shaped fixture.NazwaSkrocona, but the builder emitsNazwa(FA(3)DaneIdentyfikacyjne) - seller name rendered as "-" and B2C buyers disappeared entirely. Now readsNazwawithNazwaSkroconafallback for foreign documents.Parser extensions
Podmiot1/Podmiot2addresses (AdresL1,AdresL2,KodKraju),KodWaluty,P_6(sale date),P_8A(unit),P_9A(net unit price),RodzajFaktury,Platnosc(TerminPlatnoscidate orTerminOpis,FormaPlatnoscicode,RachunekBankowy/NrRB),PrzyczynaKorekty+ corrected-invoice number fromDaneFaKorygowanej/NrFaKorygowanej. VAT bands now cover the builder's fullP_13_x/P_14_xset (23/8/5, 0 KR/WDT/EX, zw, np I/II, oo).Layout (theme-aware, tokens only)
RodzajFaktury("Standard invoice" / "Correction invoice"), KSeF number when present# / Description / Net unit price / Qty / Unit / VAT rate / Net total / Gross total; gross derived at render (net * (1 + rate), non-numeric bands likezwrender gross = net); VAT rate displayed with%. TheUnitandNet unit pricecolumns render only when at least one current line carries them, so documents issued before the backend emittedP_8A/P_9Aget no dash-only columnsPlatnoscis present - form label from the 1-7FormaPlatnoscimap ([FEATURE] Integration — emit FA(3) Platnosc (payment method, bank account, payment term, skonto) in KSeF invoices #1311 vocabulary), payment term, bank accountStanPrzed=1"before" rows stay in the separate collapsed section ([TECH-DEBT] Invoicing correction issuance must stamp a distinct document number from the original #1364 behavior preserved).ksef-fa3-view__*classes inapps/web/src/index.cssunder a bounded section comment, design tokens only (no new tokens), ~800px centered document containerHow to test
pnpm --filter @openlinker/web exec vitest run ksef-fa3-view ksef-invoice-detail-section- 29 tests pass (fixtures: full plain invoice, minimal legacy document, KOR with before-lines, payment mapping, parse-failure contract)pnpm --filter @openlinker/web type-checkandpnpm --filter @openlinker/web lint- clean (0 errors)node scripts/check-design-tokens.mjs- passesCloses #1526
🤖 Generated with Claude Code