ldraney/westside-app
1
0
Fork 0
Code Issues 17 Pull requests Projects Releases Packages Wiki Activity Actions

feat: coach dashboard schedule tab with live data #217

Merged
forgejo_admin merged 1 commit from 213-coach-dashboard-schedule into main 2026-04-04 22:34:35 +00:00
Conversation 1 Commits 1 Files changed 1 +260 -5
forgejo_admin commented 2026-04-04 22:00:37 +00:00
Contributor
Copy link

Summary

Replaces the "Coming soon" placeholder on the coach dashboard Schedule tab with a functional schedule display. Fetches /public/schedule and filters client-side by the coach's team IDs from /coaches/me. Pure frontend -- no backend changes.

Changes

  • src/routes/(app)/coach/+page.svelte -- Added publicFetch import, schedule state variables, helper functions (formatTime, dayName, formatDate, formatDateRange, groupByTeam, sortEventsByDate), schedule data fetching inside existing onMount, and replaced the placeholder markup with practice cards grouped by team name and event cards sorted by date. Added scoped CSS for schedule card layout matching admin schedule visual patterns.

Test Plan

  • svelte-check passes with 0 errors (7 pre-existing warnings)
  • Log in as Marcus (coaches 2 teams: 17U Elite Kings, 17U Local Kings) and verify Schedule tab shows practices for both teams grouped by name
  • Verify practice cards show day name, 12-hour time (AM/PM), and venue address
  • Verify events/tournaments section shows filtered events sorted by date
  • Verify coaches with no teams see "No practices or events found" empty state
  • Verify Team and Plays tabs are unaffected

Review Checklist

  • svelte-check passes with 0 errors
  • No backend changes required
  • Pure CSS vars, no Tailwind
  • Multi-team coach support (Marcus with 2 teams)
  • Scoped styles follow existing admin schedule card patterns

Related Notes

  • Story: story:WS-S27
  • API: /coaches/me + /public/schedule (no changes)

Related

Closes #213

## Summary Replaces the "Coming soon" placeholder on the coach dashboard Schedule tab with a functional schedule display. Fetches `/public/schedule` and filters client-side by the coach's team IDs from `/coaches/me`. Pure frontend -- no backend changes. ## Changes - `src/routes/(app)/coach/+page.svelte` -- Added `publicFetch` import, schedule state variables, helper functions (formatTime, dayName, formatDate, formatDateRange, groupByTeam, sortEventsByDate), schedule data fetching inside existing onMount, and replaced the placeholder markup with practice cards grouped by team name and event cards sorted by date. Added scoped CSS for schedule card layout matching admin schedule visual patterns. ## Test Plan - `svelte-check` passes with 0 errors (7 pre-existing warnings) - Log in as Marcus (coaches 2 teams: 17U Elite Kings, 17U Local Kings) and verify Schedule tab shows practices for both teams grouped by name - Verify practice cards show day name, 12-hour time (AM/PM), and venue address - Verify events/tournaments section shows filtered events sorted by date - Verify coaches with no teams see "No practices or events found" empty state - Verify Team and Plays tabs are unaffected ## Review Checklist - [x] `svelte-check` passes with 0 errors - [x] No backend changes required - [x] Pure CSS vars, no Tailwind - [x] Multi-team coach support (Marcus with 2 teams) - [x] Scoped styles follow existing admin schedule card patterns ## Related Notes - Story: `story:WS-S27` - API: `/coaches/me` + `/public/schedule` (no changes) ## Related Closes #213
feat: replace coach Schedule tab placeholder with live practice and event display
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
5541981a03 
Fetches /public/schedule and filters by coach's team IDs from /coaches/me.
Practices grouped by team name, events sorted by date. Supports multi-team
coaches (e.g., Marcus with 2 teams). Pure CSS vars, no Tailwind.

Closes #213

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-04-04 22:04:41 +00:00
Author
Contributor
Copy link

PR #217 Review — QA Agent

DOMAIN REVIEW

Tech stack: SvelteKit 5 (runes mode with $state, $derived), vanilla CSS, Capacitor mobile wrapper.

SvelteKit patterns: Proper use of Svelte 5 runes ($state, $derived), onMount for data fetching, scoped <style> blocks. Tab navigation correctly uses role="tablist" / role="tab" / aria-selected for accessibility.

Schedule filtering logic: The coach dashboard fetches /coaches/me for team IDs, then /public/schedule for all practices/events, filtering client-side by team_id. Events use a two-tier filter: direct team_id match first, then division fallback for unscoped events, with deduplication by event ID. Correct and handles edge cases well.

Reactivity: practicesByTeam and sortedEvents use $derived correctly.

Error handling: Catch block gracefully degrades showing coach name and error message. scheduleLoading properly set in finally block.

BLOCKERS

None hard.

1. DRY violation — duplicated utility functions (strong nit)

Six functions copy-pasted between src/routes/(public)/schedule/+page.svelte and src/routes/(app)/coach/+page.svelte:

  • DAY_NAMES constant
  • formatTime()
  • formatDate()
  • formatDateRange()
  • dayName()
  • groupByTeam() (near-identical — coach version adds _team_name field lookup)

