fix: replace hardcoded practice schedules with DB query

forgejo_admin commented

2026-04-12 19:39:48 +00:00

Contributor

Summary

Replaces three hardcoded practice schedule blocks (boys travel, girls travel, local) with a single dynamic {#each} loop that fetches from the practice_schedules table. This fixes the 16U Local schedule showing Friday instead of Tuesday/Wednesday, and means schedule changes no longer require code deploys.

Changes

src/routes/contract/[token]/+page.server.ts: Added second SQL query joining practice_schedules via player_teams to fetch active schedules for the player's team. Added formatTime and formatTimeRange helpers to convert 24h times ("18:00") to friendly format ("6 – 8 PM"). Returns practices[] in page data.
src/routes/contract/[token]/+page.svelte: Replaced all three hardcoded practice schedule blocks (~40 lines of static HTML with team-name conditionals) with a single shared {#each practices} loop. Falls back to "Practice schedule will be communicated by your coach." when no rows exist.
src/lib/types.ts: Added PracticeSchedule interface for the return type.

Test Plan

Verify build passes: npm run build clean
Verify existing tests pass: 14/14 passing
Load a contract page for a player whose team has practice_schedules rows — should show dynamic schedule
Load a contract page for a player whose team has no rows — should show fallback message
Verify time formatting: "18:00"/"20:00" renders as "6 – 8 PM"

Review Checklist

Build passes (npm run build)
Tests pass (14/14)
Existing .schedule-row, .schedule-day, .schedule-detail CSS classes preserved
No hardcoded schedule data remains
Graceful fallback when no practice_schedules rows exist
Time formatting handles same-period ranges (omits redundant AM/PM)

Closes #49

Forgejo issue: #49

## Summary Replaces three hardcoded practice schedule blocks (boys travel, girls travel, local) with a single dynamic `{#each}` loop that fetches from the `practice_schedules` table. This fixes the 16U Local schedule showing Friday instead of Tuesday/Wednesday, and means schedule changes no longer require code deploys. ## Changes - **`src/routes/contract/[token]/+page.server.ts`**: Added second SQL query joining `practice_schedules` via `player_teams` to fetch active schedules for the player's team. Added `formatTime` and `formatTimeRange` helpers to convert 24h times ("18:00") to friendly format ("6 – 8 PM"). Returns `practices[]` in page data. - **`src/routes/contract/[token]/+page.svelte`**: Replaced all three hardcoded practice schedule blocks (~40 lines of static HTML with team-name conditionals) with a single shared `{#each practices}` loop. Falls back to "Practice schedule will be communicated by your coach." when no rows exist. - **`src/lib/types.ts`**: Added `PracticeSchedule` interface for the return type. ## Test Plan - Verify build passes: `npm run build` clean - Verify existing tests pass: 14/14 passing - Load a contract page for a player whose team has `practice_schedules` rows — should show dynamic schedule - Load a contract page for a player whose team has no rows — should show fallback message - Verify time formatting: "18:00"/"20:00" renders as "6 – 8 PM" ## Review Checklist - [x] Build passes (`npm run build`) - [x] Tests pass (14/14) - [x] Existing `.schedule-row`, `.schedule-day`, `.schedule-detail` CSS classes preserved - [x] No hardcoded schedule data remains - [x] Graceful fallback when no practice_schedules rows exist - [x] Time formatting handles same-period ranges (omits redundant AM/PM) ## Related Notes - Closes #49 ## Related - Forgejo issue: #49

forgejo_admin added 4 commits

2026-04-12 19:39:48 +00:00

feat: Queens pink branding + Nike EYBL update 67f63ea3fc

- Add queens-active CSS class that swaps --color-red to #e91e8c (pink)
- Apply queens-active on html element when isGirls is true
- Rename Nike Tournament to Nike EYBL Tournament
- Update Live Periods section: "best tournament in the country"

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

fix: move queens-active CSS outside :root block 187e577e79

Was nested inside :root causing :root :root.queens-active selector
which never matches. Now at top level as :root.queens-active.

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

feat: add age-based team reassignment clause to Queens contracts 95f10d3cb0

Queens rosters span graduating classes 2027-2031. This clause gives the
organization discretion to move younger players to a more age-appropriate
team, communicated to the family before taking effect. Only appears on
Queens contracts (inside the isGirls conditional block).

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

fix: replace hardcoded practice schedules with dynamic DB query 13fd5318e1

Fetches practice_schedules from the database for the player's team
instead of using three separate hardcoded blocks. Maps day_of_week
integers to day names and formats time ranges nicely.

Closes #49

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

forgejo_admin commented

2026-04-12 19:40:48 +00:00

Author

Contributor

QA Review — PR #50

Scope Check

The PR targets issue #49 (dynamic practice schedules). The branch includes a second commit (95f10d3 feat: add age-based team reassignment clause) that is out of scope for #49 — it adds Queens color CSS, Nike EYBL rename, and an age-based reassignment paragraph. This is pre-existing on the branch and not part of the practice schedule work, but it ships with this PR. Consider rebasing to isolate, or accept as-is if that content was already approved.

Code Review

src/routes/contract/[token]/+page.server.ts

SQL query joins practice_schedules via player_teams and players correctly. Filters is_active = true. Orders by day_of_week, start_time. Clean.
formatTime handles 12h conversion correctly (midnight=12 AM, noon=12 PM, hours >12 subtract). Minutes preserved when non-zero.
formatTimeRange omits redundant AM/PM when both times share the same period. Good UX.
Minor: minute variable in formatTime is computed but never used (line: const minute = parseInt(minuteStr || '0')). It's only used implicitly via the minute === 0 check further down — but wait, that check is actually if (minute === 0) which does use it. Correct.

src/routes/contract/[token]/+page.svelte

All three hardcoded practice blocks (boys travel, girls travel, local) replaced with identical {#each practices} loops. DRY improvement.
Fallback message "Practice schedule will be communicated by your coach." renders when no rows. Correct.
notes field renders inside schedule-optional span with parentheses. Matches the existing pattern for optional items.
Existing CSS classes .schedule-row, .schedule-day, .schedule-detail preserved. No visual regression expected.

src/lib/types.ts

PracticeSchedule interface added with all needed fields. Clean.

Potential Issues

No tenant_id filter on the query. The practice_schedules table has a tenant_id column per the issue schema. If multiple tenants share the database, this could return schedules from the wrong tenant. Currently likely single-tenant so low risk, but worth noting.
The second query could fail silently if practice_schedules table doesn't exist yet. If this PR deploys before the table is seeded, the query will throw a Postgres error and the contract page will 500. The table already exists per the issue description, so this is fine for now.

Nits

The \u2013 (en-dash) in formatTimeRange is a Unicode literal. Works correctly but the hardcoded HTML used –. No functional difference since this renders as text interpolation, not raw HTML.

VERDICT: APPROVED

Core practice schedule changes are correct, well-structured, and the build passes. The out-of-scope Queens commit is a minor branch hygiene issue, not a blocker.

## QA Review — PR #50 ### Scope Check The PR targets issue #49 (dynamic practice schedules). The branch includes a second commit (`95f10d3 feat: add age-based team reassignment clause`) that is **out of scope** for #49 — it adds Queens color CSS, Nike EYBL rename, and an age-based reassignment paragraph. This is pre-existing on the branch and not part of the practice schedule work, but it ships with this PR. Consider rebasing to isolate, or accept as-is if that content was already approved. ### Code Review **`src/routes/contract/[token]/+page.server.ts`** - SQL query joins `practice_schedules` via `player_teams` and `players` correctly. Filters `is_active = true`. Orders by `day_of_week, start_time`. Clean. - `formatTime` handles 12h conversion correctly (midnight=12 AM, noon=12 PM, hours >12 subtract). Minutes preserved when non-zero. - `formatTimeRange` omits redundant AM/PM when both times share the same period. Good UX. - Minor: `minute` variable in `formatTime` is computed but never used (line: `const minute = parseInt(minuteStr || '0')`). It's only used implicitly via the `minute === 0` check further down — but wait, that check is actually `if (minute === 0)` which does use it. Correct. **`src/routes/contract/[token]/+page.svelte`** - All three hardcoded practice blocks (boys travel, girls travel, local) replaced with identical `{#each practices}` loops. DRY improvement. - Fallback message "Practice schedule will be communicated by your coach." renders when no rows. Correct. - `notes` field renders inside `schedule-optional` span with parentheses. Matches the existing pattern for optional items. - Existing CSS classes `.schedule-row`, `.schedule-day`, `.schedule-detail` preserved. No visual regression expected. **`src/lib/types.ts`** - `PracticeSchedule` interface added with all needed fields. Clean. ### Potential Issues 1. **No tenant_id filter on the query.** The `practice_schedules` table has a `tenant_id` column per the issue schema. If multiple tenants share the database, this could return schedules from the wrong tenant. Currently likely single-tenant so low risk, but worth noting. 2. **The second query could fail silently if `practice_schedules` table doesn't exist yet.** If this PR deploys before the table is seeded, the query will throw a Postgres error and the contract page will 500. The table already exists per the issue description, so this is fine for now. ### Nits - The `\u2013` (en-dash) in `formatTimeRange` is a Unicode literal. Works correctly but the hardcoded HTML used `–`. No functional difference since this renders as text interpolation, not raw HTML. **VERDICT: APPROVED** Core practice schedule changes are correct, well-structured, and the build passes. The out-of-scope Queens commit is a minor branch hygiene issue, not a blocker.

forgejo_admin force-pushed 49-dynamic-practice-schedules from 13fd5318e1 to db4b24ae8f

2026-04-12 19:56:27 +00:00

Compare