fix(prestashop): read the order's real currency instead of hardcoding EUR - #2439
Conversation
… EUR Every order ingested from PrestaShop was recorded with `currency: 'EUR'`, whatever the buyer actually paid in. The value was a literal in `prestashop-order.mapper.ts` whose `// Default, can be configured` comment was never true - nothing overrode it, and `Connection.config.currency` is the product-sync default that the order path never consults. The amounts were correct; only the denomination was wrong. That matters because the ADR-040 reporting stamp multiplies the order total by the NATIVE currency's rate, and the invoicing and fiscalization mappers pass the same value through to KSeF / inFakt / Subiekt verbatim. Resolution chain, first answer wins: the order's own `id_currency`, then the shop default (`PS_CURRENCY_DEFAULT`), then refuse with the existing `PrestashopCurrencyUnknownException` (#2139), which the retry classifier already treats as terminal. `config.currency` is deliberately not in the chain - it means "product-sync default" today and the shop-default read is a strictly better fallback because it cannot drift from the shop. The resolution lives in the adapter, not the mapper: `mapOrder` is synchronous and does no I/O by contract, so `MappedPrestashopOrder` omits `totals.currency` entirely - writing a literal there is now a compile error rather than a plausible default. Also included, since re-polling cannot repair them: - a migration nulling the six FX columns on PrestaShop-sourced rows whose snapshot currency has already been corrected away from EUR, so the FX sweep re-stamps them. Scoped by that predicate rather than by operator discipline: run before the re-poll it is a verified no-op. - `readPrestashopCurrencyById`, the by-id sibling of the existing by-ISO read, with `prestashop-shop-currency.resolver.ts` refactored onto it. Closes #2277 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
Two corrections to the migration, both found by running it against a live PrestaShop stack rather than by reading it. 1. The snapshot test was a PROXY for the damage, not the damage. It cannot tell a stamp computed while the snapshot still said EUR from one an already-fixed deployment computed correctly minutes ago, so it re-opened correct figures too. Observed live: an order ingested after the fix, carrying a PLN->EUR rate and the right converted total (49 PLN -> 11.33 EUR), matched the predicate. The scope now additionally requires the stamp to be one of the two shapes a native EUR can produce - a conversion whose rate row reads `fromCurrency = 'EUR'`, or the same-currency short-circuit that writes `reportingCurrency = 'EUR'` with no rate row at all. A rate-only test would have missed that second shape entirely, and it is the worse one: the total was copied across unconverted. Live dry-run on the same data goes from 1 row to 0. 2. The docblock claimed `order_records.currency` and `order_line_items` self-heal on re-poll. Neither exists on this branch - #1985's analytics read model is not merged here, as `OrderRecordRepository.listDistinctNativeCurrencies` documents in place. The native currency lives only in `orderSnapshot.totals.currency`, which is what self-heals. Also adds the spec that pins both predicate arms. The WHERE body is parsed out of the real emitted statement and evaluated against row fixtures - the approach the sibling 1836000000000 spec established, since nothing in CI executes a migration. The top-level split is parenthesis-aware because the damage clause is itself a parenthesised disjunction, and an unrecognised clause throws rather than being skipped, so the spec cannot keep passing over a predicate it no longer describes. Verified by mutation: deleting either arm fails exactly one case. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…d in Two orders, not one. A single PLN order would pass just as well against a fix that swapped one hardcoded literal for another, or that always substituted the shop's default currency. Two orders synthesized in two DIFFERENT currencies, in the same run against the same shop, can only both hold if the value is read per-order - which is the property #2277 actually claims. `synthesizeOrder` gains a `currencyId` option threaded onto BOTH the cart and the order, since PrestaShop stores `id_currency` on each and an order whose cart disagrees is a shape no storefront checkout produces. The id is resolved at run time by the new `getCurrencyIdByIso` (mirroring the existing `getCountryIdByIso`) rather than hardcoded - currency ids are per-install, so a literal would denominate the order in whatever that shop happens to carry at that position. A shop missing either currency skips with that reason instead of asserting something weaker. Its own `order-ingestion` project with `retries: 0`: the spec synthesizes real customers, addresses, carts and orders, and the `orders` project's own comment states its strictly-read-only contract. A retry there would create a second order and assert against the wrong one. Verified live: passes on the fixed build; on a build reverted to the pre-fix behaviour the same order that reads PLN is recorded EUR while the shop says PLN. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
|
Verified on a live PrestaShop + OpenLinker stack. Write-up with screenshots: https://claude.ai/code/artifact/d8df1c1b-c9b9-4fa1-a0fa-242c97b4b751 The short version. The currency follows the order, not a default. One order ( Three builds, same order:
The migration dry run found a real defect in its own first version. It matched one row, and that row was stamped correctly (a PLN→EUR rate, 49 PLN → 11.33 EUR). The snapshot test was a proxy for the damage rather than the damage, so it also re-opened correct figures. It now requires the stamp to be one of the two shapes a native EUR can produce — a conversion whose rate reads One thing the e2e did not prove. Under the reverted build the spec went red on an ingestion timeout rather than on the currency assertion — the stack's single execution slot was backlogged. It caught the regression, but not for the right reason, so the reproduction above was confirmed directly against the database instead. |
piotrswierzy
left a comment
There was a problem hiding this comment.
Tech-lead review — ✅ Approve
Scope: 16 files, +1095/−29. PrestaShop order-currency resolution (adapter + resolver + shared read), mapper contract narrowing, FX-stamp backfill migration + spec, e2e project.
What's right, and non-obviously so
- The type is the fix, not the value. Narrowing
mapOrder's return toMappedPrestashopOrder(Omit<OrderTotals,'currency'>) makes re-introducing a literal a compile error rather than a plausible default. That is a structurally stronger fix than assigning the right string, and it's the reason this defect can't recur in the mapper. - Placement respects the layer contract.
mapOrderis synchronous/no-I/O by contract, so resolution correctly moved to the adapter — which is also what covers the webhook path and the reconciliation poll in one place. - The third rung refuses rather than substitutes, consistent with #2139 on the create side, and the refusal is never cached so a back-office fix is picked up on the next attempt. The read-failure split (404 →
undefinedvia the shared read; every otherPrestashopApiExceptionpropagates) keeps a transport blip retryable instead of collapsing into a false refusal.normalizeCurrencyIdtreating PrestaShop's unset-FK0as absent rather than as an id is exactly the distinction that would otherwise produce spurious refusals. - The migration is safe by predicate. The damage arm (
exchangeRateId → fromCurrency = 'EUR'OR the same-currency short-circuit withreportingCurrency = 'EUR') is what makes it a no-op before re-poll, and the short-circuit branch is the one a rate-only predicate would have missed — the worst case, since that total was copied across unconverted. The migration spec parsing the real emittedWHEREand evaluating it structurally (noeval) is the right shape given nothing in CI executes a migration. - Two orders in two currencies in the e2e is the correct assertion; a single-order test would pass against a shop-default substitution.
🟡 IMPORTANT — one claim the wiring does not currently support
libs/integrations/prestashop/src/application/prestashop-adapter.factory.ts (new field comment):
a process-singleton field so the per-(connection, id_currency) cache of order denominations survives across the adapter instances built per capability resolution.
That is not what happens. prestashop-plugin.ts:155 constructs new PrestashopAdapterFactory(...) on every createCapabilityAdapter call, so the factory — and both resolver fields on it — are per-build. PrestashopShopCurrencyResolver's own docblock says so explicitly ("the cache is discarded after the single call each adapter build makes … it is the correct value the moment the factory is genuinely held as a process singleton"), and PrestashopOrderCurrencyResolver's docblock repeats that caveat correctly. Only the new factory comment asserts the opposite.
The consequence is small but real and worth stating rather than implying away: today this adds one GET /currencies/{id} per ingested order, serial (awaited before hydration, deliberately). The adapter's inline comment — "The read is cached per (connection, id_currency), so the added round-trip amortises away" — asserts an amortisation that does not occur under the current wiring either.
Not blocking: one extra WebService read per order is well inside the PrestaShop rate budget, and the ordering choice (resolve-then-hydrate) is correctly argued — refusing early costs one read instead of a full hydration, and a rejected promise held across the hydration awaits would be an unhandled rejection on every other failure path.
Suggested fix (comments only, no behaviour change): align the factory comment with the two resolver docblocks — say the field placement anticipates a singleton factory and that the cache is per-build today, and soften the adapter's "amortises away" to the same conditional. The pattern in this repo is that a comment asserting a property the code does not have is worse than no comment, and both resolvers already model the honest version.
🟢 SUGGESTION
PrestashopOrderSourceAdaptertype-hints the concretePrestashopOrderCurrencyResolverrather than an interface. Consistent with the existing provisioner fields on this factory (customerProvisioner,addressProvisioner), so it is not a deviation — noting only that the spec has to cast (as unknown as PrestashopOrderCurrencyResolver) at eight construction sites, which a narrowIPrestashopOrderCurrencyResolverwould remove. Fine to leave.
Checks
- Layering: resolution in infrastructure, refusal via an existing domain exception, no core change. ✅
- Naming:
*.resolver.ts/*.spec.ts/ migration{timestamp}-{description}.ts+ matching class suffix. ✅ - Migration:
up()+down()both present;down()is a documented deliberate no-op with a stated reason (discarded figures unrecoverable) rather than an omission. ✅ - Tests: resolver chain, cache keying/TTL/clear, refusal-not-cached, non-404 propagation, adapter wiring, migration predicate with mutation-verified arms, e2e. Edge cases (
0, blank, numeric id, missingiso_code, non-string snapshot JSON) all covered. ✅ - No
anyoutside test casts; no raw SQL interpolation (all literals). ✅
The Step-5 correction in the description — terminal dead, not business_failure, since outcome is only set on the succeeded path (ADR-007) — is right, and correcting the issue's wording rather than the code was the correct call.
Approving; the comment alignment above can land in a follow-up commit here or separately.
|
Cross-PR heads-up, found while reviewing the OMS wave in parallel — merge-order conflict on the migration prefix, not a defect in this PR. Open PR #2438 ( Whichever merges second fails Suggested sequencing: land this one first (single fix, live-verified backfill, no downstream branches) and let #2438 renumber its pair to No action needed here unless #2438 merges first. |
Two conflicts, both additive, both sides kept: the check:invariants chain gains main's allegro-seller-defaults guard alongside this branch's shipping-tax-split guard, and the PrestaShop adapter factory keeps main's order-currency resolver field next to this branch's tax-rate resolver. Migration prefixes renumbered, which is Piotr's blocking item 2. #2439 landed on main claiming 1840000000000, colliding head-on with this branch's add-product-tax-rate. This epic's block moves to 1841000000000 through 1841000000004, and the analytics migration inherited from the base branch moves to 1840000000001, which keeps it ahead of the line-item alter that depends on it. #2438 is still open and also claims 1841000000000, so whichever of the two lands second has to renumber again; this branch is now correct against main as it stands. Green: type-check clean, check:invariants clean, libs/core + apps/api 4541 tests, prestashop 591. Refs #2245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
… to the invoice and the receipt (#2260) * chore(tax): open the per-line tax rate epic branch Tracking branch for epic #2245. Children merge here; this branch merges to main only when every child has landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * docs(tax): ADR-052 per-line tax-rate resolution, provenance and rounding ownership Records the rule the #2245 epic implements: the rate arrives from the ProductMaster with the product, the marketplace is a fallback when the master does not know, and when neither knows the document is held rather than guessed. OpenLinker never computes a rate and never computes an amount excluding tax. Seven decisions: the shop-then-channel resolution chain, three answer states (0 is an answer, 'not yet checked' is a fourth read state that suggests a sync rather than blocking), percent-as-string representation with provenance-only country, projection-at-sync storage with the order snapshot as the only place a rate is settled, adapter-owned rounding, gate semantics (a missing rate blocks and also refuses the manual paths; a shop-versus-channel mismatch does not block and is not a SalesDocumentGateBlockReason at all), and the non-goals. Numbered 052, not the 050 the issue names: 050 and 051 are reserved by the ADR README for the #2162 async-work-layer epic and the index directs the next new ADR to 052. Also settles the two edits ADR-014's amendment owed: its 'proposal, not a recorded refinement' preamble is dropped and its Proposed-while-its-decisions-shipped status resolves to Accepted. Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(invoicing): settle the tax-rate notation on percent-as-string InvoiceLine.taxRate had no single reading. Core divided by 100 unconditionally, so '0.23' meant 0.23%; the inFakt adapter switched on n > 1, so '23' and '0.23' both resolved to 23% while a genuine 1% rate resolved to 100%. On a 123 PLN invoice the two readings diverge by 22.72 PLN. The blast radius is narrow only because the mapper still emits an empty taxRate and each adapter substitutes its own default. Once #2054 lands, every invoice line travels through this field, so the notation is pinned first and on its own. A guess is the wrong shape here: the writer knows which notation it used, so an ambiguous value is a defect upstream and must surface. A new pure module, tax-rate-notation.types, states the contract once and every reader goes through it. Fractional notation (numeric, strictly between 0 and 1) raises FractionalTaxRateNotationError rather than being multiplied by 100 - '0.23' is indistinguishable from a genuine 0.23% rate, so normalising it would invent a value nobody stated. '0' is deliberately not fractional: a zero rate is a real answer, and PrestaShop already distinguishes it from unknown. Readers aligned: core rateFraction, the inFakt tax_symbol map and gross-to-net split, the Subiekt stawkaVAT passthrough. KSeF already rejected a fractional code, since FA3_TAX_RATE_MAP has no such key - that is now pinned by a test rather than left incidental. The empty-string path is untouched on every adapter; #2257 removes those defaults. Closes #2247 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(web): shared-UI primitives for the tax-rate states Three additions the per-line tax-rate surfaces need, and nothing else. StatusBadgeTone gains 'conflict' - two sources disagreeing about the same fact, which needs attention but is not an error because the document still issued. The family has no -fg member, so the rule reads --status-conflict-strong; the ramp base gives roughly 2.5:1 and fails contrast, and nothing would have caught it since the token checker is one-directional and a missing token resolves to nothing silently. The family already had a consumer through a className override (.status-badge.delivery-rider-chip--not-connected), so that override is migrated onto the tone in the same change rather than left as a second owner with a second text token. The className survives as a layout hook, so the existing delivery-chip assertions still hold. AlertTone gains the same member. Alert picks role=alert only for 'error', so a conflict alert lands on role=status - the right politeness level for a non-blocking advisory, now following from the tone rather than from a per-call choice. SalesDocumentBlockCopy['tone'] widens to match, since its value is passed straight into Alert. AbsentValue moves out of listings-list-page into shared/ui. Absence versus zero is the central claim of this epic - a rate of 0 is a real answer and 'no rate' holds the document - so it needs one implementation. It renders the wording visually hidden rather than as an aria-label, because aria-label on a bare span is prohibited and commonly dropped. Adding the tone member compile-forced BLOCK_TONE_FOR_BADGE and TONE_CLASS, the timeline's two exhaustive records; both gain the member together with the TimelineEvent tone and the .order-activity__dot--conflict rule. Closes #2253 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(products): per-line tax rate on the order contract and the master reads Core had nothing to carry a tax rate in. OrderItem and IncomingOrderItem have no tax field, and the only signals on an order are one aggregate OrderTotals.tax and an order-wide taxTreatment - neither of which can describe a mixed-rate basket. So the mapper emits an empty rate and each provider adapter guesses. This gives core the field, the vocabulary, a way to ask the two masters, and a place to keep the answer. The rate is a string code (23, 8, 5, 0, zw, np, oo), not a number: 0, exempt, reverse charge and intra-EU zero are four different things on a document and all look like zero. Notation is percent-as-string, settled in #2247. Storage is a projection pulled at product sync onto products and product_variants, exactly as price and currency already are. Issuance must not depend on the shop being reachable, and 'which products lack a rate' has to be a query rather than a crawl - both partial indexes exist for that. Three distinctions are load-bearing and each has a test. Zero is an answer, never an absence. A read that establishes nothing reports kind: 'unknown' and is stored as a null code, never as '0'. Never-checked is not no-rate. taxRateReadAt separates them: null timestamp means nobody has asked, a timestamp with a null code means the master answered and has none. Without that split, the day this ships the whole catalogue reads as incomplete and the pre-rollout coverage count measures nothing. A variant override always wins where the shop carries one - the open question the epic left for this child. It is not a conflict to arbitrate: a variant value is the more specific statement of the same fact, and the shop had to be edited deliberately for the two to differ. An absent override means 'no opinion', so a product-keyed master resolves through the product row unchanged. A third resolution arm, 'inherited', exists for a WooCommerce variation whose tax_class is 'parent': storing nothing is honest, where recording unknown would show the variant as rate-less and copying the product's code down would leave a duplicate that goes stale. PrestaShop delegates to the existing PrestashopTaxRateResolver rather than re-walking the three-hop chain, so the sync path and the order-create path cannot disagree about one shop. Its transport unknown is re-raised rather than reported as unknown - a failed call says nothing about the shop's configuration, and recording it would freeze a false 'no rate' onto the row. WooCommerce resolves class name to the store's rate table for the store's own country; tax_status 'none' is a resolved zero, no row is not-configured, and several different rates is ambiguous rather than a pick, because picking would be OpenLinker computing tax. recordTaxRate is a separate writer from upsert on both repositories, and the columns are deliberately absent from toOrm - the sync upsert carries no rate, so round-tripping them would blank a value the tax read just wrote, and a blanked rate holds documents. Same single-writer rule as order_records.cancelledAt. Order ingestion settles the rate onto the stored snapshot, shop first and channel second, and carries taxSource plus taxRateReadAt alongside it so a reader can tell no rate, never read and pre-rollout apart (#2245 F3). One additive migration, nullable, no backfill. Closes #2054 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(invoicing): carry the tax rate onto the document and hold when it is absent The mapper stops emitting an empty taxRate and carries the code the order line was settled with. It stays a passthrough - no derivation, no default - and an empty value still reaches the adapter unchanged, because a mapper that threw would turn an operator-fixable data gap into a failed job. The gate is what refuses. missing-tax-rate joins SalesDocumentGateBlockReasonValues on both sides of the mirror. It is the first reason for which 'this cannot be issued' is literally true: every other one means 'auto-issue did not happen', and issuing by hand past those is a legitimate operator action. Issuing past this one means a provider substituting a guessed rate onto a real fiscal document, so it closes the manual paths too - InvoiceService.issueInvoice refuses before the lock and before any persisted state is touched, POST /invoices answers 422 with the reason and retryable:false, and bulk issue reports a named ineligibility rather than attempting it. The check runs on the COMMAND rather than on the order, so no caller can bypass it by composing lines itself, and the correction path - which composes its lines from an already-issued document - is unaffected. It runs BEFORE the trigger-model gate, because on a manual connection both apply and reporting the weaker reason would leave an operator clicking a button that refuses. A zero rate passes everywhere. Export, intra-EU and exempt goods are legitimately zero, and blocking on them would hold documents for a correctly configured catalogue. Shipping now splits across the rates in a mixed basket, in proportion to line gross, with the rounding remainder on the largest part so the parts sum exactly to what the buyer paid. A single-rate basket still yields one line. This is division, not tax computation: core groups an amount it was given and cuts it into parts that add back up to it. A single line with no rate makes the mix unknowable, so the split refuses and the whole document waits. Two instants record when a hold started and ended. The reason column is level-triggered and nulled the moment it clears, so without them the operator-facing age has no clock and the 'the rate arrived, the invoice issued' timeline entry has no instant to hang on. They are derived from the TRANSITION inside the same UPDATE, because only that statement knows both the old and the new value; blockedAt is stamped on none-to-blocked alone so a change of reason inside one episode does not reset an age somebody is watching. Closes #2248 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(products): append-only tax-rate provenance journal A mutable 'rate source' field only says how things stand now. It cannot answer when the shop changed the rate, what OpenLinker last wrote onto a channel, or whether somebody overwrote it afterwards - and that last question is what makes a shop-versus-channel disagreement attributable rather than mysterious. So provenance is a journal: one row per CHANGE, never one per read. The catalogue sweep runs every twenty minutes and most rates never move, so writing unconditionally would grow the table by the size of the catalogue per tick and bury the handful of rows that matter. isNewTaxRateObservation owns that rule in one pure place. The dedup compares the value, the origin AND the frozen flag. A seller freezing a field without changing its value is a real change in what the value means - it is now something a person set - and losing it would leave the disagreement surface unable to say so. Append-only by construction, not by convention: the port declares append, findLatest and findLatestPerConnection, and no update, upsert or delete. A journal whose rows can be edited cannot answer the question it exists for, so adding a mutating method is not a refactor. Same discipline as ExchangeRateRepositoryPort. Origin distinguishes shop, channel and written-by-us. The third is the reason the journal exists at all: it records OpenLinker's own write onto a channel, so a later channel observation carrying a different value proves somebody changed it after we did. Master product sync records every rate it observes. The write is best-effort and separate from the catalogue write - the journal is provenance, so losing an entry costs an audit trail rather than a rate, and failing the sync over it would trade the thing that matters for the thing that explains it. One index serves both reads, since the latest-for-one-connection lookup and the latest-per-connection listing walk the same prefix. Note on scope: taxSource and taxRateReadAt already reach the ORDER SNAPSHOT line (#2054, epic F3). The order_line_items half of this issue is not implemented here because that table does not exist on main - it is defined in #2014, which is still open. Closes #2250 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(integrations): read the channel's tax rate and propagate the shop's onto offers The channel half of the resolution chain, and the write back. READS. Erli reports a required per-line taxRate on every order line and OL's own type already modelled it while the mapper discarded it - it now reaches the order contract. Allegro's lineItems[].tax has been live since March 2024 and the OL type had no field for it at all; it is modelled and read. Both map their platform vocabulary to the neutral code inside the adapter, tested in both directions, with a round-trip test over every value the Erli mapper claims to support. An unreadable value maps to absent, never to '0'. A rate OL cannot read is not a zero-rated sale, and Erli's enum is category-dependent and may gain values. WRITES. The shop's rate is stamped onto CreateOfferCommand by OfferBuilderService in the same pass that already carries price and stock, read from OL's own catalogue projection so publishing does not depend on the shop being reachable and the offer carries the same rate an invoice for it would. There is no OL-side rate field to type into, deliberately: a rate entered in OL would be a fourth source no master or channel could be corrected from. Nothing is published with the rate omitted. That is precisely how the rate-less offers this epic exists to fix were produced, and the failure surfaces months later on somebody's invoice rather than at publish time. Allegro refuses on no rate, on an exemption code (its rates array carries numbers), and on a rate the category does not allow - the last naming the permitted values, because when Allegro says the category allows 23% and OL sent 5% the shop record is almost certainly the wrong one. Erli refuses on no rate and on 'oo', which its enum cannot express; omitting would publish a product Erli then marks not-buyable with missingTaxRate, which nobody sees. The permitted-values read is best-effort. A failure to LIST is not a failure to publish, so it warns and proceeds with the shop's value; Allegro validates the body itself, and refusing because a secondary discovery call was unavailable would be worse than the check. The update path propagates too, so an offer's rate follows the catalogue instead of freezing at first publish. There an unmappable code is dropped rather than raised: an update that cannot state its tax is a partial update of an offer that already sells, and raising would take a title or price fix down with it. A frozen Erli taxRate is skipped, force is never sent, and it is reported once per connection at info level rather than as a recurring publish error - a seller freezing the field is a deliberate decision, and it is exactly the signal that makes a later disagreement attributable to a person. Closes #2249 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(orders): mark pre-rollout orders and measure catalogue rate coverage Two rollout chores that keep the first day honest. HISTORICAL ORDERS. An order ingested before per-line rates existed carries none on any line, and the document issued for it used whatever the provider adapter defaulted to. It is MARKED rather than blocked: blocking would stop history nobody is going to retrofit, and nothing about it can be corrected after the fact. The marker's only job is to keep a net-revenue figure honest - such an order is excluded from one rather than presented as a confirmed rate, because there is nothing to back-compute from. Recorded per RECORD, not per line. The lines live in a jsonb snapshot, so a per-line marker would rewrite every snapshot in the table for a value that is uniform across an order and that no surface renders per line. The frontend deliberately shows nothing for it - it appeared in one place with no action attached, so it is analytics data rather than a badge. This backfill is not the one the epic forbids. What must never be invented is a tax RATE; recording that an order predates the feature is a fact about OpenLinker's own history, and it is exactly what stops a later reader mistaking a defaulted rate for a stated one. The migration is idempotent by construction: it marks only rows where no line has ever carried a rate, so an order ingested between two runs is not retroactively called historical. There is no cutover instant to get wrong. COVERAGE. Counts are per shop connection, because that is the unit an operator fixes - 'the catalogue has no rates' is not actionable when three shops feed it and only one is incomplete. A product mapped on two connections counts under both, which is the honest answer since both shops would have to carry the rate. The grouping joins identifier_mappings by table name, the read-model posture the stock aggregation in findMany already uses, rather than importing another context's ORM entity. docs/operations/tax-rate-coverage.md carries the query, what the three states mean, how to read the answer, and why this gates #2257: until the defaults come out a rate-less product still produces a document with a guessed rate, and after it produces nothing at all. Running the two in the wrong order turns a slow, visible data problem into an immediate outage. The 2026-08-21 baseline measured zero coverage and is recorded on the issue. Closes #2256 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(fiscalization): hold a receipt when a line has no tax rate The same rule as the invoice, on the fiscalization path. FiscalRegistrationService.register refuses before the read gate and before any row is written, so a held sale leaves no pending record to reconcile. The order-to-command mapper stops emitting an empty rate and carries the line's own, and shipping splits across a mixed-rate basket exactly as it does on an invoice - a receipt has to state a rate per line too. The accepted cost is LATE registration, and it is chosen deliberately. The alternative is a receipt carrying a tax letter nobody confirmed, which reaches the buyer and the daily report and cannot be recalled. A late registration can be completed; a wrong one has to be corrected. THE REVERSAL POINT IS ONE BRANCH: the assertEveryLineHasATaxRate call in register. Nothing else in this context consults the rate, and the exception's docblock says so, so reversing this is a one-line change rather than a redesign. The eparagony adapter's own empty-rate arm is kept for the same reason. The per-connection tax letter stays supported but stops being a fallback. Its docs no longer describe it as "what we use when we do not know": core refuses one step earlier now, so that arm is unreachable while the gate stands, and presenting it as a safety net would describe a trade OpenLinker no longer makes. splitShippingAcrossRates moves from invoicing into sales-documents. Both document contexts need it and a fiscal receipt is not an invoice, so neither could own it for the other - sales-documents is the dependency-free leaf that exists for exactly this case, and the barrel-purity spec already enforces that it stays one. The module imports nothing, so the property holds. On the elapsed-time signal: a held sale has no fiscal record to hang a clock on, and it does not need one. The block is recorded on the ORDER as missing-tax-rate with salesDocumentBlockedAt (#2248), and that reason is document-kind agnostic - it says the order has no fiscal document, whichever kind was due. A second clock would be a second answer to one question. Closes #2252 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(invoicing): copy the per-line net back from the issued document OpenLinker computes no net amount (ADR-052), so a stored per-line net has to be the document's own figure rather than a recomputation. Otherwise the record disagrees with the paper by a grosz here and there and no reader can tell which is right. Nothing carried a provider-computed net before this. documentContent held core's own recomputation, marked NON-AUTHORITATIVE in the code, and issuedLineSnapshot carried only unitPriceGross plus taxRate. So this is mostly an adapter-contract change: IssueInvoiceResult gains an optional documentLines, matched to the command's lines by 1-based line number. The fallback is per LINE, not per document, so a provider that reports some lines and not others still contributes what it has. Shipping lines are part of the numbering because they are real document lines; what "skip shipping" means is that they have no ORDER line to transcribe onto, not that they shift the mapping. A correction reports its own amounts and they overwrite the stored ones, so the record follows the latest effective document rather than keeping pre-correction figures the paper no longer states. inFakt reads the created invoice's own services[] - it is the calculator on that path - converting integer groszy back to PLN on the adapter side where that wire detail belongs. KSeF has nothing external to copy, so the adapter reports what it wrote. The figures come from the same lineNet the XML uses, so the reported net cannot drift from P_11, and the offline pending-submission path reports them too: that window is about transmission, not about what the document says. The KSeF rounding bug is fixed and the rule is stated once. P_9A is TKwotowy2, which permits EIGHT fraction digits, and the builder was rendering it through the 2dp money(). On 100 x 1.99 at 23% the line net is 161.79, but a unit net rounded to 1.62 multiplies back to 162.00 - the document contradicted itself by 21 grosze and a reader checking P_9A x P_8B == P_11 was right to complain. The rule: the LINE is the unit of rounding and the unit price is derived from it at the schema's full precision, never the other way round. The buyer paid a gross line amount, so the line's net is what anchors to a real figure; a unit net is a derived display value that only sometimes has an exact 2dp form. Closes #2251 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(web): tax-rate states on the orders list, order detail and invoice panel Every tax-rate state on the orders surfaces, plus the backend contract two of them needed. BACKEND. A rate conflict gets its OWN field, its own count and its own filter, not a gate reason (epic F1). invoicingBlockedBadge returns null whenever an invoice plausibly exists, and a conflict does not stop the invoice, so routing it through the block machinery would make the badge unreachable on exactly the rows it describes - and SalesDocumentAttentionReasonValues would have counted it inside salesDocumentBlocked, against its own chip. The evidence is taxRateChannel on the order line, written ONLY when the channel disagreed with the shop. Its presence is the conflict, so no reader compares two fields and nothing goes wrong when only one system answered. The summary also reports the oldest still-held instant, so the blocked chip can carry an age. ORDERS LIST. A Rate conflict chip driven by its own count, mounting on filterActive || count for the nine-line reason the invoicing chip already documents: gating on the count alone unmounts the only way to clear the filter the moment remediation succeeds. No tone on it - .chip.chip--active overrides every .chip--{tone}, so an inactive conflict chip would read as pressed beside an active accent one. The age folds into the blocked chip's own label rather than becoming a third dotted badge in a row that already carries two SLA badges. The conflict badge uses the listings page's RowBadge shape - visible label plus the wording in a visually-hidden span - because the hint is the only statement of the fact on this surface and aria-label on a bare span is prohibited and commonly dropped. A new empty arm sits ABOVE both single-filter arms: with two filters active and no rows, "nothing is blocked from invoicing" would be a statement about a set the other filter narrowed. liveRegion stays polite, like the neighbour it sits beside. ORDER DETAIL. A tax-rate column on the line-items panel, following the epic's central claim: an answer is text, only an exception is a badge. A rate, a zero and an exemption all read the way the money beside them reads, with a provenance caption; only no-rate and conflict get colour. No hideBelow - this panel passes no cardView, so the class would simply delete the blocking state on a phone, on the one screen that diagnoses it. Only flagged and Jump to next flagged are borrowed from the bulk review step for long orders. The totals panel's Tax row keeps its snapshot value and gains a caption naming whose number it is, because Allegro and Erli report zero there and it will visibly disagree with the line rates. INVOICE PANEL. Three remedy branches, not one sentence: a blank rate on a mapped product, an item in no catalogue (fixing the offer will not release this order - the marketplace stamped the rate at purchase), and an ambiguous shop tax class. Plural-safe with a count, because a forty-line order with six rate-less lines cannot be told about one product. The Issue invoice button is disabled with the reason ON the control (epic F2). It renders on invoiceSettled && not-issued independently of the block copy, so until now a red "will not be issued" alert sat above a live button that issues it - and the backend now answers 422. The conflict alert is informational and lands on role="status" via the conflict tone. The shipping split preview lives here rather than beside the line items, because shipping has no order line: it exists only once a document is composed. One unknown line rate collapses it to a single waiting row rather than showing a proportion OpenLinker cannot compute. Fix and re-check names the latency and links to the products list rather than opening a sync dialog here: the connection to sync is the SHOP that owns the product, which this panel does not know. TIMELINE. The block entry is dated from the persisted instant instead of timestamp: null, and a release entry exists at all - by the time an order is released the reason is gone, so nothing else records that it was ever held. Closes #2254 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(web): tax-rate states on products, publish wizards and the receipt panel The surfaces where an operator FIXES a rate, plus the read contract they needed. BACKEND. Product and variant tax rate reach the wire, with taxRateReadAt beside the code - it is the only thing separating "the shop has no rate" from "nobody has asked", and the two need different remedies. A taxRateState filter partitions the catalogue into missing / not-checked / known, expressed against the two columns rather than a stored enum so no writer can leave them disagreeing with the data they describe. PRODUCTS. One KpiCard tile, not MetricCard, which is @deprecated with "no new callers should be added". The tone is COMPUTED, so a healthy install shows no colour - hardcoding it would paint a red border around a zero. The tile renders only where a ProductMaster connection exists: with no shop to ask, "312 products have no tax rate" is a statement about an install that was never going to have one. The "Not checked yet" tile is deliberately NOT built. It reads zero forever after week one, costs a permanent grid slot for a one-off migration, and would print the same number twice forty pixels from the suggestion beneath it. The sync suggestion is dismissible PER SESSION, because a permanent dismissal removes the only route to the remedy with no way back. It never claims a complete result either: syncing is per connection while "not checked" is per product, so a product mapped on two shops can be half checked. VARIANTS. Inherited is not the same as absent, and drawing it as absent sends the operator to fix the wrong record. A variant with no override of its own renders the PRODUCT's rate with an inherited caption; only a variant whose product has no rate either shows the badge. A present override wins outright, matching effectiveTaxRate on the backend. WIZARDS. Two blocker chips, and the path stays SOFT like every other blocker there: the flagged rows are excluded and the rest publish. A hard block would be a behaviour change to a shipped component for no gain, since the document gate catches a rate-less sale later anyway. The channel-managed chip is a LINK rather than the dotted-underline button that opens the editor, the same carve-out AlreadyListedChip makes, because the fix is not in OpenLinker at all. The three new rejection codes are translated for StructuredErrorList, which already renders the dotted path and the code chip unchanged. The messages name whose entry is probably wrong: when Allegro says the category allows 23% and OL sent 5%, the shop record is the likelier mistake. RECEIPT. The register button refuses with the reason on the control, and an alert says the connection's tax letter is not used to fill the gap - which is the whole point of #2252 and the thing an operator would otherwise assume. buildShopProductUrl returns a real link for WooCommerce and an honest null for PrestaShop, where no URL is constructible: the admin directory is randomised at install and editing needs a per-employee token. A button whose href is a guess is worse than no button. Closes #2255 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(integrations): remove the hardcoded tax-rate defaults from the providers The last child. Delete the guesses this epic exists to replace. inFakt substituted 23% whenever core left the rate empty - which was always, because core had no per-line rate to give. Subiekt substituted "23" in its line mapper. KSeF fell back to a per-connection defaultTaxRate that itself defaulted to "23". On the issued document a silent 23% is indistinguishable from a confirmed 23%, and the whole cost of being wrong lands on the seller. All three are gone. No adapter substitutes a rate; each fails loudly instead, raising the same neutral MissingTaxRateException core does so the failure is legible at every layer and keeps its existing 422 mapping. The guards are defence in depth: core refuses a rate-less command before any adapter runs (#2248), and the receipt path does the same (#2252). Nothing is lost by failing here. A rate-less line was never going to produce a document anyway - inFakt cascades an empty tax_symbol into services.gross and value.tax_values rejections (live-verified 2026-07-01), the Subiekt bridge answers "StawkaVAT jest wymagana", and resolveP12('') throws. The difference is that the failure now names the product an operator can fix instead of a wire field they cannot. The KSeF defaultTaxRate setting is retired: unread by the adapter, unwired in the factory, and no longer validated on save. There is no frontend control to retire - the field was config-only, edited through the raw connection config, which is worth stating because the issue expected one. No migration touches stored configs. A leftover value is inert, and the validator now accepts it WITHOUT checking it: validating a key nothing reads would tell an operator their value is accepted when it is ignored, and rejecting it would fail a connection save over a dead key. #2053's visibility instrumentation (warnOnEmptyTaxRateFallback) is removed rather than left reporting a substitution that no longer happens. The four tests that locked the old behaviour are inverted deliberately, each saying why in place rather than being quietly deleted: the mapper spec's empty shipping rate is reworded (the line is still emitted - dropping it would understate the total - it simply carries no rate and the gate refuses the document), and it gains the single-rate inheritance case; the inFakt and Subiekt defaults now assert a refusal, with inFakt additionally asserting that nothing was sent, so no draft is left behind; and the KSeF mapper spec asserts UnmappedTaxRateException. Closes #2257 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(orders): transcribe the per-line tax rate onto order_line_items The half of #2250 that could not exist on main: order_line_items is defined in #2014, so the epic now sits on that branch instead. The order snapshot is where a rate is SETTLED. This table is the queryable copy (#1985), so an analytics read never expands JSON to answer "which lines carry which rate" or "how much revenue was booked at 8%". All three columns travel together, and that is the point. A single nullable rate cannot separate no-rate, never-read and pre-rollout, so taxSource and taxRateReadAt ride alongside it - the same rule the snapshot line follows (#2245 F3). Transcribed verbatim, never re-derived. A copy that parsed or defaulted would make the row disagree with the snapshot it copies, and the snapshot is the one that issues documents. The existing delete-then-reinsert write path owns the whole row, so there is no second writer for these three to race with. Additive, nullable, not backfilled: a rate invented for a historical line is exactly the guess this epic removes. Rows written before the migration carry nulls until the order is re-ingested, and #2256's taxRateEra marker is what keeps them out of a net-revenue figure meanwhile. The four epic migrations are renumbered 1837000000010-13. The analytics branch already claims 1837000000000, and two migrations sharing a timestamp leaves TypeORM's ordering undefined. Two one-line import changes ride along, applied by `pnpm lint` itself (the package script runs eslint with --fix): `ITaxRateJournalService` and `OrderRecordRepositoryPort` become value imports because decorator metadata needs them. The second is on a file this branch inherited from #2014, and the rule only starts firing once both branches' code is in one tree. Refs #2250 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(invoicing): carry the per-line tax rate through the order rehydrator Found by a curl pass against a local API, not by a unit test. `orderFromReadySnapshot.readItems` is an ALLOWLIST, so a field it does not name is silently dropped. It did not name the tax fields, and every MANUAL issuance path rehydrates through it - `POST /invoices`, bulk issue, corrections. So a correctly rated order arrived at the mapper with no rate at all and the #2248 gate refused it with "1 of 1 lines carry no tax rate", pointing the operator at a product that was already configured. The auto-issue path composes its command from the live `Order` and never touches this function, which is exactly why the specs did not catch it: the two paths diverge here, and only one of them was covered. Verified end to end afterwards: an order carrying `taxRate: "5"` now passes the gate and reaches the provider, while a rate-less one is still refused 422 with `reason: missing-tax-rate`. Also renames `MissingTaxRateFinding.firstProductId` to `firstLineRef`, and says in the docblock which reference each caller supplies. The gate reads order items and passes an internal product id; the write-path guard reads an `IssueInvoiceCommand`, whose lines carry only a name, so it passes the line label. The curl pass surfaced that too - the API was returning `"firstProductId": "Printed apron"`. A field name that is true on one caller and false on the other is worse than a vaguer one, and this value is rendered to an operator and logged. Refs #2248 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * test(orders): pin the rehydrator's tax-rate passthrough A regression spec for the allowlist gap the curl pass found: `readItems` silently drops any field it does not name, and every manual issuance path rehydrates through it. Also pins that a `taxSource` outside the union is dropped rather than passed through, since the column is read as a discriminator downstream. Refs #2248 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(allegro): parse the real /sale/tax-settings shape Found against the live sandbox, not by a test. The response type was a guess and it was wrong. Rates are nested one level deeper than `taxSettings[].rates[]` - the real body groups them by country and puts the values under `rates[].values[]`: GET /sale/tax-settings?category.id=257366&countryCode=PL {"subjects":[…], "rates":[{"countryCode":"PL", "values":[{"label":"23%","value":"23.00","exemptionRequired":false}, {"label":"Select","value":null,"exemptionRequired":false}]}], "exemptions":[…]} So the parser produced an empty list against every real response, `permitted` was always `[]`, and the category check silently never fired. The publish would have gone out with a rate the category refuses and come back as an opaque Allegro validation error - exactly the failure the check exists to replace. Two details the live probe settled and the guess had no way to know. A `null` `value` is the UI's "Select" placeholder rather than a rate, so it is dropped instead of parsed to NaN. And a category with no tax options at all answers 404 with an `errors` array rather than an empty 200, which the caller's catch already degrades to "could not read" - the correct outcome. The parsing moves into a pure `readPermittedTaxRates`, pinned by a spec whose fixture is the captured live body verbatim. A guessed shape needs a captured payload; a hand-written one just repeats the guess, and a private method reached only through an HTTP fake is not where that belongs. Verified after the fix against the same live category: permitted [23], a 23% shop rate publishes, a 5% one is refused naming "23%". Refs #2249 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(allegro): send the tax rate as the exact string Allegro published Found live on the sandbox. It would have failed EVERY offer publish. Allegro matches `taxSettings.rates[].rate` against the seller's configured VAT settings as a STRING, and the match is exact. The adapter sent a number: PATCH /sale/product-offers/7781761841 {"taxSettings":{"rates":[{"rate":23,…}]}} 422 SETTING_NOT_FOUND "No VAT setting found for the rate: 23 of country: PL" (path: taxSettings) The same call carrying "23.00" succeeds. So the field is typed as a string and the value is the one Allegro itself published in /sale/tax-settings, taken verbatim when that listing was read and falling back to two decimals when it was not. `readPermittedTaxRates` therefore returns both forms - the number to compare against OpenLinker's own percent code, the string to put on the wire. Verified end to end against the live sandbox after the fix, both directions: "23" (the rate the category permits) succeeds, and a negative control with "7" comes back 422 "No VAT setting found for the rate: 7.00" - which also proves the success was a real write rather than a no-op. Also makes the update path REACHABLE. `OfferFieldUpdate.taxRate` shipped in the contract and both adapters patch it, but nothing could set it: the HTTP DTO did not expose the field and the worker handler required one of price/title/ description to be present, so a rate-only update was rejected before it ran. Both are fixed, and a rate-only update is legitimate - propagating the shop's rate onto a live offer touches nothing else. Refs #2249 Refs #2245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(orders): carry the per-line tax rate into the order snapshot (#2254) The snapshot item projection in OrderRecordService.persistOrder is an allowlist, and it is the WRITER half of the pair readItems (orderFromReadySnapshot) reads back. It named neither taxRate nor its provenance, so the settled rate was lost at persistence time and every MANUAL issuance path - POST /invoices, bulk issue, corrections - rehydrated a rate-less order and was refused by the #2248 missing-rate gate, pointing the operator at a product that was already configured correctly. The auto-issue path composes from the live Order and never reads the snapshot, which is why the two paths disagreed. This is the mirror of the reader-side fix already made in readItems: both allowlists have to name the same fields. Found end to end against a live PrestaShop -> Allegro sandbox purchase, where order_line_items carried taxRate=23 / taxRate=5 with taxSource='shop' while the snapshot for the same order carried none. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(tax): address PR #2260 review - all 14 findings Review of PR #2260 raised 5 blocking, 6 important and 3 suggestion findings. Every one is addressed. BLOCKING 1 - the branch was red. It was redder than the review found: besides the four named core suites, the Erli and Allegro offer-manager specs, the KSeF fa3-xml builder, the KSeF fa3 mapper, and the inFakt and Subiekt strict specs were all failing, and `pnpm lint` was failing too on migration ordering. All green now: 4339 core+api tests, 387 worker, 3520 web, 2445 across the five provider and marketplace packages. BLOCKING 2/3/4 - the rollout was an immediate 100% outage on three surfaces and `taxRateEra` was inert. Strict enforcement is now one switch, OL_TAX_RATE_STRICT_ENABLED, OFF by default, resolved through a single helper in the sales-documents leaf so no site can test half the question. With it off every enforcement point behaves as it did before the epic: the three invoicing providers substitute their documented default, both issuance gates and the fiscal-registration gate pass, and the Allegro and Erli offer creates publish with the rate omitted. Two refusals deliberately survive the switch - an exemption code the channel cannot express, and a rate the target category rejects - because both mean the shop DID name a rate the channel cannot carry. taxRateEra is now read: a pre-rollout order is exempt from both the auto-issue gate and the write-path guard even with the switch on, so the back catalogue issues as it did before, which is what ADR-052 promised. BLOCKING 5 - an inherited variant read now clears a stored override, so a stale 5% cannot outlive the operator setting the variation back to parent and silently state 5% VAT on a 23% sale. IMPORTANT 6/7 - a WooCommerce transport failure no longer persists as "the shop has no rate". The PrestaShop resolver no longer states a confirmed 0% from the absence of a matching rule and no longer picks an arbitrary rule when several match; it reports unknown with the same vocabulary the WooCommerce sibling uses. IMPORTANT 8 - the gate and the write-path guard now decide on the same line set, including the case where the shipping split is uncomputable, so an order that would throw at compose time is blocked visibly rather than silently. MissingTaxRateException is a terminal business_failure in the worker handler, following the OrderAlreadyInvoicedException precedent. IMPORTANT 9/10 - the invoice panel no longer re-derives the shipping split; it calls a single mirror of the core function, held identical by a new check-shipping-tax-split-mirror invariant, so the previewed parts sum to the shipping the buyer paid and an exemption code renders as zw rather than zw%. The permanently-false ambiguous tax-class branch is gone. IMPORTANT 11 - the journal is reachable. written-by-us is written on the offer-create path and on the field-update handler, channel is written from the inbound order line (the channel's own value, never OpenLinker's resolved one), and getLatestPerConnection has an HTTP consumer. SUGGESTION 12/13/14 - the operator-writable taxRate on PUT offers/:id/fields is dropped per ADR-052. splitShippingAcrossRates takes the currency's minor-unit exponent instead of assuming two decimals, and the fiscalization mapper's own copy of that table is gone: one table, two consumers. Named coverage type replaces four duplicated inline shapes; misplaced docblock and mid-import constant fixed; the runbook's coverage query now accounts for variant overrides and no longer calls not_checked harmless when the gate blocks it identically. Also here, found while fixing the above: five migrations this epic owns sorted before origin/main's newest, so they are renumbered to 1840000000000-04. The base branch's own 1837000000000 had the same problem and is moved to 1839000000001 to keep lint green on this branch; #1985 should be aware. Docs reconciled with the shipped behaviour: the runbook, ADR-052's consequences, and the Listings, Invoicing and Sales Documents sections of the architecture overview. Refs #2245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * feat(web/listings): product-tier category, honest blocker copy, seller-details preflight (#2241) * feat(web/listings): product-tier category, honest blocker copy, seller-details preflight A three-variant product listed one variant and blocked the other two with a single chip reading `manual category`. Five defects behind that, all frontend. Readiness now reads the product tier. `recomputeVariantBlockers` derived the category from the variant tier only, while the submit pins `row.override.overrides.categoryId ?? row.resolvedCategoryId` as the family category - so setting the shared category, the action the banner recommended, could never clear a sibling's chip. `productCategoryIdOf` is the one chain both now use, and `noCardCategoryIds` uses it too: a category pinned at the product tier previously never fetched its required-parameter schema, so `needs-product-parameters` could not fire and the row read ready into a 422. The blocker vocabulary names causes. `no-ean` split into `no barcode` and `invalid barcode` (add one vs correct the one you typed are different fixes); `no-match` reads `no catalog match`, `multi-match` reads `multiple matches`. `bulk-blocker-copy.ts` holds one sentence pair per id, interpolating the offending barcode and the destination's display name, so the Review chip's tooltip and the editor banner cannot drift. The copy names the configured category mapping as the second route to a category, because for a large batch that is the fix that scales. The editor shows cause + `category not set`; the Review table shows the cause alone, since a row already carries up to four chips and a constant fifth pushes real signal off the end. Category is a field, not a disclosure. It moves out of the collapsed `Override base title / description / category` accordion onto the variant panel, and every category blocker's primary action is now `Set category for all N variants`, which switches scope AND opens the picker. `Fix on base` - which only switched scope - is gone from that path. Seller details are checked before submit. Allegro's `collectMissingSellerDefaultsFields` is the first statement of `createOffer`, unconditional, and covers the ship-from location as well as producer and safety information, so an incomplete connection fails every child job after a green Review. `OfferValidationContribution.validateBatch` is the plugin seam for a connection-level precondition; Review renders it as one banner for the batch, never a chip per row. Same failure shape as `allegro:title-too-long` (#1962), one level up. The outcome chain fails closed. `computeBlockers` had no final `else`, so an outcome added backend-first - the planned `lookup-failed` is exactly that - would have produced a ready row with no category. It now emits `unknown-category-result`. The confirmation can describe a dropped variant. `blockedCount` and `alreadyListedCount` are named separately (already-listed variants carry no blocker by design and the backend skips them at intake), and `1 offers` is fixed. Deliberately out of scope, stated in the issue: the backend `lookup-failed` outcome, a cache-bypassing re-check (a successful empty result is cached 24 h, so an FE-only retry would be a no-op), live chip clearing while typing, and making the seller-detail fields required at connection-save time. Refs #2240 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * test(e2e): pin the wizard's category-blocker states, and name the destination by connection Seven states, driven through the real wizard against a stubbed OL API, so the project needs no seeded catalogue, no Allegro connection and no shared auth artifact - it stubs the session bootstrap too, which is what lets it run on any served build. Each test writes a named screenshot into its own output directory and attaches it, because this change is mostly about what the operator is told and an assertion on a string is necessary but not sufficient evidence for that. The load-bearing one is `setting the product category clears the blocker on every inheriting sibling`: before #2240 that Review stayed at 1 ready / 2 needing attention no matter how many times the shared category was set, because readiness read the variant tier while the submit pinned the product tier. Two departures from what the states were first written as, both because the app turned out to be right and the test wrong: - the invalid-barcode case asserts that the field refuses the value in place (`Invalid GTIN checksum`), rather than round-tripping through Save to look for a chip. The save is legitimately blocked by the pre-existing GS1 guard, so the chip is unreachable that way; the `invalid-barcode` blocker id itself is asserted in `bulk-policy.test.ts`, which can check the id rather than a rendering of it. - the destination in the editor's blocker copy is now the CONNECTION's name, not the platform label, matching what the Review table's chips already said. An operator can hold two Allegro connections, and "Allegro" does not tell them which catalogue was consulted. The e2e run is what surfaced the inconsistency. Refs #2240 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * test(e2e): cover every category-blocker state, and fix the two the run found 21 tests, 24 screenshots, up from 7 and 7. One case per cause (no catalog match, no barcode, invalid barcode, multiple matches, unknown result), one per editor surface, one per fix step, one per destination shape, one per batch precondition, and the four pre-submit states. The stub is now configurable per test - variant outcomes, destination declaration, seller defaults, already-listed variants, required product parameters - so each state is reached by the wizard's own code rather than by a locator that happens to match. Two production defects the expansion surfaced, both fixed here. `unknown result` degraded to `no catalog match` on the second recompute. `variantCategoryResult` reconstructs the outcome from the row's own blockers, and an unrecognised discriminant fell through to `no-match` - so a row that had just said "the answer was not understood" started claiming the catalogue has no match for that barcode, which is precisely what the lookup did not say. The reading now survives a reblock, pinned by a unit test as well as the E2E case. "Only this variant" was a dead end. The editor's own save requires a category on the product (`makeBulkEditModalSchema`'s `requireCategory` for a browsable destination), so a per-variant-only override could never be saved: Save bounced to the shared scope with a field error the operator was never sent to. The per-variant override is therefore a REFINEMENT of the shared value, not an alternative to it - the banner offers the product tier alone, the copy no longer promises "or just for this variant", and the override is taken from the category field once a product category exists. The banner's category action is also gated on the row actually lacking a category, so a blocker like `invalid barcode` - category-adjacent but surviving a set category - no longer points at a control that is already filled in. Refs #2240 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(web/listings): address review on the bulk-wizard category blockers Five review findings from #2241. Two change behaviour. - The seller-defaults mirror was unguarded AND already stale. It only checked that `safetyInformation` was an object, while the adapter's gate requires a `type` plus a value on whichever arm that type selects - so a connection carrying a description and no type read green and had every offer rejected, the exact under-reporting the review predicted. The frontend now mirrors the gate arm for arm, declares the path set it mirrors, and `scripts/check-allegro-seller-defaults-mirror.mjs` (with --self-check, under check:invariants) fails the build when the two sides name different paths. One divergence is kept and documented: a whitespace-only string is reported missing here and passes the adapter, because blank-after-trim is not a value and Allegro rejects it either way. - A batch-level precondition now LOCKS the submit instead of warning. It is connection-wide and deterministic, so no subset of the batch can succeed and a banner an operator can read past explains the wasted batch rather than preventing it. The contract is stated on OfferBatchIssue, including what it asks of a plugin: report only what the destination declares, and never let a mirror of a destination gate be stricter than the gate. And three that do not. - UNRECOGNISED_CATEGORY_RESULT was an `as unknown as EanMatchResult` double cast. It is now its own member of a wider CategoryOutcome union: "this build did not understand the answer" is a fact about the wizard, not a shape the destination sends, and the compiler can say so - which also makes computeBlockers' final arm reachable through the type system rather than around it. - The invalid-barcode collapse was written identically in bulk-policy and the Resolve step. It moves to `collapseToInvalidBarcode` beside isCategoryBlocker, for the reason productCategoryIdOf was extracted one screen earlier in the same diff. - docs/architecture-overview.md § Listings documents the new validateBatch / OfferBatchIssue slot, the reworked blocker vocabulary and the single category chain, at the depth the section uses for comparable changes. Refs #2240 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(web/listings): type collapseToInvalidBarcode's return, drop the trailing cast collapseToInvalidBarcode now declares its return as BulkRowBlocker[] instead of string[], so both call sites (recomputeVariantBlockers and the Resolve step) no longer need `as BulkRowBlocker[]` after it - the last nit from Piotr's review. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> --------- Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(prestashop): read the order's real currency instead of hardcoding EUR (#2439) * fix(prestashop): read the order's real currency instead of hardcoding EUR Every order ingested from PrestaShop was recorded with `currency: 'EUR'`, whatever the buyer actually paid in. The value was a literal in `prestashop-order.mapper.ts` whose `// Default, can be configured` comment was never true - nothing overrode it, and `Connection.config.currency` is the product-sync default that the order path never consults. The amounts were correct; only the denomination was wrong. That matters because the ADR-040 reporting stamp multiplies the order total by the NATIVE currency's rate, and the invoicing and fiscalization mappers pass the same value through to KSeF / inFakt / Subiekt verbatim. Resolution chain, first answer wins: the order's own `id_currency`, then the shop default (`PS_CURRENCY_DEFAULT`), then refuse with the existing `PrestashopCurrencyUnknownException` (#2139), which the retry classifier already treats as terminal. `config.currency` is deliberately not in the chain - it means "product-sync default" today and the shop-default read is a strictly better fallback because it cannot drift from the shop. The resolution lives in the adapter, not the mapper: `mapOrder` is synchronous and does no I/O by contract, so `MappedPrestashopOrder` omits `totals.currency` entirely - writing a literal there is now a compile error rather than a plausible default. Also included, since re-polling cannot repair them: - a migration nulling the six FX columns on PrestaShop-sourced rows whose snapshot currency has already been corrected away from EUR, so the FX sweep re-stamps them. Scoped by that predicate rather than by operator discipline: run before the re-poll it is a verified no-op. - `readPrestashopCurrencyById`, the by-id sibling of the existing by-ISO read, with `prestashop-shop-currency.resolver.ts` refactored onto it. Closes #2277 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(api): scope the FX-stamp reset to stamps actually derived from EUR Two corrections to the migration, both found by running it against a live PrestaShop stack rather than by reading it. 1. The snapshot test was a PROXY for the damage, not the damage. It cannot tell a stamp computed while the snapshot still said EUR from one an already-fixed deployment computed correctly minutes ago, so it re-opened correct figures too. Observed live: an order ingested after the fix, carrying a PLN->EUR rate and the right converted total (49 PLN -> 11.33 EUR), matched the predicate. The scope now additionally requires the stamp to be one of the two shapes a native EUR can produce - a conversion whose rate row reads `fromCurrency = 'EUR'`, or the same-currency short-circuit that writes `reportingCurrency = 'EUR'` with no rate row at all. A rate-only test would have missed that second shape entirely, and it is the worse one: the total was copied across unconverted. Live dry-run on the same data goes from 1 row to 0. 2. The docblock claimed `order_records.currency` and `order_line_items` self-heal on re-poll. Neither exists on this branch - #1985's analytics read model is not merged here, as `OrderRecordRepository.listDistinctNativeCurrencies` documents in place. The native currency lives only in `orderSnapshot.totals.currency`, which is what self-heals. Also adds the spec that pins both predicate arms. The WHERE body is parsed out of the real emitted statement and evaluated against row fixtures - the approach the sibling 1836000000000 spec established, since nothing in CI executes a migration. The top-level split is parenthesis-aware because the damage clause is itself a parenthesised disjunction, and an unrecognised clause throws rather than being skipped, so the spec cannot keep passing over a predicate it no longer describes. Verified by mutation: deleting either arm fails exactly one case. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * test(e2e): assert a PrestaShop order keeps the currency the buyer paid in Two orders, not one. A single PLN order would pass just as well against a fix that swapped one hardcoded literal for another, or that always substituted the shop's default currency. Two orders synthesized in two DIFFERENT currencies, in the same run against the same shop, can only both hold if the value is read per-order - which is the property #2277 actually claims. `synthesizeOrder` gains a `currencyId` option threaded onto BOTH the cart and the order, since PrestaShop stores `id_currency` on each and an order whose cart disagrees is a shape no storefront checkout produces. The id is resolved at run time by the new `getCurrencyIdByIso` (mirroring the existing `getCountryIdByIso`) rather than hardcoded - currency ids are per-install, so a literal would denominate the order in whatever that shop happens to carry at that position. A shop missing either currency skips with that reason instead of asserting something weaker. Its own `order-ingestion` project with `retries: 0`: the spec synthesizes real customers, addresses, carts and orders, and the `orders` project's own comment states its strictly-read-only contract. A retry there would create a second order and assert against the wrong one. Verified live: passes on the fixed build; on a build reverted to the pre-fix behaviour the same order that reads PLN is recorded EUR while the shop says PLN. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> --------- Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(tax): address PR #2260 review round 2 - N1-N7 N1 (blocking) - taxRateEra was honoured on the invoice route only. The fiscal-registration write gate and the Subiekt line mapper read only isTaxRateStrictEnabled(), so a pre-rollout order could pass the era-aware auto-issue receipt gate (clearing any persisted reason), enqueue, and then be refused at the write gate with nothing recorded - the exact silent decline finding 8 removed, recreated on the era axis. taxRateEra is now threaded end to end on the fiscal path (command, mapper, job payload, worker handler, manual HTTP route) and resolved through isTaxRateEnforced everywhere an order is in hand; a channel publish, which has no order, correctly reads the switch alone. MissingFiscalTaxRateException is now terminal in the worker handler, matching invoicing-issue.handler's precedent. Found and fixed while testing: the Subiekt bridge error translator was swallowing MissingTaxRateException into an indeterminate transport error. N7 - the switch had no unit test anywhere, which is plausibly why N1 went unnoticed. Added. N2 - the two refusals a channel publish keeps even with the switch off (an exemption code the destination cannot express, a rate its category rejects) are a deliberate decision, not an oversight: a live offer stating the wrong tax charges a real buyer wrongly, which is worse than a failed batch an operator can see. Recorded in ADR-063 and the runbook, which previously contradicted itself on this within twenty lines. N3 - the "Fix and re-check" link pointed at a query param the products page does not read. Fixed, with the href now asserted. N4 - the shipping-split mirror guard compared the functions but not the four currency-classification constants they read from, so a dropped currency or a changed default exponent passed silently. Extended and verified against both drifts. N5 - the panel read shipping/currency straight off…
What changed
Every order ingested from PrestaShop was recorded with
currency: 'EUR', whatever the buyer actually paid in. The value was a literal atprestashop-order.mapper.ts:81, and the// Default, can be configuredcomment beside it was never true — nothing overrode it, andConnection.config.currencyis the product-sync default that the order path never consults.The amounts were always correct. Only the denomination was wrong, which is why it went unnoticed: it looks cosmetic until you follow it downstream.
The currency now comes from the order itself.
Resolution chain
First answer wins:
id_currency, resolved to ISO 4217 via a new cached resolver.PS_CURRENCY_DEFAULT), through the existingPrestashopShopCurrencyResolver.PrestashopCurrencyUnknownException([BUG] Integration/PrestaShop — resolveCurrencyId silently books an order in the wrong currency instead of refusing #2139), already classified non-retryable.Connection.config.currencyis deliberately not in the chain. It means "product-sync default" today, and giving it a second meaning makes it unreadable; the shop-default read is a strictly better fallback because it cannot drift from the shop. An order that simply omitsid_currencyis a shop-default case, not a refusal — an absent field says nothing is wrong with the shop's configuration, whereas an id that resolves to nothing does.Why the resolution lives in the adapter, not the mapper
IPrestashopOrderMapper.mapOrderis synchronous and does no I/O by contract, so it cannot perform theGET /currencies/{id}read — which is precisely how it came to emit a hardcoded literal.MappedPrestashopOrdernow omitstotals.currencyentirely, so writing a literal there is a compile error rather than a plausible default.PrestashopOrderSourceAdapter.getOrderfills it before the order leaves the adapter, which covers both the webhook path and the reconciliation poll in one place.Step 5 finding: the refusal does reach the runner
The issue asked to verify this, because the classifier docstring warns that on the order-create path the exception is swallowed by
Promise.allSettledinOrderSyncService. Traced on the ingest path:OrderIngestionService.syncOrderFromSourcecallsgetOrderoutside any try/catch (order-ingestion.service.ts:216).MarketplaceOrderSyncHandlerwraps it inSyncJobExecutionError, preservingcause.SyncJobRunner.isNonRetryableErrorunwrapsSyncJobExecutionError.cause(sync-job.runner.ts:425) before consulting the classifier.PrestashopRetryClassifierAdapter.isNonRetryablematches, and the job ismarkDeadon its first attempt with no retry budget spent.One correction to the issue's wording. The acceptance criterion says "terminal
business_failure". The real outcome is a terminaldead, notbusiness_failure—outcomeis only ever set on the succeeded path (ADR-007). The property that matters (terminal, no retry storm, operator-visible message) holds; the label in the issue does not.isCredentialRejecteddoes not match, so the connection is correctly never flaggedneeds_reauth.Backfill migration
1840000000000-reset-fx-stamp-for-mislabelled-prestashop-orders.ts.Re-polling repairs the snapshot on its own, because the upsert replaces
orderSnapshotwholesale — and on this branch that is the only place the native currency lives. There is noorder_records.currencycolumn and noorder_line_itemstable here: #1985's analytics read model is not merged intomain, asOrderRecordRepository.listDistinctNativeCurrenciesdocuments in place. The ADR-040 reporting stamp does not: it is a write-once conditional UPDATE guarded byreportingCurrency IS NULL, that predicate sits in both arms of the sweep, and nothing anywhere in the codebase clears a stamp. A 249 PLN order stamped at the EUR rate stays wrong forever without this.The migration nulls the six FX columns so the sweep re-admits the row.
Order of deployment is safe by predicate, not by operator discipline. The scope is PrestaShop-sourced rows whose snapshot currency is already no longer
EUR— i.e. rows the corrected code has re-polled. Run before that re-poll it matches nothing and is a verified no-op; it can never null a stamp that would only be recomputed from the same wrong snapshot and re-closed. A genuinely-EUR PrestaShop order is left alone, since its stamp was right all along.down()is intentionally a no-op: the pre-migration values are not recoverable from anything still on the row, and restoring them would mean re-asserting a total the operator was told to stop trusting.Note for whoever runs it: the FX sweep defaults to a 30-day
createdSincewindow, so older orders need either amaxAgeDaysbump on the scheduler descriptor or directmarketplace.order.fxStampenqueues.Operator report — what code cannot repair
Invoices already issued, fiscal registrations, and orders already pushed to a destination shop carry the wrong currency and are not reversible in software. For KSeF that means the document already cleared with the tax authority (
EURis a valid FA(3)KodWaluty, so nothing rejected it).Affected orders:
Run this before re-polling — afterwards the snapshot reads the corrected currency and the rows stop matching.
A correction inherits the original document's currency from
OriginalDocumentSnapshot(invoicing.controller.ts:1350), so whoever issues one has to change it by hand.Scope notes
PLN(erli-order.mapper.ts:203) but its API carries no currency field at all — not the same defect, left alone.Verified live
Run against a PrestaShop + OpenLinker stack, on one order (PrestaShop order 6, 49.00, shop default PLN):
And the property the fix actually claims, on the same order: switching it to EUR in the shop and re-polling makes OL read
€ 49.00; switching it back makes OL readPLN 49.00again. The currency follows the order, not a default.The migration was dry-run against that stack's real data. With the snapshot test alone it matched 1 row — a correctly-stamped order — which is what produced the second commit on this branch; with the damage predicate it matches 0.
Testing
pnpm lint— 0 errors (the migration-timestamp invariant caught a collision withorigin/main's1839000000000and the file was renumbered).pnpm type-check— clean.@openlinker/integrations-prestashop— 567 tests pass, 37 suites. New coverage for currency-from-order, shop-default fallback, refusal, and cache behaviour;prestashop-order.mapper.spec.ts:119'stoBe('EUR')assertion is gone, and the eight adapter-spec construction sites carry the new dependency.WHEREand evaluates it against row fixtures. Mutation-verified: deleting either predicate arm fails exactly one case.apps/e2e/tests/order-ingestion/order-currency.spec.ts— synthesizes two PrestaShop orders in two currencies and asserts each is recorded in its own. Own project withretries: 0, because it mutates. Passes against a live stack.One honest note on that e2e: under the reverted build it failed on an ingestion timeout rather than on the currency assertion (the stack's single execution slot was backlogged), so the regression itself was confirmed directly against the database instead — the table above.
Closes #2277
Related to #2275