feat: generic checkout page — dynamic products + Stripe (#55) #56
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/westside-app!56
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "55-feat-generic-checkout-page-dynamic-produ"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
/checkoutroute consumes generic checkout API from basketball-api #127custom_fieldsJSONChanges
src/routes/checkout/+page.svelte: Main checkout page — fetches products by category, renders cards with select interaction, dynamic form fields (number/select/text), Stripe session creation, validationsrc/routes/checkout/success/+page.svelte: Order confirmation pagesrc/routes/checkout/cancel/+page.svelte: Payment cancelled with retry linkTest Plan
npm run buildpassesReview Checklist
Related
plan-wkq— Phase 11 girls jersey checkoutPR #56 Review
DOMAIN REVIEW
Tech stack: SvelteKit 5 (Svelte runes /
$state), vanilla CSS, Stripe redirect flow, parent-token auth pattern.Security
{@html}usage anywhere -- all dynamic content is auto-escaped by Svelte. XSS risk is mitigated.create-session. This matches the existing pattern fromjersey/+page.svelte. The token never touchesinnerHTMLor similar.categoryquery parameter is interpolated into the fetch URL (/checkout/products?category=${category}). Since this hits a same-origin API and is not rendered as HTML, the frontend risk is low. Backend should validate/whitelist the category value.UX / Responsiveness
max-width: 700px,padding: 2rem 1remgives breathing room at 375px.checkout-card-featuredclass is applied based onprice_cents > 0 && product_type !== 'opt_out'but there is no CSS rule for.checkout-card-featured-- the class does nothing. The old jersey page had a visual badge for "Best Value"; this page lost that distinction.Svelte Patterns
$state,$page).{@const selectedProduct = ...}inside the template block is idiomatic Svelte 5.gotoimport is used butgotois only called for opt-out redirect -- appropriate.Accessibility
onclickon adiv(checkout-card-body) withoutrole="button",tabindex="0", oronkeydownhandler. Keyboard-only users cannot select a product by pressing Enter/Space on the card body; they must tab to the button inside. This is a partial mitigation since the button exists, but the card body click area is a dead zone for keyboard users.<label>elements with matchingfor/id-- good.role="alert"oraria-live="assertive", so screen readers may not announce validation errors.aria-labelor screen-reader text -- loading state is visually indicated only.BLOCKERS
1. BUG:
checkout_tokenis never written to sessionStoragecheckout/cancel/+page.svelteline 8 reads:But
checkout/+page.sveltenever callssessionStorage.setItem('checkout_token', token). The jersey page (jersey/+page.svelteline 26) has the equivalentsessionStorage.setItem('jersey_token', token)call -- this was missed in the port.Result: The "Try Again" link on the cancel page will never render. Users who cancel Stripe payment land on a dead-end page with no way back.
Fix: Add
sessionStorage.setItem('checkout_token', token)incheckout/+page.svelte'sonMount, after the token-null check, mirroring the jersey page pattern.2. Cancel page hardcodes
category=jerseycheckout/cancel/+page.svelteline 23:This page is supposed to be the generic checkout cancel page, but it hardcodes
category=jersey. If the checkout flow is used for any other product category in the future, the retry link sends users to the wrong product listing.Fix: Store the category alongside the token (e.g.,
sessionStorage.setItem('checkout_category', category)) and read it back in the cancel page.3. No test coverage
550 lines of new functionality with zero tests. The Test Plan in the PR body has only unchecked manual test items. There are no automated tests anywhere in this repo (no
.test.jsor.spec.jsfiles outsidenode_modules).Per BLOCKER criteria: "New functionality with zero test coverage."
Mitigation note: This repo has no test infrastructure at all (no vitest, no playwright config). This is a systemic gap, not specific to this PR. If the team decision is that westside-app does not require automated tests at this stage, this blocker can be waived by explicit owner approval. But it must be acknowledged.
NITS
Unused import:
checkout/cancel/+page.svelteimportspagefrom$app/stores(line 2) but never uses it. Dead import.Unused CSS class:
.btn-secondaryis defined incheckout/+page.sveltestyles (line 429) but never applied in the template. Dead CSS.DRY: duplicated submit button block: The submit button markup is duplicated between the
{#if selectedProduct?.custom_fields?.length}and{:else}branches (lines 213-225 vs 229-241 in the on-disk file). These are identical blocks. Could extract to a snippet or restructure the conditional so the button is always rendered outside the custom-fields check.formatPricedoes not handle fractional cents:Math.round(cents / 100)will round $89.99 (8999 cents) to$90. If the API ever returns non-round-dollar amounts, this will silently truncate. Consider(cents / 100).toFixed(2)with a dollar sign, or keepMath.roundbut document the assumption.No empty-products state: If the API returns an empty array, the page shows the header and an empty grid with no message. Add a "No products available" fallback.
checkout-card-featuredclass has no CSS: The class is conditionally applied (line 150) but has no corresponding style rule. Either remove the class or add the intended visual treatment.Error banner accessibility: Add
role="alert"to the.checkout-errordiv so screen readers announce validation errors.Card keyboard accessibility: The
checkout-card-bodydiv has anonclickbut no keyboard interaction support. Either addrole="button" tabindex="0" onkeydownor remove the div click handler and let the button be the sole interaction target (with visual styling to make this clear).SOP COMPLIANCE
55-feat-generic-checkout-page-dynamic-produreferences issue #55plan-wkqPhase 11PROCESS OBSERVATIONS
jersey/routes and the newcheckout/routes serve overlapping purposes. Oncecheckout/is validated, the oldjersey/routes should be deprecated/removed to avoid maintaining two checkout flows. Worth tracking as a follow-up issue.VERDICT: NOT APPROVED
Two functional bugs (sessionStorage never written, hardcoded category) must be fixed before merge. The test coverage blocker is acknowledged as a systemic repo gap -- recommend explicit owner decision on whether to enforce or waive for westside-app.
PR #56 Re-Review
DOMAIN REVIEW
Stack: SvelteKit (Svelte 5 runes), vanilla CSS, Woodpecker CI, Stripe integration.
Previous blockers -- verification:
sessionStorage key mismatch -- RESOLVED.
src/routes/checkout/+page.sveltelines 36-37 now writecheckout_tokenandcheckout_categoryto sessionStorage.src/routes/checkout/cancel/+page.sveltelines 8-9 read those exact keys. No mismatch.Hardcoded
category=jerseyin cancel retry link -- RESOLVED. Cancel page readscheckout_categoryfrom sessionStorage (line 9, defaulting to'jersey') and passes the dynamic{category}variable in the retry link (line 24).Woodpecker CI fixes:
git init+git remote add+git fetch ${CI_COMMIT_SHA} --depth 1+git checkout ${CI_COMMIT_SHA}-- this handles both branch pushes and PR merge commits whereCI_COMMIT_BRANCHmay not exist.pull_requestevent added to thewhenclause, and the validate step'swhenincludespull_requestin its event list. Correct.Accessibility fixes:
src/routes/register/+page.svelte: All<label>elements now haveforattributes matchingidon their inputs. Radio groups correctly use<span>+role="radiogroup"+aria-labelledbypattern for both age gate and payment method. Photo upload label correctly changed to<span>since the existing<label for="photo">below handles the association.src/routes/players/[id]/+page.svelte: Contract signature label now hasfor="contract-sig"matchingid="contract-sig"on the input.src/routes/players/[id]/billing/+page.svelte: Stripe element mock labels correctly changed from<label>to<span>-- these are not real inputs, so<label>without aforattribute was generating warnings.src/routes/coach/+page.svelte:flatMap/mapcallbacks now have/** @type {any} */JSDoc annotations, fixing implicit-any warnings.All changes are clean and correct.
Checkout page architecture review:
custom_fieldsJSON -- good extensibility.formatPriceusesMath.round(cents / 100)-- works for whole-dollar amounts (which these are).BLOCKERS
None. Both previously identified blockers are resolved.
Note on test coverage: This repo has no test framework (
package.jsonhas notestscript, no test files exist). CI validation issvelte-check+npm run build. Given this is a parent-facing checkout page with Stripe integration, adding a test framework (Vitest + Testing Library) is strongly recommended but is out of scope for this PR and should be tracked separately.NITS
Checkout page product cards use
onclickon a<div>(line 156): Thecheckout-card-bodydiv hasonclickbut norole="button",tabindex="0", or keyboard handler. Screen reader and keyboard-only users cannot select products via the card body. The<button>below it works for keyboard, but the card body click target is inaccessible. Consider addingrole="button" tabindex="0" onkeydownor making the entire card a button.Token in query string is logged in server access logs: The checkout token appears in the URL (
?token=...). This is standard for email-based magic links and Stripe's own pattern, but worth noting for log-scrubbing awareness.formatPricetruncates fractional dollars:Math.round(cents / 100)drops cents. If a product ever costs $12.50 (1250 cents), it would display as$13. Consider(cents / 100).toFixed(2)if fractional prices are possible..btn-secondarydefined in checkout page CSS but unused: Line 433-435 of+page.sveltedefines.btn-secondarybut it is never applied in the template. Dead CSS.Success page has no navigation: After checkout, the success page shows a message but no link back to the main site. Users are left on a dead-end page.
SOP COMPLIANCE
55-feat-generic-checkout-page-dynamic-produreferences issue #55)plan-wkq)svelte-checkPROCESS OBSERVATIONS
for/idattributes,<label>to<span>changes).VERDICT: APPROVED
PR #56 Review
DOMAIN REVIEW
Tech stack: SvelteKit 5 (Svelte 5 runes), static adapter, Keycloak auth, Stripe checkout integration, Woodpecker CI. Applying SvelteKit/accessibility/security/CI checklists.
Checkout page (
src/routes/checkout/+page.svelte)Query parameter injection in API URL (line 40): The
categoryvalue is read from the URL query string and interpolated directly into the API URL without validation or encoding:A crafted URL like
?category=jersey%26admin=truewould inject additional query parameters into the backend request. UseencodeURIComponent(category)or validate against an allowlist of known categories.formatPricetruncates fractional dollars (line 117):Math.round(cents / 100)drops cents -- 9950 cents renders as$100instead of$99.50. Use(cents / 100).toFixed(2)or at minimum.toLocaleString()with currency formatting. This is a correctness bug for any non-round-dollar product.Checkout card
onclickdiv is not keyboard-accessible (line 156): The.checkout-card-bodydiv hasonclickbut norole="button",tabindex="0", oronkeydownhandler. Keyboard-only users cannot select a product by clicking the card body. The button in the footer IS accessible, so this is degraded but not broken -- keyboard users can still reach the "Select" button.Error banner missing
role="alert"(line 145): The.checkout-errordiv renders dynamic error text but lacksrole="alert"oraria-live="assertive", so screen readers won't announce errors when they appear.DRY: submit button duplicated (lines 217-229 and 233-245): The submit button with its conditional text (submitting spinner / price / confirm) is copy-pasted between the
custom_fieldsbranch and the no-fields branch. Extract to a{@const}block or a shared snippet.outline: noneon focus (line 413):.checkout-input:focus { outline: none; }removes the default focus indicator. Theborder-colorchange to#d42026on#0a0a0abackground provides 2.7:1 contrast which does not meet WCAG 2.1 AA for non-text elements (3:1 minimum). Consider adding a visible focus ring or increasing the border width on focus.Unused CSS class:
.btn-secondaryis defined but never used in the checkout page template.Success page (
src/routes/checkout/success/+page.svelte): Clean and minimal. No issues.Cancel page (
src/routes/checkout/cancel/+page.svelte): Token retrieved fromsessionStoragefor the retry link is appropriate. TheencodeURIComponentconcern applies here too -- the token and category are interpolated into thehrefwithout encoding, though since the token originated from a URL param and was stored as-is, the risk is low.CI config (
.woodpecker.yaml): The clone fix (git init+git fetchby SHA) is the correct pattern for PR events whereCI_COMMIT_BRANCHmay not resolve. Theevent: pull_requesttrigger addition is necessary for PR validation. Both changes are correct.a11y fixes (
register,players/[id],players/[id]/billing): All correct.for/idassociations properly wired.<label>correctly changed to<span>for elements without a single associated input (Card Number, Expiration, CVC mocks; Player Photo; radio groups).role="radiogroup"+aria-labelledbycorrectly added to age gate and payment method radio groups.TypeScript fix (
coach/+page.svelte): JSDoc@type {any}annotations onflatMap/mapcallbacks are correct for untyped API responses.BLOCKERS
categoryquery param not encoded before API interpolation --src/routes/checkout/+page.svelteline 40. User-controlled input (categoryfrom URL) is concatenated directly into a fetch URL. While the backend likely validates this, the frontend should eitherencodeURIComponent(category)or validate against an allowlist (['jersey', ...]). This is unvalidated user input flowing into an HTTP request.formatPricecorrectness bug --src/routes/checkout/+page.svelteline 117. Any product with a non-round-dollar price (e.g., $99.50 = 9950 cents) will display the wrong amount. For a payment page, displaying incorrect prices is a blocker.NITS
Keyboard accessibility on card body click area: Add
role="button" tabindex="0"and anonkeydownhandler (Enter/Space) to.checkout-card-body, or remove the div click and rely solely on the button.Error
role="alert": Addrole="alert"to the.checkout-errordiv so screen readers announce validation errors.DRY the submit button: The identical submit button block appears twice. Factor it out.
Focus visibility: Replace
outline: nonewith a visible focus indicator that meets 3:1 contrast. Consideroutline: 2px solid #d42026; outline-offset: 2px;instead.Unused
.btn-secondaryCSS rule: Dead code in the checkout page styles.encodeURIComponenton cancel page retry link: The token/category values in thehrefon the cancel page should be encoded for defense-in-depth.SOP COMPLIANCE
55-feat-generic-checkout-page-dynamic-produreferences issue #55)plan-wkq)npm run buildpassing and manual testing. For a payment checkout page, this is a significant gap, though it is a repo-wide gap, not unique to this PR.PROCESS OBSERVATIONS
formatPricebug means prices could display incorrectly, which directly impacts trust and potentially legal compliance for consumer-facing payment pages. Thecategoryinjection is lower risk since the backend likely validates, but defense-in-depth is standard for payment flows.VERDICT: NOT APPROVED
Two blockers must be fixed:
categorybefore API URL interpolationformatPriceto handle fractional dollar amounts correctlyPR #56 Review
DOMAIN REVIEW
Tech stack: SvelteKit 5 (Svelte 5 runes), vanilla CSS, Keycloak auth, Stripe checkout integration.
Previous blockers resolved -- verification:
Unvalidated category parameter -- FIXED. Line 40:
encodeURIComponent(category || 'jersey')properly sanitizes the query parameter before URL interpolation. Prevents injection of arbitrary query strings or URL manipulation.formatPrice truncation bug -- FIXED. Lines 117-121: Division-based approach (
cents / 100) with modulo check correctly handles all cases:9950->99.5->$99.50(fractional cents:.toFixed(2))9000->90->$90(whole dollar: no trailing zeros)0->FreeCheckout page (
+page.svelte):submittingstate prevents double-submit -- good.create-sessionpasses token as query param -- consistent with the API design.Cancel page (
cancel/+page.svelte):tokenandcategorydirectly into the URL without encoding. Sincetokencomes from the API (UUID/JWT format) andcategorywas previously stored from a URL param, this is low risk but inconsistent with theencodeURIComponentfix on the main page. Not a blocker since these values originate from controlled sources.Success page (
success/+page.svelte):Accessibility fixes (register, billing, player detail pages):
for/idlabel-input pairing added across 15+ form fields.<label>changed to<span>for non-input elements (Stripe mock fields, radio groups) -- correct semantic HTML.role="radiogroup"andaria-labelledbyadded to radio button groups -- good ARIA patterns.CI changes (
.woodpecker.yaml):event: pull_requesttrigger -- enables CI on PRs, good practice.git clone --branchtogit init + git fetch SHA-- fixes PR checkout whereCI_COMMIT_BRANCHmay not exist on the remote. This matches the pattern from pal-e-platform PR #118.BLOCKERS
None. Both previously identified blockers are resolved.
Regarding test coverage: this repo has zero test infrastructure (no vitest, no test runner, no test files). The
package.jsonhas no test script. This is a pre-existing repo-wide gap, not introduced by this PR. The PR's Test Plan appropriately identifies manual testing steps. Blocking a checkout page PR for the absence of a test framework the project has never had would be scope creep -- but this gap should be tracked as a separate issue.NITS
Cancel page token encoding: The retry link at
cancel/+page.svelte:24does notencodeURIComponentthetokenorcategoryvalues in the href. Low risk since both come from controlled sources, but worth making consistent with the main page's approach.Hardcoded logo URL:
checkout/+page.svelte:143hardcodes the MinIO logo URL. Other pages likely do the same, so this is a pre-existing pattern, but a shared constant or config would be cleaner..btn-secondarydefined but unused: Line 435-438 in the checkout page style block defines.btn-secondarybut it is not used in the template. Dead CSS.checkout-card-featuredclass: Applied conditionally at line 157 but has no corresponding CSS rule in the style block. The class is set but has no visual effect.SOP COMPLIANCE
55-feat-generic-checkout-page-dynamic-produ-> issue #55)plan-wkq)PROCESS OBSERVATIONS
.woodpecker.yamlchange to trigger onpull_requestevents is a solid improvement for deployment frequency -- PRs now get validated before merge.VERDICT: APPROVED