Add components for manual and gift card payment settings - #821
Open
gciotola wants to merge 21 commits into
Open
Conversation
✅ Deploy Preview for commercelayer-react-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
commit: |
Consolidate the two existing ADRs into docs/adr/ and add three new ones covering the payment_sessions model: how the Payments Model is detected, the Payment Session lifecycle, and the PlaceOrderButton split. Switch to a date prefix for new ADRs. Sequential numbers collide silently across parallel branches, which is how the repo ended up with two 0001s. Moving the react-components ADR to the repo root also fixes six code comments that already referenced it at that path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure rename ahead of the Payments Model split. The component's logic is untouched so the history stays blame-able: its enablement machine is built on payment_method / payment_source / payment_response / onsubmit refs, none of which exist on the payment_sessions model. PlaceOrderButton is a temporary re-export alias, replaced by the router in the next commit. The existing spec now targets the branch directly, since its fixtures carry no available_payment_methods and would otherwise resolve to the undetermined state. See docs/adr/2026-08-18-place-order-split-by-payments-model.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the first slice of the newer payment model, alongside the existing one rather than replacing it. An application on the older model upgrades without touching its component tree. Detection. usePaymentsModel() derives the model from the order: available_payment_settings wins over available_payment_methods when both are present, which 2026-05 allows since it is purely additive. <Order> now asks for available_payment_settings on every fetch — without it an absent array cannot be told apart from one never requested. Domain logic in core-components. Session creation, the reuse and selection predicates and the place-order machine are plain functions with no React, so they are testable without rendering — which is exactly what the 598-line PlaceOrderButton is not. Selection. Choosing a setting creates a Payment Session with no amount_cents; the server sizes it to the remaining amount and silently caps an explicit value. Selection is derived from the order on every render, never held locally, and is single per order: switching leaves the previous session behind — deleting it may be refused for a sales-channel token — so the most recent live session wins. Placing. The authorization is created at place time, not at selection, so changing payment method never has to undo an accounting record. _placeable is then retried 5 times at 1s: authorizing is asynchronous, so an early refusal is not a real failure. There is no client-side coverage gate — the threshold is a payment rule an organization can change. PlaceOrderButton is now a router over the two branches; the older one is untouched. See docs/adr/2026-08-18-*.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps found while mounting these components in mfe-checkout. <PaymentMethod> kept rendering. Only <PaymentSetting> stepped aside for the other model, but 2026-05 is additive: an order on the payment_sessions model still carries available_payment_methods, so both trees rendered and the shopper saw two sets of payment options, one of them dead. This is where the precedence rule actually takes effect — without it, mounting both trees together needs a conditional in every consuming app. <PaymentSetting> had no way to report a selection. <PaymentMethod> has onClick; the counterpart was missing, so an app could not refresh its own order state when the choice changed. onSelect fires after the selection is stored and the order refetched, not on click. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The include was never registered, so every order loaded without available_payment_settings and usePaymentsModel() reported payment_source for orders that carry both arrays — the payment_source tree rendered and the newer one stayed empty. The guard was reading `withoutIncludes` as a consumer opt-out. It is not: it starts `true` and means "nothing has asked for an include yet", and registering one is what flips it. Skipping on `true` meant skipping always. Also drop the second phase: addResourceToInclude marks the resource loaded in the same call, so the two-phase idiom the containers use is belt-and-braces and one call suffices. Covered by a spec asserting the retrieve params, for both this relationship and the nested session ones <PaymentSetting> registers — they have to reach the *initial* fetch, since adding an include later does not refetch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Returning null from render does not stop a mounted component's effects. PaymentMethod stepped aside visually but its auto-select effect still ran, wrote a payment_method and created a payment source — and the API drops available_payment_settings for an order with either set (order_payments.rb:169 via old_payments_engaged?). The order then read as payment_source, so the older tree took over: settings appeared for an instant, then vanished for good. Guard the two order-writing effects on the model as well as the render. Observed end to end: a fresh order lost its five payment settings on first page load and kept them afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tton PrivacyAndTermsCheckbox announced a change through PlaceOrderContext in container mode and through a DOM event only in standalone mode. The newer place-order button uses neither context nor standalone detection, so under a (deprecated) <PlaceOrderContainer> — which mfe-checkout still mounts — it never heard the checkbox and stayed disabled for good. Dispatch the event in both modes. It is additive: the older branch keeps working through the context call, which still runs first. Two specs asserted the old "no event in container mode" contract and now assert the new one. Verified in the browser: ticking the checkbox enables the button on an order using the manual payment setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
usePaymentsModel() held the rule itself, so an application's own data layer could not reach it — mfe-checkout had to copy the precedence logic into its provider, where a hook cannot be called. Two copies of a domain rule that must never disagree. getPaymentsModel(order) is now a plain function in core-components and the hook is a thin binding over it. This is the boundary the ADRs set out: domain logic without React in core-components, React on top. The derivation cases move to the function's own spec; the hook keeps only what proves the binding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
<PaymentSetting> holds one errors state for the whole list while isPending is per-setting. Invisible today with only the manual setting implemented; the moment a second exists, a Stripe selection error renders under the manual entry. Filed next to the implementation table rather than as its own ADR: that table is what the person adding the second setting reads, and that is exactly when this bites. Also record why <PaymentSetting> must stay mounted on the older model — it registers the payment-session includes, which has to happen before the order is fetched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
<PaymentMethod> has autoSelectSinglePaymentMethod and the symmetric prop belongs on <PaymentSetting>, but the obvious condition is a trap: the component renders only implemented settings, so "one entry rendered" is not "one option offered". Today that would commit shoppers to bank transfer while the card options the organization pays for stay invisible. Under the correct condition — one entry in the raw available_payment_settings — it cannot fire on any organization that still has unimplemented settings, so writing it now means shipping it untested. Deferred to the second setting. Also record the observation behind it: creating a session drops available_payment_methods to zero, the mirror of what writing payment_method does to available_payment_settings. Whichever tree acts first commits the order, which is why neither may act without the shopper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sion Reporting the last refusal tells someone whose payment is still settling that something went wrong. The direction to explore is a thank-you page with a payment-status summary and a refresh, re-offering the payment components when something did fail — but it reaches into checkout navigation and post-placement behaviour, so it needs its own questions rather than a decision made in passing. Records the interim rule too: on timeout nothing is rolled back, so a gift card authorized during a timed-out attempt stays bound to the order and loses its remove control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First slice of the gift card iteration: the pure derivations and the API operations, with no React involved. derivePaymentSessionsState is the single source for every number the payment UI needs. The remainder is derived rather than read from order.session_amount_cents, which does not move until a session is authorized — and gift cards are authorized at place time, so the server's figure stays at the full total for as long as the shopper is choosing. What is summed are the amounts the server itself computed per session. applyGiftCard sends an explicit amount_cents from the second card onwards. Without it the server sizes each card against a remainder that has not moved: on a 7100 order a 2000 card followed by a 5000 one produces sessions of 2000 and 7100 — 9100 of credit for a 7100 order, and nothing server-side prevents it. Sending an amount is safe because the server clamps it down, never up. The "never send amount_cents" rule still holds for the session paying the difference. (Note the playground has this bug; it is not a precedent.) Applying or removing a gift card also deletes the session paying the difference: amount_cents is immutable, so once the remainder moves that session is not stale but wrong. Bound to the action that causes it rather than to an effect comparing amounts. Only sessions that have taken nothing are deleted — the API refuses otherwise and surfaces the refusal as an unhandled 500, and a sales-channel token cannot delete an authorization. A charged gift card can only be refunded, which this iteration does not implement. placeOrderWithPaymentSessions now takes the order and authorizes N gift cards before the optional session paying the difference, stopping at the first failure. Gift cards are excluded from findCurrentPaymentSession: they are additive, not one of the alternatives a radio group picks between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second slice: the React surface over the domain operations. usePaymentSessionsState binds derivePaymentSessionsState, and is the one source for the applied cards, what is still owed, and whether anything is left to pay — read by the gift card components, the method selector, <TotalAmount> and <GiftCardAmount>. Seven PaymentSettingGiftCard* components, mounted outside <PaymentSetting>: gift cards are additive rather than one of the alternatives a radio group picks between, so several can be active at once. The input and its submit appear and disappear together, swap for an "add another" control after the first card, and all three vanish once the order is covered or anything has been authorized — applying a card that is not needed fails with a 422 about amount_cents that a shopper cannot act on. Gift card errors live on their own context, apart from the method's: the two families have separate UIs and a failure in one must never surface under the other. That closes the shared-error debt that was due with this setting. <PaymentSetting> renders nothing once nothing is left to pay, and takes a readonly prop that shows only what was chosen — the same recap <PaymentSource readonly> gives the older model, which mfe-checkout was having to hand-roll. <TotalAmount> deducts the gift cards on this model. On the older one the same gift card was a negative line item and the total already came back net, so showing the gross would tell a shopper to pay money already covered. <GiftCardAmount> sums the sessions, since gift_card_amount_cents stays zero. A spec caught a real hazard on the way: isCovered read as true whenever the total was 0, so an order fetched without total_amount_with_taxes_cents in its fields hid the entire payment step. It now requires a total to exist before reporting coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gift cards are handled by <PaymentSettingGiftCard>, so the radio group filters them out — but it was doing so through the "not implemented yet" branch and warning about them in development. They are implemented; they just live elsewhere, because they are additive rather than one of the alternatives the group picks between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A gift card is a payment here, not a discount: the order total never moves and what drops is the amount still owed. That inverts the older model, where the gift card was a negative line item and the total already came back net. The new ADR carries the facts that cost this iteration the most to establish: session_amount_cents does not move until authorization, so the remainder has to be derived; the second gift card would otherwise be sized for the whole order, which is why it is the one place an explicit amount_cents is sent (and why the playground's version over-covers); authorizing a gift card charges it immediately and a void always fails by construction; and deleting a charged session comes back as an unhandled 500. Updates the manual-setting ADR where this supersedes it — the amount_cents rule, the deletion rule reformulated as "only what took no money", the shared-error debt closed, the setting table — and extends CONTEXT.md with Applied Gift Card, Remaining Amount and Coverage. Also corrects the Current Payment Session entry, which still claimed its amount equals the order total. With gift cards it covers the remainder, and it is never a gift card session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…le checkout page ui
The rebase landed on a main that had replaced the privacy & terms channel: `PLACE_ORDER_RECHECK_EVENT` and the `privacy-terms` localStorage key are both gone, and acceptance now lives in the in-memory `termsAcceptanceStore`, keyed by order and deliberately not persisted. `PlaceOrderButtonPaymentSessions` read the removed key behind the removed event, so the gate could never open. It now subscribes through `useTermsAndConditions()`, which is the supported channel and the same one `placeOrderPermitted` reads — so the two branches cannot disagree. The additive DOM event this branch had added to `<PrivacyAndTermsCheckbox>` is no longer needed and goes with it. `terms-acceptance.spec.tsx` arrived from main with an order carrying neither available_payment_methods nor available_payment_settings. With `<PlaceOrderButton>` now routing on the Payments Model that order is undetermined and gets the inert button, so most of the suite was passing for the wrong reason. The fixture now names its payment methods. Also two lint errors that only surface with main's biome: the `paymentsModel` guard added to PaymentMethod's effects was missing from their dependency lists, and PaymentSettingGiftCardErrors keyed its spans by array index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every place attempt used to open with a `_placeable` that was guaranteed to
fail. Authorizing runs in a background job, so the check ran while the money
was still moving and the API answered 422 "the payment doesn't cover the
minimum required percentage" — three runs out of three against a real order.
The retry recovered, so the only visible trace was a console error, but the
budget was really four attempts out of five and a slower API would have
reported a coverage failure on a payment that was about to succeed.
Each attempt now reads the order back before touching `_placeable`. The
authorization states are what says whether a refusal is worth waiting on, and
`_placeable` cannot see them. That turns the wasted attempt into a cheap GET
and lets the loop end as soon as an authorization has actually failed, instead
of spending the whole budget waiting for a verdict that already arrived. The
cost is flat in the number of sessions, since every state arrives in the same
GET — which a per-session wait could not promise. Defaults move to 8 attempts
of 500ms accordingly.
The last attempt asks anyway: `_placeable`'s refusal is the only message the
API gives us, and a place that fails with nothing on screen is worse than one
that reports the coverage rule. `requires_action` is deliberately not treated
as in flight — it waits for the shopper, not the server, so polling it would
spend the budget on a flow this iteration does not implement.
Note the failure check is scoped to the sessions paying for this order. A
session burnt on an earlier attempt stays on the order after the shopper
re-selects, and reading it as this attempt's verdict would end the loop before
the authorization just created had a chance to settle.
Two races go with it:
- `_place` refused on an order `auto_place` placed in the window after the
placeability check passed. Being a state error rather than a payment rule,
it did not map to a placeability error and was rethrown as a hard failure
on an order that was paid for and placed. It now re-reads the order and
treats a placed one as the success it is.
- the place button refetched the order after a reported error but not after a
thrown one. Authorizations may already have been created when it threw, and
the stale order still showed their sessions without one — which reads as
"nothing has been charged yet", so a second click authorized again and took
the money twice.
Verified against real orders on the new-payments-demo organization: wire
transfer alone, and a 10 USD gift card plus wire transfer for the difference.
Both place with a single `_placeable`, no 422, and payment sessions summing
exactly to `total_amount_with_taxes_cents`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`getOrganizationConfig` rejected when its request failed, and none of its eight call sites caught it. Several are `useEffect` bodies and async click handlers, so the rejection went unhandled and reached the host application — where, under `next dev`, the error overlay covers the page and absorbs every click. One optional setting failing to load took the whole checkout down. `null` is already what this function returns when there is no config to give, and every caller reads the result optionally and falls back to a computed application link, so returning it on a failed request needs no change anywhere else. Only the request is guarded: a bad token or a malformed config is a fault to surface, not a blip to absorb. The log level is load-bearing rather than a matter of taste. `next dev` promotes a `console.error` to an overlay issue whose dialog covers the page just as an unhandled rejection does, so reporting a condition we have just recovered from at that level defeats the point of recovering. Verified both ways by aborting the request and watching a click either land or time out on an element that was plainly visible. Found while building the payment_sessions e2e suite in mfe-checkout, where this turned an intermittent API failure into a suite that could not run. The organization config is fetched several times per page load, so the odds of catching one are better than they look. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gciotola
force-pushed
the
feat/820-payment-sessions-structure
branch
from
September 2, 2026 11:54
d7c6eea to
b26c180
Compare
gciotola
marked this pull request as ready for review
September 2, 2026 14:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #820
usePaymentsModelto understand which payments model the application should usePlaceOrderButtonthat now is a simple router for the the payment modelsI am currently using preview versione of commercelayer-sdk that implements api versioning.
We will update to final version when it will be released as tracked in #829