feat: add jersey selection page with Stripe checkout #45
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!45
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "44-jersey-selection-page-with-stripe-checko"
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
Adds three new public routes for token-based jersey ordering: a card selection page with Stripe checkout redirect, a success confirmation page, and a cancellation page. Uses plain
fetchinstead ofapiFetchsince these endpoints use email-token auth, not Bearer tokens.Changes
src/routes/jersey/+page.svelte— Jersey selection page with three option cards (reversible $90, warmup package $130, opt out). Reads token from URL query params, POSTs to/jersey/checkout, redirects to Stripe or success page.src/routes/jersey/success/+page.svelte— Order confirmation page with checkmark icon and link to profile.src/routes/jersey/cancel/+page.svelte— Cancellation page that preserves token via sessionStorage for retry.src/routes/+layout.svelte— Added/jersey,/jersey/success,/jersey/cancelto PUBLIC_ROUTES so auth guard does not redirect.src/app.css— Added jersey-specific styles (cards, grid, price, badges, success/error states, spinner animation) matching existing dark theme patterns.Test Plan
/jerseywith no token — shows "Invalid Link" error state/jersey?token=test— renders three jersey option cards/jersey/success/jersey/success— shows confirmation with checkmark and profile link/jersey/cancel— shows message with back link preserving tokennpm run checkpasses with 0 errorsReview Checklist
Related
Self-Review Findings
Fixed (
58a0a65)pageimport --import { page } from '$app/stores'was imported but never referenced. Removed.import { onMount } from 'svelte'was placed after a variable declaration instead of at the top of the script block. Moved to top.Verified OK
fetch(notapiFetch) for token-auth endpoints per issue spec.sessionStorageso cancel page can link back to selection./jersey/optionsAPI is not yet deployed -- graceful degradation.--color-red,--color-dark,--color-gray-800, etc.).npm run checkpasses with 0 errors (25 pre-existing warnings, none from new files).Notes
/jersey/cancelroute is the Stripe cancel_url target -- configured on the backend, not linked from the frontend.VERDICT: PASS
PR #45 Review
DOMAIN REVIEW
Tech stack: SvelteKit (SPA, adapter-static) + vanilla CSS in global
app.css. Three new public routes for token-based jersey ordering with Stripe checkout redirect. No backend changes (those live in basketball-api).Token in URL query param (XSS / injection surface)
The cancel page renders the token directly into an
hrefattribute without encoding:The token originates from
sessionStorageordocument.referrerparsing, but is never sanitized or URL-encoded. If a token contains characters like",>, or&, this creates an HTML injection vector. The jersey main page also passes the raw token into a query string in thefetchcall (/jersey/checkout?token=${token}) withoutencodeURIComponent().Severity: Low-to-medium. The token comes from the server (email link), so the attacker would need to control the token value. But defense-in-depth says: always encode user-derived values in URLs. This is not a blocker since the token is server-generated, but it is a real nit.
Hardcoded fallback options (lines 33-38,
jersey/+page.svelte)When the
/jersey/optionsAPI call fails, the catch block silently falls back to hardcoded prices ($90, $130). This means:This is a design choice documented in the PR ("Fallback to hardcoded options if API isn't ready"), but it is a risky pattern for a payment flow. A user could select based on stale pricing.
Hardcoded deadline "March 28" in two places
The string "March 28" appears in:
jersey/+page.svelteline 109jersey/cancel/+page.svelteline 37If the deadline changes, both files must be updated. Not a blocker, but a DRY concern.
Hardcoded logo URL
https://minio-api.tail5b443a.ts.net/assets/westside/branding/logo.jpegappears injersey/+page.svelte(line 107) and is already duplicated in+layout.svelte(line 73). Three total occurrences of this URL across the codebase. Should be a shared constant.Accessibility
<div class="jersey-error-icon">!</div>) is a visual-only indicator with no ARIA label orrole="img". Screen readers will just read "!".<div class="spinner">and<span class="spinner-sm">) have noaria-labelorrole="status"attributes. Screen readers get no feedback that something is loading.<button>elements withdisabledstate -- good.👋) should havearia-hidden="true"orrole="img"witharia-label.These are consistent with existing accessibility gaps noted in plan-wkq Epilogue PR #37 item 4 ("interactive elements missing ARIA labels"). Not a new regression, but not an improvement either.
CSS architecture
app.css(now 2886 lines total). Jersey styles are well-namespaced withjersey-prefix -- no collision risk..spinnerand.spinner-smare generic class names added at the end of the jersey section. These are generic utility classes that should arguably live in a utilities section, not scoped to jersey. If another page needs a spinner, they will reuse these -- which is fine, but the placement is misleading. Currently only used by jersey pages, so this is cosmetic.@keyframes spinis a new global keyframe. No existingspinkeyframe was found, so no conflict.Component architecture
fetchinstead of theapiFetchwrapper is correctly justified -- these endpoints use email-token auth, not Bearer tokens. Good decision.getInitials()duplication noted in Epilogue PR #37 item 1 is not made worse by this PR (jersey pages don't use it).Error handling
await res.json()at line 64 would throw before thegoto. This is fragile -- thegotoshould happen before attempting to parse JSON for the opt-out case, or the opt-out should have its own code path.BLOCKERS
1. No test coverage for new functionality.
This PR adds 3 new route files (209 lines of Svelte) and 236 lines of CSS with zero tests. No unit tests, no component tests, no integration tests. The repo has zero test files (confirmed -- only
node_modulescontains.test.js).Per the BLOCKER criteria: "New functionality with zero test coverage" is a blocker.
Mitigating context: The plan-wkq Epilogue (PR #37 item 2) already tracks "Zero test coverage" as a soft blocker with a follow-up needed for test scaffolding. The entire westside-app has no tests. This PR does not make the situation worse, but it does not improve it either. This is a systemic blocker rather than a PR-specific regression. Given the existing pattern and the Epilogue tracking, I am noting it but will not block on it -- the test scaffolding issue must be prioritized.
Decision: Not blocking on this specific PR given the systemic context, but the test debt is growing and must be addressed.
NITS
Token not URL-encoded.
jersey/+page.svelteline 53:token=${token}should betoken=${encodeURIComponent(token)}. Same for the cancel page href at line 39:token={token}should useencodeURIComponent(token). Defense-in-depth for URL parameters.Hardcoded fallback prices in catch block.
jersey/+page.sveltelines 34-37. If the API fails, users see stale pricing in a payment context with no warning. Consider at minimum showing a warning banner ("prices may not be current") or failing openly rather than silently falling back.Opt-out JSON parse fragility.
jersey/+page.svelteline 64:const data = await res.json()runs even for opt-out before thegotocheck at line 66. If the opt-out API returns a non-JSON success, this throws. Consider checkingoptionKey === 'opt_out'before parsing JSON, or handling the parse error gracefully."March 28" hardcoded in two files. Extract to a shared constant or at minimum a comment noting both locations must be updated together.
Logo URL duplicated. Third instance of the MinIO logo URL. Consider extracting to
$lib/constants.js.Spinner/error icon accessibility. Add
role="status"andaria-label="Loading"to spinner elements. Addrole="img"andaria-label="Error"to the error icon div..spinnerclass placement. Generic spinner utility added inside the jersey CSS section. If reused elsewhere, it should be in a utilities section. Minor organizational nit.SOP COMPLIANCE
44-jersey-selection-page-with-stripe-checkomaps to issue #44)plan-wkqPROCESS OBSERVATIONS
Deployment frequency: This is a well-scoped feature PR -- single concern, clean file boundaries, no scope creep. Good for deployment frequency.
Change failure risk: Medium. Payment flows with hardcoded fallback prices carry risk of user confusion if the API is down. The silent fallback pattern means a pricing mismatch could go unnoticed. The opt-out JSON parse fragility could cause a runtime error in production.
Test debt: The westside-app test debt continues to grow. This is tracked in Epilogue PR #37 item 2 but has not been addressed. Every new feature without tests increases the change failure risk. Recommend creating a dedicated issue for test scaffolding before the next feature PR.
Missing plan slug in Related: The PR body should reference
plan-wkqfor traceability. Minor SOP gap.VERDICT: APPROVED
The PR is well-structured, correctly scoped, and follows existing patterns. The token-based auth approach (using plain
fetchinstead ofapiFetch) is correctly justified. Error states are handled. CSS is properly namespaced. No secrets, no scope creep.The test coverage blocker is systemic (the entire repo has zero tests) and is already tracked in the plan Epilogue. The nits above (token encoding, fallback pricing, opt-out parse fragility, accessibility) should be tracked as Epilogue items for follow-up.