Recommend extracting to a shared $lib/schedule-utils.js module. Not a hard blocker (not auth/security path) but a maintenance risk — any bug fix must be applied in two places.

2. No test coverage (pre-existing)

The entire project has zero test infrastructure (no vitest/jest/playwright in package.json). This is a pre-existing condition, not introduced by this PR.

NITS

  1. Inconsistent $derived style (line 166): filteredPlayers uses $derived(() => {...}) returning a closure, while practicesByTeam and sortedEvents use $derived(expression). Pick one pattern.

  2. Loose typing: Heavy use of any types throughout. Consider defining interfaces for Practice, Event, and Coach objects.

  3. Missing tabpanel aria-labelledby: The role="tabpanel" divs lack aria-labelledby attributes linking to their tab buttons. Minor a11y gap.

  4. CSS breakpoint: @media (min-width: 640px) doesn't match standard project breakpoints (375/768/1280). Reasonable but worth noting.

  5. parent_phone in coach view: Showing parent phone numbers directly — consider whether this PII needs access control or visual de-emphasis.

SOP COMPLIANCE

  • ✅ Branch named after issue: 213-coach-dashboard-schedule
  • ✅ PR body follows template (Summary, Changes, Test Plan, Related)
  • ✅ No secrets committed — API calls use keycloak token auth properly
  • ✅ No unnecessary file changes — single file, tightly scoped
  • ⚠️ No plan slug referenced (not required for all PRs)

VERDICT: APPROVED

Functionally correct, proper error handling, appropriate SvelteKit 5 patterns. DRY duplication is a real concern but doesn't meet strict blocker criteria. Lack of tests is a project-wide gap, not introduced here. Recommend utility extraction as a fast follow-up.

## PR #217 Review — QA Agent ### DOMAIN REVIEW **Tech stack**: SvelteKit 5 (runes mode with `$state`, `$derived`), vanilla CSS, Capacitor mobile wrapper. **SvelteKit patterns**: Proper use of Svelte 5 runes (`$state`, `$derived`), `onMount` for data fetching, scoped `<style>` blocks. Tab navigation correctly uses `role="tablist"` / `role="tab"` / `aria-selected` for accessibility. **Schedule filtering logic**: The coach dashboard fetches `/coaches/me` for team IDs, then `/public/schedule` for all practices/events, filtering client-side by `team_id`. Events use a two-tier filter: direct `team_id` match first, then division fallback for unscoped events, with deduplication by event ID. Correct and handles edge cases well. **Reactivity**: `practicesByTeam` and `sortedEvents` use `$derived` correctly. **Error handling**: Catch block gracefully degrades showing coach name and error message. `scheduleLoading` properly set in `finally` block. ### BLOCKERS None hard. **1. DRY violation — duplicated utility functions (strong nit)** Six functions copy-pasted between `src/routes/(public)/schedule/+page.svelte` and `src/routes/(app)/coach/+page.svelte`: - `DAY_NAMES` constant - `formatTime()` - `formatDate()` - `formatDateRange()` - `dayName()` - `groupByTeam()` (near-identical — coach version adds `_team_name` field lookup) Recommend extracting to a shared `$lib/schedule-utils.js` module. Not a hard blocker (not auth/security path) but a maintenance risk — any bug fix must be applied in two places. **2. No test coverage (pre-existing)** The entire project has zero test infrastructure (no vitest/jest/playwright in `package.json`). This is a pre-existing condition, not introduced by this PR. ### NITS 1. **Inconsistent `$derived` style** (line 166): `filteredPlayers` uses `$derived(() => {...})` returning a closure, while `practicesByTeam` and `sortedEvents` use `$derived(expression)`. Pick one pattern. 2. **Loose typing**: Heavy use of `any` types throughout. Consider defining interfaces for Practice, Event, and Coach objects. 3. **Missing `tabpanel` aria-labelledby**: The `role="tabpanel"` divs lack `aria-labelledby` attributes linking to their tab buttons. Minor a11y gap. 4. **CSS breakpoint**: `@media (min-width: 640px)` doesn't match standard project breakpoints (375/768/1280). Reasonable but worth noting. 5. **`parent_phone` in coach view**: Showing parent phone numbers directly — consider whether this PII needs access control or visual de-emphasis. ### SOP COMPLIANCE - ✅ Branch named after issue: `213-coach-dashboard-schedule` - ✅ PR body follows template (Summary, Changes, Test Plan, Related) - ✅ No secrets committed — API calls use keycloak token auth properly - ✅ No unnecessary file changes — single file, tightly scoped - ⚠️ No plan slug referenced (not required for all PRs) ### VERDICT: APPROVED Functionally correct, proper error handling, appropriate SvelteKit 5 patterns. DRY duplication is a real concern but doesn't meet strict blocker criteria. Lack of tests is a project-wide gap, not introduced here. Recommend utility extraction as a fast follow-up.
forgejo_admin deleted branch 213-coach-dashboard-schedule 2026-04-04 22:34:35 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend
No labels
domain:backend
domain:devops
domain:frontend
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/westside-app!217
No description provided.