1962fdea - Let legacy Yapeal CHF holders switch between their old and new personal IBAN - #1320
Merged
Conversation
…al IBAN Grandfathered customers with an active Yapeal CHF personal IBAN now see their new Bank Frick IBAN by default (KYC 50+) and can flip back and forth with a provider toggle next to the payment details. Every flip requests a fresh quote pinned to the explicit personalIbanProvider, so the stored transaction request - and with it every later invoice or receipt PDF - always names exactly the IBAN that was on screen. The screens read GET buy/personalIban (which now names the issuing bank per row) to decide whether a switch target exists; without the new API field the feature stays inert and the flow behaves as before. An automatic Frick default that the server rejects with KycRequired falls back to a selector-less quote with a permanently visible hint instead of a blocking KYC screen - the customer never chose Frick themselves. The four screen-test suites mock the virtual-iban hook with a reference- stable, reset-proof interface (CRA sets resetMocks true), and the new e2e scenario pins both toggle states with static route mocks and baselines.
🤖 PR Review Bot❌ TypeScript: 14 errorsThis is an automated review. Please address the issues above. |
…nt hygiene, gated first quote, full test coverage of the new paths The request provider is now derived from the same debounced snapshot as the request currency (the live derivation keeps feeding only the invalidation signature), the row list and every provider choice reset on an in-place account swap and commit only for the identity that started the load, the automatic-Frick KYC fallback classifies on the actually sent selector and clears on an explicit toggle click, and the first quote of an eligible legacy holder waits for the row list exactly when it could change the default - a failed load resolves the gate immediately. New tests pin the toggle button in both directions, the real getPersonalIbans hook, the five screen scenarios (default Frick, toggle to Yapeal, visible KYC fallback, hint cleared on explicit choice, account-swap reset) and the non-Yapeal-bank branch of the alternative lookup; the /buy/info flow carries the same provider-toggle e2e scenario with two committed baselines.
GET buy/personalIban now flows through useBuy().getPersonalIbans (SDK rule; the local hook keeps only the pre-existing POST), and the row list carries an identity tag so neither the list nor a quote response can ever commit for an account that is no longer signed in - the quote signature includes the customer identity and every commit re-checks it. The request provider derives from the same debounced snapshot as the request currency, the displayed verification follows the provider that was actually sent (an explicit Yapeal answer after a URL-Frick entry is valid, not blocked), the first quote of an eligible legacy holder additionally waits while the user context itself is still loading, and the row-list load fails open to an empty list after ten seconds so the core buy flow can never hang on the optional switch offer. The KYC fallback now renders as its own visual variant with committed baselines on both screens, the /buy/info suite carries the full scenario set (default Frick, toggle, fallback, hint clear, account swap, load rejection), the same-snapshot and identity races are pinned by dedicated tests, and the suites' console traps now name the offending component.
…ntext wait The mismatch acknowledgement and selector suppression now carry the customer identity like the provider override already did - an account swapped in place can no longer inherit the previous account's dismissal or suppression. The user-context part of the first-quote gate gets the same ten-second fail-open cap as the row list, so a hanging user reload can never block the core buy flow; after the cap the quote proceeds selector-less and a late load triggers the ordinary corrective quote. The provider-unavailable rejection renders as its own visual variant with committed baselines on both screens, the handbook descriptions name the KYC fallback and the new error case, and the suites pin the row-list timeout, the stale-rejection guard, the verification-failed branch and the toggle/fallback rendering with fake timers and deferred promises - resolved inside act, which also settled a test-order-dependent act warning for good.
…ng races - Gate explicit personal-IBAN requests provider-neutrally per wallet - Tag payment info state with the customer identity and reset completion and confirmation state on identity changes; guard the confirm callback against stale identities - Wait for user data before deriving the live Frick default and clear payment info synchronously before requesting new payment details - Use fully static user and error fixtures in the provider-unavailable e2e scenarios and add identity-race and completion-leak regression tests
The real payment-info response always carries asset.uniqueName, and the buy screen's exact-price echo suppression compares exactly this field. Without it every static quote triggered a redundant reverse quote cycle ~500ms later that replaced the payment info object and reset the collection-IBAN display toggle mid-interaction.
…entity gaps - Extract the row-loading/timeout state machine into usePersonalIbanRows and the provider decision tree into a pure derive helper consumed by both buy screens - Evaluate KYC only against a user whose accountId matches the session identity; a foreign user counts as still loading and joins the existing ten-second fail-open - Tag the automatic-Frick KYC fallback flag with the customer identity - Offer a 'Show available IBAN' recovery action on the provider- unavailable error instead of a retry-only dead end, and reflect it in the e2e scenarios and baselines - Commit completion only on a successful confirmation; surface errors and guard against unmount and identity changes - Bundle the provider-switch props, replace enum casts with a typed parser, type the deferred test helpers and drop the now-unused Frick-only request predicate
…e KYC hint - Hold the quote generation across the confirm round trip and drop any resolution for a quote the form has since replaced; invalidation also clears the confirming state - Split the automatic-Frick suppression flag from the KYC hint so recovering from an unavailable provider no longer shows a misleading KYC message; only a genuine KycRequired fallback renders the hint - Localize the recovery action, pass the switch target through the toggle callback, drop the duplicate provider derivation on the info screen and refresh stale comments - Cover the identity round trip in a dedicated row-hook suite, click the recovery action in both unavailable e2e scenarios asserting the selector-free follow-up request, and pin the stale-generation confirmation race
Each quote request persists its own transaction request, so the approximate quote and the exact-price refinement of the same generation carry different ids. Confirmation is now gated on the final quote commit and the confirm callback additionally re-checks the displayed request id before committing completion, so a confirmation can never target a request the screen has since replaced. Also aligns test-mock indentation and moves the JSX-free row-hook suite to a .ts extension.
…pletion screen The completion screen read the live quote state, so a re-quote triggered by late-loading context could swap the payment information a customer had already acted on - both on screen and in the close payload handed to the host application. Both buy flows now snapshot the completed information under the customer identity and render only from that snapshot, cleared alongside the completion flag on an identity change. Also pins the close payload to the passed payment information and makes the quote fixtures disjoint so freezing the approximate instead of the final quote can no longer pass unnoticed.
Contributor
Author
|
10 review passes (conformance and logic, each pass covering the full diff) until both dimensions came back clean. Summary of what the later passes actually changed, since most of it is behaviour rather than polish: Cross-account and timing correctness
Dead ends removed
Structure
Test findings worth naming
Deliberately declared, not fixed here (see the PR description): the raw One reported point was declined with reasons: an additional test combining the real completion component, a close click and an in-flight re-quote. Both ends of that chain are already pinned — the screen passing the frozen information (asserted during and after the background re-quote) and the component handing exactly its prop to the close channel (reference identity) — with nothing but prop forwarding in between. This PR stays in draft until the SDK release it depends on has landed; the dependency bump follows, and CI is red by design until then. Deutsch10 Review-Durchläufe (Konformität und Logik, jeder über den vollständigen Diff), bis beide Dimensionen sauber waren. Was die späteren Durchläufe geändert haben, ist überwiegend Verhalten, nicht Kosmetik: Konto- und Timing-Korrektheit
Sackgassen beseitigt
Struktur
Nennenswerte Testbefunde
Bewusst deklariert, hier nicht behoben (siehe PR-Beschreibung): der rohe Ein gemeldeter Punkt wurde begründet abgelehnt: ein zusätzlicher Test, der die echte Abschlusskomponente, einen Klick auf Schliessen und eine laufende Neuanfrage kombiniert. Beide Enden dieser Kette sind bereits abgesichert — der Bildschirm übergibt das eingefrorene Abbild (geprüft während und nach der Hintergrund-Neuanfrage), und die Komponente reicht exakt ihr Prop an den Schliessen-Kanal weiter (Referenzidentität) — dazwischen liegt nichts als Prop-Durchreichung. Der PR bleibt im Entwurf, bis das benötigte SDK-Release vorliegt; danach folgt die Abhängigkeitsanhebung. Bis dahin ist die CI absichtlich rot. |
The personal-IBAN provider types and the row-list hook this branch needs are published as @dfx.swiss/react 1.8.0-beta.2 (pulling in @dfx.swiss/core 0.7.0-beta.1), so the local tarball override is no longer required.
TaprootFreak
marked this pull request as ready for review
August 12, 2026 12:46
Contributor
Author
|
Ready for review. The SDK dependency this branch was waiting on is published, so the last blocker is closed: All checks are green on the final head, including the handbook image build that is skipped while a PR is a draft, and the PR reports mergeable. Local chain on Node 20 (the version this repository's CI uses): lint, 84 suites / 1,052 tests, One remaining item in the checklist above is not closable from here: DFXswiss/api#4813 has to be deployed for the feature to exist at all. Until then this frontend behaves exactly as it does today — without the DeutschBereit zum Review. Die SDK-Abhängigkeit, auf die dieser Branch gewartet hat, ist veröffentlicht — damit ist der letzte Blocker erledigt: Alle Checks auf dem finalen Stand sind grün, inklusive des Handbook-Image-Builds, der bei Entwürfen übersprungen wird; der PR ist mergefähig. Lokale Kette auf Node 20 (der Version, die die CI dieses Repositories nutzt): lint, 84 Suiten / 1'052 Tests, Ein Punkt der Liste oben lässt sich von hier aus nicht schliessen: DFXswiss/api#4813 muss deployt sein, damit das Feature überhaupt existiert. Bis dahin verhält sich dieses Frontend exakt wie heute — ohne das |
TaprootFreak
added a commit
to joshuakrueger-dfx/services
that referenced
this pull request
Aug 12, 2026
DFXswiss#1320 rewrote the same five files while this branch waited, so the rebase left them holding one side or the other. They now carry both: develop's provider generalization untouched — the Yapeal parsing, the precedence helper, the provider switch and every test around them — and this branch's work lifted back on top of it. Where the two genuinely met, the collection toggle keeps its own state in the content component so the QR branch can see it, and the provider switch stays wired exactly as develop passes it down; both buttons sit in the same IBAN row. The dependency bump is gone: develop already carries a newer SDK than this branch pinned.
6 tasks
TaprootFreak
added a commit
that referenced
this pull request
Aug 13, 2026
#1270) * Make the collection-IBAN toggle readable and let the QR code follow it The toggle added in #1258 works, but it is hard to read: it uses a colour emoji next to the monochrome DFX icons, it never says which of the two accounts is currently shown, and it never states the reason it exists, so the customers it was built for do not find it. - Replace the emoji with IconVariant.SWAP, without size and colour props so it matches the neighbouring CopyButton exactly. Drop the extra ml-1, which broke the row's gap-3 rhythm, and add a focus ring and aria-pressed. - Name the state and the reason in the row's existing infoText slot, the same pattern the remittance-info row already uses. On the collection account the hint also states that the remittance info is mandatory: a transfer there is attributable through the reference only. - Carry the switch into the QR tab. It encoded the personal IBAN even after switching, so it handed the customer the very IBAN their bank rejects. toCollectionIbanGiroCode rewrites line 7 of the EPC payload and nothing else; holder, amount and remittance stay identical. Both the text and the QR branch are gated on canOfferCollectionIban, so display, copy and QR can never diverge. It fails closed - if the payload is not a well-formed SCT GiroCode carrying the displayed IBAN, no QR code is rendered at all and the screen asks for manual entry. Behaviour and gating are unchanged; canOfferCollectionIban is untouched. The accessible names stay verbatim so the tests from #1258 remain valid. * Let the PDF invoice follow the switch as well The invoice button sits inside the QR tab, and the document it opens is rebuilt server-side from the stored request - so it kept naming the personal vIBAN while the screen, the copy button and the QR code already showed the collection account. The customer ended up holding two documents naming different receiving accounts. DFXswiss/backend#4686 adds an optional collectionAccount switch to PUT /buy/paymentInfos/:id/invoice. This sends it while the collection account is displayed, using the SDK's own BuyUrl.invoice builder through useApi so the request stays identical apart from the query. The parameter is omitted, never sent as false, because the backend maps any present value to true. The three rejection tokens the endpoint can answer with all mean the same thing to a customer - the invoice cannot be issued for the collection account right now, the details on screen still apply - so they share one message while staying separate tokens for the logs. * Drop the error token the endpoint no longer emits Review of the api side found that the reference guard behind CollectionAccountInvoiceReferenceMissing could not be reached: buy.bankUsage is NOT NULL in the schema and set on every route creation, so the guard and its token were removed there. Mapping a token no consumer can receive would be dead code on this side too. * Match the renamed collection-account error token The api renames CollectionAccountInvoiceRequiresPersonalIban to CollectionAccountInvoicePersonalIbanMissing, so the token says what the guard found instead of what it wanted. The customer-facing message is unchanged; only the string it matches on moves. * Call the buy invoice through the SDK instead of a hand-built URL * Never open the invoice PDF for a mode or session the UI has left * Refuse to rewrite a GiroCode that does not carry the quote's remittance info * Cover the three files this pull request touches CONTRIBUTING requires every touched file to reach 100 percent on all four metrics and the per-file numbers to be stated in the description. Measured on the previous head they were 70.96/94.59/40/68.96 for payment-info-buy.tsx, 90.32/88.23/100/92.85 for payment-qr-code.tsx and 98.82/98.76/100/100 for personal-iban.ts. The gaps were the render branches and copy callbacks of the payment rows, the early return that sends a customer with incomplete KYC to their profile instead of issuing an invoice, and the generation guard in the catch block - the error half of the stale-response protection, which until now had only its success half pinned. That last one is the reason this is more than bookkeeping: a late rejection arriving after the customer switched modes must not surface an error for a mode they have left, and nothing held that property in place. Tests only. * Present the IBAN toggle as a plain action button and cover its last guard branch * Capture the fail-closed QR state as a committed handbook baseline The buy spec asserted twice that the manual-entry hint is not visible, but nothing ever captured it when it is: a GiroCode whose remittance line does not carry the quote's reference must render no QR at all. The new case mirrors the neighbouring QR test with one changed payload line and pins the state to a baseline like the other four. * Refuse to rewrite a GiroCode that populates both remittance carriers EPC069-12 allows exactly one remittance carrier — structured line 9 or unstructured line 10. The mismatch check accepted a payload carrying both as long as one of them matched the quote's reference; such a payload is non-compliant and is now not rewritten at all. The rebuild semantics are pinned alongside: CRLF input normalizes to LF and a trailing blank line is dropped, with the IBAN line remaining the only changed field. * Pin both halves of the collection-QR guard, not just the IBAN one The case added in the previous commit reaches the guard's false arm only through a missing IBAN, although its title names the remittance info as well. Branch coverage cannot see the difference - Istanbul counts the ternary once, and the IBAN case already covers it - so the file read 100 percent while dropping the second operand changed nothing: 21 of 21 tests stayed green. The case is now an it.each over both fields. Each half of the condition fails its own case when removed, and the parameter is in the title so a failure says which one. Worth knowing how these die: not through the assertions but through a TypeError, because without the guard the payload builder is handed undefined and calls trim or replace on it. That is what the guard is for - it keeps undefined away from a function that assumes strings - so the crash is the property being pinned rather than a weak proof. An empty string is a different matter and stays green under the same mutation: there the builder's own check rejects it and the screen looks identical. Tests only. * Keep the PDF invoice available when only the QR image is refused The fail-closed rewrite gates the QR image, but the invoice is built server-side and does not depend on it — replacing the whole component with a hint also took the button away. The hint now renders inside PaymentQrCode with the button below it, pinned by a regenerated baseline. Three hardenings alongside: the generation guard bumps in a layout effect so a response cannot land between commit and the passive-effect flush; the GiroCode parser validates the EPC character-set line (1-8) and no longer repairs leading whitespace by trimming the whole payload; and the collection-invoice error state gets its own e2e case and baseline. * Accept the remittance reference on the unstructured line only The api emits the reference exclusively on unstructured line 10, and a quote reference is never a valid ISO 11649 structured reference — this function never validated one, so a payload carrying the reference on line 9 is not the api's output and is now refused instead of rewritten. This subsumes the dual-carrier refusal. Two test gaps closed alongside: the offer-gate rerender case now pins the mock attributes, and a txId change during an in-flight request is pinned to discard the response. * Delete the dead guard branch by making the rewrite its own gate The caller guarded toCollectionIbanGiroCode with a ternary whose false arm is unreachable — canOfferCollectionIban already requires iban and remittanceInfo — and covering it meant forcing the gate mock into a state the app cannot reach. The function now accepts both fields as possibly undefined and refuses first thing when either is missing, the caller passes the quote fields straight through, and the refusal is pinned by two direct unit tests. The module mock in the toggle spec is gone with it: every remaining case tests through the real gate. * Re-arm the invoice guard when the signed-in account changes A deep-link param push replaces the session in place — wallet.context applies updateSession without unmounting the screen — so an invoice requested under the previous session could still open after the swap. The generation guard now also keys on user.accountId, which changes exactly when the signed-in identity changes and not on periodic context refreshes, so legitimate in-flight downloads are not discarded. * Key the invoice guard on the session, not the lagging user context An in-place token swap changes the session synchronously, but the SDK's user context reloads only when the logged-in boolean flips — which it does not across a swap — so user.accountId can keep serving the previous account and the guard added for this scenario never fired. The guard now keys on session.account from useApiSession, and the test models reality: the session changes while the mocked user stays stale. The GiroCode docstring also stops asserting backend behaviour it cannot prove; the structured-carrier refusal is now justified locally — no ISO 11649 validation exists here, so that carrier is outside the validated shape, and a wrong refusal costs only the QR image. * Pin the two combinatorial states the baselines missed The toggle baselines are shot against a quote without a paymentRequest and therefore show the tabless presentation — the Text tab with the tab bar visible in the collection state had no baseline. And the fail-closed QR state and a rejected invoice can occur together, but neither test ever produced them at once. Both states are pinned now, and the toggle test carries the same lang=en pin as its siblings so the locale of the test account cannot decide what the baselines show. * Track both session identity fields and align the gate with the rewrite The guard now keys on session.account and session.user, so a token swap keeping one but changing the other still re-arms, and a concurrency test pins that a stale response cannot clear a newer request's loading state. The e2e invoice-error case asserts the request actually carries collectionAccount=true instead of trusting a mock that answers anything. canOfferCollectionIban refuses whitespace-only fields, matching the trim semantics of the rewrite it gates, and the rewrite's length guard states its real minimum: eleven lines, since the unstructured reference on index 10 is required — which also let the now-dead length ternary go. The Text tab with the tab bar visible is pinned in both toggle states. * Validate the amount line before rewriting a GiroCode The rewrite validated every field it depends on except the amount: a payload whose line 7 disagrees with the quote would have been rewritten, and the collection QR would offer a different amount than the Text tab beside it. Line 7 must now carry EUR plus exactly the quote's amount — numeric equality, so EUR100 and EUR100.00 both match 100 — and a missing amount refuses like the other quote fields. * Mock the tab container the way it actually behaves The toggle spec's StyledTabContainer mock rendered every tab at once — a state the real component never produces, since it renders only the active tab. The mock is stateful now, tabs switch by click like in production, and the QR assertions activate their tab first. * Hold the amount line to strict EPC grammar before comparing Number() coerces more than EPC069-12 allows: EUR1e2, EUR0x64 or a bare EUR would have compared equal to a clean amount. The numeric part must now be plain digits with at most two decimals, and the comparison runs in cents, so EUR100 and EUR100.00 still both match 100 while exponent, hex and signed spellings refuse. * Cap the amount grammar at the EPC ceiling in canonical form Nine integer digits is where EPC069-12 stops (999999999.99), and a canonical amount carries no leading zeros — EUR000000000100 numerically equals 100 but is not the format the standard permits, so both now refuse rather than compare. * Lift the collection hardenings onto the per-currency world of #1305 The rebase onto develop kept this branch's versions of five files, which carried the pull request's hardenings but predated #1305's EUR+CHF generalization. This commit restores that architecture — the per-currency collection map, FRICK_CURRENCIES, getFrickCollectionIban and getOfferableCollectionIban — and lifts the hardenings onto it: the whitespace gate lives in getOfferableCollectionIban now, the display follows the per-currency IBAN, and the GiroCode rewrite keeps every gate while stating why it stays EUR-bound: EPC069-12 exists for EUR only, a CHF quote carries a QR-Bill the first check refuses, so CHF fails closed into the hint with the PDF button. The #1305 test coverage travels along — per-currency unit cases, the CHF toggle and the two non-Frick-currency e2e cases — and the three CHF baselines are regenerated on this branch, which changes the toggle's look the same way it does for EUR. * Gate the rewrite on the quote currency and the EPC field count The EUR binding rested on the backend's payload contract — a CHF quote carries a QR-Bill the svg check refuses. Now the quote's own currency must say EUR before anything else is read, and a payload longer than EPC069-12's twelve fields is not reassembled. The three currency e2e cases inherit the file's lang=en pin, and the CHF fail-closed QR state gets its own baseline: hint and PDF button, no code. * Keep the collection QR when the quote carries no source amount The amount line is optional in EPC069-12, and the api leaves it empty whenever the customer quotes by target amount — the two amount fields are mutually exclusive, so buying 0.05 ETH sends no source amount at all. The gate demanded EUR plus a figure and refused those payloads, so the collection QR silently disappeared for a perfectly ordinary flow while the personal QR still rendered. An absent amount contradicts nothing: it is the same absence the personal QR carries, and the customer types the figure in their banking app. A populated amount must still match the quote to the cent. * Re-apply the collection work on top of the Yapeal generalization #1320 rewrote the same five files while this branch waited, so the rebase left them holding one side or the other. They now carry both: develop's provider generalization untouched — the Yapeal parsing, the precedence helper, the provider switch and every test around them — and this branch's work lifted back on top of it. Where the two genuinely met, the collection toggle keeps its own state in the content component so the QR branch can see it, and the provider switch stays wired exactly as develop passes it down; both buttons sit in the same IBAN row. The dependency bump is gone: develop already carries a newer SDK than this branch pinned. * Refuse a malformed amount line and compare the amount exactly A line of spaces is not an absent amount: absent states nothing, spaces state nonsense, and everywhere else this function refuses malformed input rather than passing it through. Only a genuinely empty line takes the no-amount path now. The comparison also stops rounding to cents, which let a quote of 100.004 pass against a payload reading EUR100.00 while the screen showed and copied the unrounded figure. Plain equality accepts every canonical spelling — EUR100 and EUR100.00 both parse to 100 — and rejects the divergence. The e2e case that pinned the empty-amount payload is gone with them: it carried no screenshot, so it proved function rather than appearance, and the same property is already pinned where it belongs, in the unit test. * Let the payload decide its own decimals, and declare the fakes Two decimals is the standard's limit for what a payment carries, not a limit on what a customer may enter: the amount field takes any figure and the api writes it into the payload unformatted, so a purchase of 100.004 produced a line the grammar rejected and the collection QR disappeared for it. The character guard stays — plain decimal digits, so nothing can be read as a number other than what stands there — while the equality against the displayed amount does the deciding, which is the only property that ever mattered here. The e2e cases this branch adds fake two things the suite never faked before: a customer's KYC completeness and the invoice endpoint's rejection. CONTRIBUTING requires each fake to be declared with what a green run does not prove, and this repository had nowhere to put such an entry — the section exists now, with all three fakes of these specs. * Compare the amount line against what the api writes Three rounds tuned a decimal bound that was arbitrary in both directions: too tight and a legitimate purchase lost its QR, too loose and two different amounts collided onto the same floating-point value near a hundred million, so a payload could carry an amount the screen never showed. The bound is gone. The api renders that line as the currency prefix followed by the quote amount, so the exact string is known here and comparing against it needs no parsing, cannot be read as a number other than what stands there, and leaves nothing left to tune. A payload spelled differently is not this api's output and is refused. * Count both specs in the reality declaration Both collection-invoice cases install the KYC and invoice-400 fakes, not one. * Refuse a payload that names another creditor The rewrite kept line 5 as it found it and trusted the toggle's own gate for the holder. Both accounts are held by DFX AG, so a payload naming anyone else is not one this function may retarget. * End the creditor name at its boundary A bare prefix test also accepts a company whose name merely begins with the holder's, so the name must end at the address separator or the line's end. --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the CHF deposit cutover (#1305) and the API half DFXswiss/api#4813: customers who still hold an active legacy Yapeal CHF personal IBAN see their new Bank Frick IBAN by default and can flip back and forth with a provider toggle — with the guarantee that every PDF names exactly the IBAN that was on screen.
Open, and not closable from this branch
bankfield onGET /buy/personalIbanthe switch offer never appears and this frontend behaves exactly as today (feature inert by construction). But the feature only exists once the API is live.feat(core): add the Yapeal personal IBAN provider member packages#211 merged and released, then the dependency bump lands here.Done: the SDK is published as@dfx.swiss/react1.8.0-beta.2 (pulling in@dfx.swiss/core0.7.0-beta.1) and consumed from the registry — the local tarball override is gone and CI is green.What
GET /buy/personalIban, now naming the issuing bank per row). An explicit decision tree — no fallback cascade — picks the provider: a toggle click always wins; an explicit URL/widget selector stays untouched; otherwise an existing still-payable Yapeal holder with KYC ≥ 50 getspersonalIbanProvider=Frick; otherwise no selector is sent and the server default (the existing Yapeal row) applies. Below KYC 50 nothing changes at all.bankId/virtualIbanIdon the transaction request (api#4813). The PDF button hangs off that request's id, so invoice and receipt rebuild from the stored IDs — the document can never name a different account than the screen did, in either toggle state.KycRequired(stale local KYC knowledge), the screen re-quotes without a selector and keeps a permanent hint ("we are showing your existing IBAN instead") — no blocking KYC screen for a choice the customer never made. An explicit toggle click or URL selector keeps the ordinary KYC error path.normalizePersonalIban/toPersonalIbanProviderRequest/isUnrecognizedPersonalIbanSelectornow recognize every SDK enum member instead of hardcoding Frick, sopersonal-iban=Yapealin the URL is an explicit (fail-closed, server-validated) selector instead of being silently dropped; the newPersonalIbanProviderNotAvailabletoken maps to customer-readable copy.GET /buy/personalIbanis consumed viauseBuy().getPersonalIbans(feat(core): add the Yapeal personal IBAN provider member packages#211) inside a sharedusePersonalIbanRowshook; the provider decision tree itself is a pure, unit-tested helper. The list is identity-tagged: an in-place account swap resets rows, override and fallback state (all of them — the KYC-fallback flag included), and neither a late list response nor a late quote response can commit for an account that is no longer signed in — the quote signature carries the customer identity, the committed payment info is stored together with the identity, method and the provider that was actually sent, and every commit re-checks the identity. KYC itself is evaluated only against a user object whoseaccountIdmatches the session identity; a foreign user counts as still loading. Completion and in-flight confirmation state reset on an identity change too, completion commits only on a successful confirmation (a rejected confirm surfaces as an error instead of a "Done!" screen), the confirm callback guards against unmount, and confirming is only possible once the final (exact-price) quote of a generation is displayed — each quote request persists its own transaction request, so this closes the window where a confirmation could target the approximate request while the screen moves on to the exact one. The displayed verification follows the sent provider (an explicit Yapeal answer after a URL-Frick entry is valid), the request provider derives from the same debounced snapshot as the request currency, the first quote additionally waits while the user context itself is loading, and both waits fail open after ten seconds so the core buy flow can never hang on the optional switch offer./buy, on the explicit "I have issued the transfer" action on/buy/info) and render only from that snapshot. Late-arriving context — a user reload, the personal-IBAN row list, a KYC re-derivation — can still re-quote in the background, but it can no longer swap the information a customer already acted on, neither on screen nor in the close payload handed to the host application. The snapshot is cleared together with the completion flag on an identity change. Theuserprop stays the raw context value on purpose: the completion component treats a missing user as "still loading" and then renders only a spinner while the screen suppresses the back button, so an identity-filteredundefinedwould strand a customer who has already paid — and that value gates nothing but the spinner and a copy variant, never account data.PersonalIbanProviderNotAvailable), the error offers a "Show available IBAN" action that clears the selection and re-quotes selector-free — matching what the error copy promises, instead of a retry-only loop.buy.screenandbuy-info.screenshare the row hook and the derive helper and keep only their flow-specific wiring;payment-info-buyonly renders the toggle.Tests
getYapealAlternative(bank/currency/active/acceptsPayments/status filters),isVerifiedYapealPersonalIbanResponse, and the new error-token mapping — full branch coverage onpersonal-iban.ts.resetMocks: true, which stripsjest.fn()implementations before every test, and an unstable factory reference re-fires the load effect every render. Plain functions pinned inside the factory survive both;createPersonalIbanrejects loudly if a suite ever reaches it.KycRequired, selector-less retry, permanent hint). The /buy/info suite carries the full scenario set (default Frick, toggle, fallback, hint clear, account swap, load rejection), and dedicated tests pin the same-snapshot, identity and combined user-timeout races with deferred promises and fake timers, plus completion-leak regressions on both screens.YAPECHZ2in the fixture) and back; the provider-unavailable rejections run against fully static/v2/userfixtures (with theaccountIddecoded from the live session JWT, keeping the fixture coherent with the identity check) and the complete NestJS error shape, assert the new recovery action and click it, pinning the selector-free follow-up request in the request body. Eight committed baselines, each proven by a second run against the committed file, all visually verified.Per-file coverage and declared deviations
Measured over the full suite on this head (statements / branches / functions / lines):
src/util/personal-iban.tssrc/hooks/personal-iban.hook.tssrc/hooks/personal-iban-rows.hook.tssrc/hooks/virtual-iban.hook.tssrc/components/payment/payment-info-buy.tsxsrc/components/payment/buy-completion.tsxsrc/screens/buy.screen.tsxsrc/screens/buy-info.screen.tsxDeclared deviation (CONTRIBUTING § Coverage, reviewer's call): the two screens and the payment-info component do not reach 100% on the whole file — the gap is dominated by pre-existing screen scaffolding this PR does not touch (the same state these files carried into #1305). The new decision tree, both toggle directions, the KYC fallback, the account-swap resets and the race guards are pinned by dedicated tests; a small remainder of defensive guard branches inside the new code is exercised only through the scenario suites rather than each by a dedicated test. Bringing an ~1100-line screen to 100% belongs to a preparatory coverage PR, not under this title.
Declared deviation (§ full-stack test): no
e2e-stack/scenario is added here — the same declared deviation as in #1270, which is itself still open. The scenario needs a grandfathered Yapeal row and DFXswiss/api#4813 inside the stack image — until that API lands in the harness, the case is untestable there; the PR's own static Playwright suite pins both toggle states deterministically on/buyand/buy/info. It follows once the API half is merged into the stack.Pre-existing, reported not fixed
buy.screen.tsxcarries an emptyonSubmithandler with a 2023 TODO ("fix broken form validation and onSubmit"); the actual submit path runs through the button handlers, so the dead handler has no runtime effect. It predates this PR (git blame 2023) and is reported here per the review rule rather than silently skipped; fixing the form wiring belongs to its own change.useVirtualIbanstill firesPOST buy/personalIbanthrough a rawuseApi().callinstead of the SDK, which CONTRIBUTING asks to avoid. It is pre-existing and identical ondevelop; this PR only narrows the file (theGETmoved touseBuy().getPersonalIbans, and the response type now comes from the SDK). The remainingPOSTbelongs topersonal-iban.screen, not to the provider toggle, and migrating it means adding the endpoint to the SDK first — that would reopen the already-reviewed packages PR this branch is waiting on for its release. Reported here rather than silently skipped; it follows in its own change once the SDK release has landed.developmerge-base against the same stack): the real-quote Frick issuance test needs a KYC ≥ 50 seed account and currently valid price rows, and the two Wallet-2 screenshots compare against baselines committed long before this branch that encode a different seeded account state. Fixing the seed or regenerating those baselines is environment work outside this diff.Evidence
Local chain on this head (Node 20, SDK tarball override, own API stack for the e2e auth):
lint,CI=true npm test(84 suites / 1,052 tests),build:dev,widget:dev, index-swap check — all green. The eight provider e2e cases (/buyand/buy/info: both toggle directions, the KYC fallback and the provider-unavailable rejection) run deterministically against their eight committed, visually verified baselines; the two full spec files pass twice in a row with the identical result set (16 of 19, remainder = the three pre-existing environment cases above).One test-only correctness find along the way: the static quote mocks omitted
asset.uniqueName, which the buy screen's exact-price echo suppression compares — every static quote therefore triggered a redundant reverse quote cycle that could replace the payment info mid-interaction and reset the collection-IBAN toggle. All eleven static quote mocks now carry the field the real API always returns.