feat: port 3000, Stripe redirect, tryout date fix

forgejo_admin commented

2026-03-19 01:26:11 +00:00

Author

Owner

PR #52 Review

PR: feat: port 3000, Stripe redirect, tryout date fix
Branch: 51-feat-port-fix-stripe-redirect-handler-tr (from issue #51)
Files changed: 4 (+10 / -4)

DOMAIN REVIEW

Tech stack: SvelteKit (static build) + nginx + Docker, deployed to k8s. Frontend consumes a FastAPI backend (basketball-api).

1. Port change (Dockerfile + nginx.conf) -- CORRECT

The EXPOSE 80 to EXPOSE 3000 and listen 80 to listen 3000 changes align with the existing k8s manifests:

k8s/deployment.yaml line 23: containerPort: 3000
k8s/deployment.yaml lines 69-70: port: 3000, targetPort: 3000
k8s/ingress.yaml line 13: number: 3000

This was a mismatch that would have caused the container to serve on port 80 while k8s probed port 3000 -- meaning the app was unreachable. This fix is correct and necessary.

2. Photo upload endpoint (/upload/photo to /api/register/upload-photo) -- CORRECT

The old path /upload/photo does not match any route in the basketball-api. The new path /api/register/upload-photo follows the API's route prefix convention. The apiUpload function in $lib/api.js prepends API_BASE (https://basketball-api.tail5b443a.ts.net), so the full URL becomes https://basketball-api.tail5b443a.ts.net/api/register/upload-photo -- consistent with the API's /api/register namespace.

3. Stripe redirect handler -- CORRECT, NO OPEN REDIRECT RISK

The new code at lines 145-149 of register/+page.svelte:

if (result?.redirect_url) {
    window.location.href = result.redirect_url;
    return;
}

I verified the backend source (basketball-api/src/basketball_api/routes/register.py line 1279):

if body.payment_method == "card":
    return {"redirect_url": settings.stripe_tryout_link}

The redirect_url value is server-controlled -- it comes from settings.stripe_tryout_link, which resolves to a hardcoded Stripe buy.stripe.com URL. The backend test (test_promo_registration.py line 157) even asserts "stripe.com" in body["redirect_url"]. There is no user-supplied input flowing into this URL. No open redirect risk.

This also follows an established codebase pattern -- src/routes/jersey/+page.svelte line 69 does the identical thing: window.location.href = data.checkout_url.

4. Tryout date "This Tuesday" -- FUNCTIONALLY CORRECT BUT FRAGILE

The change from "This Saturday" to "This Tuesday" fixes the immediate inaccuracy (March 24 is a Tuesday). However, this is a hardcoded relative day reference that will go stale after March 24.

BLOCKERS

None. All four changes are correct, security review passes, and the PR scope is tight.

Regarding test coverage: this repo has zero project-level tests (no *.test.* or *.spec.* files outside node_modules). The changes here are:

Infrastructure config (Dockerfile, nginx.conf) -- tested by deployment
A string literal change (day of week)
A URL path fix (endpoint alignment)
A 5-line redirect handler following an existing codebase pattern

Given that (a) no test infrastructure exists in this repo, (b) the Stripe redirect is tested on the backend side (test_promo_registration.py::TestCardRegistration::test_card_payment_returns_redirect_url), and (c) the changes are minimal config/wiring fixes, I am not blocking on missing frontend tests for this PR.

NITS

"This Tuesday" will go stale. After March 24, the banner will be misleading. Consider either:
- Using the actual date: "March 24" instead of a relative day reference
- Making this data-driven (fetch next tryout date from the API)
This is not blocking since it is a pre-existing pattern and the PR is fixing an already-stale value, but it will need attention again soon.
No loading/disabled state during Stripe redirect. After window.location.href = result.redirect_url, the finally block sets submitting = false, which briefly re-enables the submit button before the browser navigates away. This is cosmetic -- the return prevents further execution and the navigation is near-instant -- but a submitting = true hold until navigation completes would be slightly cleaner. Non-blocking.

SOP COMPLIANCE

Branch named after issue (51-feat-port-fix-stripe-redirect-handler-tr references #51)
PR body has Summary, Changes, Test Plan, Related sections
Related references plan slug (plan-wkq)
No secrets committed (verified -- no API keys, tokens, or credentials in diff)
No unnecessary file changes (4 files, all directly related to the 3 stated fixes)
Commit messages are descriptive (single commit PR, title is clear)

PROCESS OBSERVATIONS

Deployment frequency: This unblocks the container from actually serving traffic on the correct port. High-impact fix that should deploy quickly.
Change failure risk: Low. Port alignment is verifiable by k8s health probes. Stripe redirect is server-controlled. Endpoint path fix aligns with API convention.
Documentation gap: The "This Tuesday" pattern will require another PR after March 24. Consider tracking this as discovered scope if not already planned.

VERDICT: APPROVED

## PR #52 Review **PR:** feat: port 3000, Stripe redirect, tryout date fix **Branch:** `51-feat-port-fix-stripe-redirect-handler-tr` (from issue #51) **Files changed:** 4 (+10 / -4) --- ### DOMAIN REVIEW **Tech stack:** SvelteKit (static build) + nginx + Docker, deployed to k8s. Frontend consumes a FastAPI backend (`basketball-api`). **1. Port change (Dockerfile + nginx.conf) -- CORRECT** The `EXPOSE 80` to `EXPOSE 3000` and `listen 80` to `listen 3000` changes align with the existing k8s manifests: - `k8s/deployment.yaml` line 23: `containerPort: 3000` - `k8s/deployment.yaml` lines 69-70: `port: 3000`, `targetPort: 3000` - `k8s/ingress.yaml` line 13: `number: 3000` This was a mismatch that would have caused the container to serve on port 80 while k8s probed port 3000 -- meaning the app was unreachable. This fix is correct and necessary. **2. Photo upload endpoint (`/upload/photo` to `/api/register/upload-photo`) -- CORRECT** The old path `/upload/photo` does not match any route in the basketball-api. The new path `/api/register/upload-photo` follows the API's route prefix convention. The `apiUpload` function in `$lib/api.js` prepends `API_BASE` (`https://basketball-api.tail5b443a.ts.net`), so the full URL becomes `https://basketball-api.tail5b443a.ts.net/api/register/upload-photo` -- consistent with the API's `/api/register` namespace. **3. Stripe redirect handler -- CORRECT, NO OPEN REDIRECT RISK** The new code at lines 145-149 of `register/+page.svelte`: ```js if (result?.redirect_url) { window.location.href = result.redirect_url; return; } ``` I verified the backend source (`basketball-api/src/basketball_api/routes/register.py` line 1279): ```python if body.payment_method == "card": return {"redirect_url": settings.stripe_tryout_link} ``` The `redirect_url` value is **server-controlled** -- it comes from `settings.stripe_tryout_link`, which resolves to a hardcoded Stripe `buy.stripe.com` URL. The backend test (`test_promo_registration.py` line 157) even asserts `"stripe.com" in body["redirect_url"]`. There is no user-supplied input flowing into this URL. No open redirect risk. This also follows an **established codebase pattern** -- `src/routes/jersey/+page.svelte` line 69 does the identical thing: `window.location.href = data.checkout_url`. **4. Tryout date "This Tuesday" -- FUNCTIONALLY CORRECT BUT FRAGILE** The change from "This Saturday" to "This Tuesday" fixes the immediate inaccuracy (March 24 is a Tuesday). However, this is a hardcoded relative day reference that will go stale after March 24. --- ### BLOCKERS None. All four changes are correct, security review passes, and the PR scope is tight. Regarding test coverage: this repo has **zero project-level tests** (no `*.test.*` or `*.spec.*` files outside `node_modules`). The changes here are: - Infrastructure config (Dockerfile, nginx.conf) -- tested by deployment - A string literal change (day of week) - A URL path fix (endpoint alignment) - A 5-line redirect handler following an existing codebase pattern Given that (a) no test infrastructure exists in this repo, (b) the Stripe redirect is tested on the backend side (`test_promo_registration.py::TestCardRegistration::test_card_payment_returns_redirect_url`), and (c) the changes are minimal config/wiring fixes, I am not blocking on missing frontend tests for this PR. --- ### NITS 1. **"This Tuesday" will go stale.** After March 24, the banner will be misleading. Consider either: - Using the actual date: "March 24" instead of a relative day reference - Making this data-driven (fetch next tryout date from the API) This is not blocking since it is a pre-existing pattern and the PR is fixing an already-stale value, but it will need attention again soon. 2. **No loading/disabled state during Stripe redirect.** After `window.location.href = result.redirect_url`, the `finally` block sets `submitting = false`, which briefly re-enables the submit button before the browser navigates away. This is cosmetic -- the `return` prevents further execution and the navigation is near-instant -- but a `submitting = true` hold until navigation completes would be slightly cleaner. Non-blocking. --- ### SOP COMPLIANCE - [x] Branch named after issue (`51-feat-port-fix-stripe-redirect-handler-tr` references #51) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references plan slug (`plan-wkq`) - [x] No secrets committed (verified -- no API keys, tokens, or credentials in diff) - [x] No unnecessary file changes (4 files, all directly related to the 3 stated fixes) - [x] Commit messages are descriptive (single commit PR, title is clear) --- ### PROCESS OBSERVATIONS - **Deployment frequency:** This unblocks the container from actually serving traffic on the correct port. High-impact fix that should deploy quickly. - **Change failure risk:** Low. Port alignment is verifiable by k8s health probes. Stripe redirect is server-controlled. Endpoint path fix aligns with API convention. - **Documentation gap:** The "This Tuesday" pattern will require another PR after March 24. Consider tracking this as discovered scope if not already planned. --- ### VERDICT: APPROVED

2026-03-19 01:23:53 +00:00

Summary

Container port changed from 80 to 3000 to match k8s service configuration
Tryout banner corrected: "This Saturday" → "This Tuesday" (March 24)
Photo upload endpoint fixed and card payment Stripe redirect handler added

Changes

Dockerfile: EXPOSE 80 → 3000
nginx.conf: listen 80 → 3000
src/routes/+page.svelte: Banner day text "This Saturday" → "This Tuesday"
src/routes/register/+page.svelte: Photo upload endpoint /upload/photo → /api/register/upload-photo, added Stripe redirect handler

Test Plan

Site loads on port 3000 in container
Home page shows "This Tuesday" in tryout banner
Photo upload works during registration
Card payment redirects to Stripe checkout

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #51
plan-wkq — Phase 1 (Commit Uncommitted Work)

## Summary - Container port changed from 80 to 3000 to match k8s service configuration - Tryout banner corrected: "This Saturday" → "This Tuesday" (March 24) - Photo upload endpoint fixed and card payment Stripe redirect handler added ## Changes - `Dockerfile`: EXPOSE 80 → 3000 - `nginx.conf`: listen 80 → 3000 - `src/routes/+page.svelte`: Banner day text "This Saturday" → "This Tuesday" - `src/routes/register/+page.svelte`: Photo upload endpoint `/upload/photo` → `/api/register/upload-photo`, added Stripe redirect handler ## Test Plan - [ ] Site loads on port 3000 in container - [ ] Home page shows "This Tuesday" in tryout banner - [ ] Photo upload works during registration - [ ] Card payment redirects to Stripe checkout ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Closes #51 - `plan-wkq` — Phase 1 (Commit Uncommitted Work)

forgejo_admin added 1 commit

feat: port 3000, Stripe redirect, tryout date fix (#51 ) 0ebe032b60

- Container port 80→3000 to match k8s service config
- Tryout banner: "This Saturday" → "This Tuesday"
- Photo upload endpoint fixed to /api/register/upload-photo
- Card payment redirects to Stripe checkout URL from API

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>