Add athletics modules#7573
Conversation
…oday/upcoming buckets
…ks across full date range
…r/minute selection has effect
…ting and API-driven TODAY sub-sections
- Fix filter seeding bypass: gate seed effect on Zustand persist hydration (_hasHydrated) so saved user filter preferences are not overwritten before AsyncStorage hydration completes - Parse dates once at query time: add ProcessedScore type (Score + parsedDate: Date) and a React Query select transformer in query.ts that pre-parses date_utc into a Date object; remove all per-render parseGameDate call sites in utils.ts and list.tsx - ccc-server companion change normalises date_utc/date_end_utc to ISO 8601 so the app can use new Date(score.date_utc) reliably on Hermes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…o.plist Agent-Logs-Url: https://github.com/StoDevX/AAO-React-Native/sessions/cbb20f00-608d-4b8d-91ce-5131a4a9b31f Co-authored-by: drewvolz <5240843+drewvolz@users.noreply.github.com>
hawkrives
left a comment
hawkrives
left a comment
There was a problem hiding this comment.
Automated code review of the Athletics module — high-effort pass focused on correctness, with some cleanup notes. Two of these break core functionality on a real device; the rest are medium UX/correctness issues and minor cleanups. Details are left as inline comments.
🔴 Critical
query.tsparses dates withnew Date(date_utc)instead of the Hermes-safeparseGameDate, so all date grouping silently fails on-device (tests pass only under Node).row.tsxnever displays a final score for finalized games (score panel is gated onindicator === 'O').
🟡 Medium: can't persist an empty filter; date_utc treated as local time; unused includePrefix prop; non-chronological Upcoming ordering; brittle length-only filter-hint check.
⚪ Cleanup: duplicated Upcoming grouping; dead types; unrelated .gitignore entry.
— Posted by Claude (automated review), on behalf of @hawkrives
| queryKey: keys.all, | ||
| queryFn: ({signal}): Promise<Score[]> => | ||
| client.get('athletics/scores', {signal}).json<Score[]>(), | ||
| select: (scores): ProcessedScore[] => |
There was a problem hiding this comment.
🔴 Critical: production date parsing fails on-device.
This select builds parsedDate with new Date(score.date_utc), but utils.ts ships a hand-written parseGameDate whose own doc-comment says Hermes (the RN engine) cannot parse the API's "M/D/YYYY h:mm:ss AM/PM" format via Date.parse.
So on a real device every parsedDate becomes Invalid Date: isSameDay and date > todayStart are all false in groupScoresByDate, and Yesterday/Today/Upcoming all render empty (headers read undefined, undefined NaN). The unit tests stay green only because they call parseGameDate directly under Node/V8 — the shipped path is never exercised.
Fix: select: (scores) => scores.map((s) => ({...s, parsedDate: parseGameDate(s.date_utc)})).
— Claude (automated review)
| </View> | ||
|
|
||
| <View style={styles.gameInfo}> | ||
| {item.status.indicator === 'A' ? ( |
There was a problem hiding this comment.
🟠 Finalized games never display their score.
The score panel below is gated on item.status.indicator === 'O' (ongoing) only. The Today FINALIZED bucket in list.tsx selects indicator !== 'O' && result !== '', so finalized games take the indicator === 'A' branch here and render item.time (or nothing) — the final team_score/opponent_score and the W/L result are never shown for a completed game.
Consider rendering the score panel whenever a score exists (e.g. result !== '' or both scores present), not just for live games.
— Claude (automated review)
|
|
||
| // Seed the filter store with all sports on first load — only after | ||
| // hydration is complete so we don't overwrite a previously saved filter. | ||
| React.useEffect(() => { |
There was a problem hiding this comment.
🟡 Deselecting all sports silently reverts to "show all".
This seeding effect re-seeds selectedSports with every sport whenever selectedSports.length === 0. So when a user clears all filters (via the section "All" toggle or by toggling each off), setSelectedSports([]) immediately triggers a re-seed on the next render and the UI snaps back to all-selected. There's no way to persist an empty selection, and the filter predicate already treats length === 0 as "show all" — so the clear-all affordance is effectively dead.
— Claude (automated review)
| if (ampm === 'PM') { | ||
| hour24 += 12 | ||
| } | ||
| return new Date(year, month - 1, day, hour24, minutes, seconds) |
There was a problem hiding this comment.
🟡 date_utc is interpreted as local time.
new Date(year, month-1, day, hour24, ...) constructs a Date in the device's local zone, but the source field is named date_utc. For a UTC instant like 1/27/2025 2:00:00 AM, a user at UTC-6 is really at 8 PM on Jan 26 — but this buckets and labels the game as Jan 27. Games near midnight UTC land in the wrong Today/Yesterday/Upcoming section with the wrong day-name header for any non-UTC user.
If the intent is to show the school's local wall-clock time, that's defensible — but then the field/handling should be documented as such rather than named _utc.
— Claude (automated review)
| type Props = { | ||
| data: Score[] | ||
| /** When true, show a date prefix above each game (used in Upcoming view). */ | ||
| includePrefix?: boolean |
There was a problem hiding this comment.
🟡 includePrefix is declared and passed but never used.
This prop is documented ("show a date prefix above each game") and list.tsx passes includePrefix={selectedSection === Constants.UPCOMING}, but AthleticsRow destructures only {data} and never reads it. The documented Upcoming date-prefix behavior doesn't exist. Either wire it up or drop the prop (and the per-row comparison in list.tsx).
— Claude (automated review)
| } | ||
| byDate[key].push(score) | ||
| } | ||
| return Object.keys(byDate) |
There was a problem hiding this comment.
🟡 Upcoming sections render in insertion order, not chronological order.
byDate is keyed by formatted strings like "Sunday, January 26". Object.keys() iterates non-integer string keys in insertion order, which here equals the order scores arrive from the API. If athletics/scores returns games grouped by sport (or otherwise unsorted), the Upcoming tab shows dates out of order (e.g. Feb 1 above Jan 28). Sort the keys (or the upcoming scores) by parsedDate before building sections.
— Claude (automated review)
| ) | ||
|
|
||
| export function selectShowChangeFiltersMessage(state: FilterState): boolean { | ||
| const {selectedSports, totalSports} = state |
There was a problem hiding this comment.
🟡 Filter-hint check compares only lengths, not sets.
selectedSports.length === totalSports treats "all selected" as a count match. But selectedSports is persisted across sessions and can hold sports no longer in the current data, while totalSports counts only current sports. If a season drops one sport and adds another, the lengths can coincide while the sets differ — suppressing the hint even though the user is filtering out a current sport (and vice-versa). Comparing membership (e.g. every current sport is in selectedSports) would be robust.
— Claude (automated review)
| return [] | ||
| } | ||
|
|
||
| if (selectedSection === Constants.UPCOMING) { |
There was a problem hiding this comment.
⚪ Duplicated date grouping.
groupScoresByDate (utils.ts) already emits one section per upcoming day keyed by formatDateString. This branch flattens those sections back into a list and rebuilds the same Record<string, ProcessedScore[]> by formatDateString — re-implementing the shared util's grouping. The two copies can drift (key format, ordering). Consider reusing the upcoming sections groupScoresByDate already produced.
— Claude (automated review)
| coverage: Coverage | ||
| } | ||
|
|
||
| export interface AthleticsResponse { |
There was a problem hiding this comment.
⚪ Dead types.
AthleticsResponse, AthleticsData, and GroupedScores are exported but never imported anywhere in the module. AthleticsResponse also describes a {scores: Score[]} envelope that query.ts never uses (queryFn does .json<Score[]>()), and GroupedScores (Score[]) parallels DateGroupedScores (ProcessedScore[]). Removing them avoids a future reader having to reconcile two "grouped scores" shapes.
— Claude (automated review)
|
|
||
| # generated logs files in CI | ||
| /logs | ||
| .worktrees/ |
There was a problem hiding this comment.
⚪ Unrelated to this feature.
.worktrees/ looks like a personal git-worktree / dev-tool artifact with no connection to the Athletics module. Bundling it into a feature PR muddies .gitignore blame. Worth splitting out (and possibly into a personal global gitignore instead).
— Claude (automated review)
|
@copilot please make changes to address Hawken's feedback |
Co-authored-by: drewvolz <5240843+drewvolz@users.noreply.github.com>
Head branch was pushed to by a user without write access
This pull request introduces an Athletics section, providing users with an interface for viewing and filtering St. Olaf athletics scores. The implementation includes navigation integration, data fetching and grouping, UI components for listing scores, filtering by sport, and persistent filter preferences. Additionally, the app is configured to allow insecure HTTP loads from the athletics API domain for development purposes.
The most important changes are:
Navigation Integration
Athleticsscreen to the navigation stack and defined its navigation key and options (routes.tsx,types.tsx,index.ts).Data Fetching and Grouping
query.ts,utils.ts,types.ts).UI Components
list.tsx,tabbar.tsx,row.tsx,filters.tsx).User Preferences and State Management
store.ts).