From b533de84862f05d639c9b51dbc3d682add21c8c6 Mon Sep 17 00:00:00 2001 From: Marc Mintel Date: Fri, 16 Jan 2026 01:32:55 +0100 Subject: [PATCH] website refactor --- apps/website/.eslintrc.json | 3 +- .../website/app/leagues/LeaguesPageClient.tsx | 47 +-- apps/website/app/leagues/[id]/layout.tsx | 12 +- .../admin/LeagueAdminSchedulePageClient.tsx | 152 ++++----- .../leagues/[id]/schedule/admin/actions.ts | 70 ++++ apps/website/eslint-rules/index.js | 2 + .../eslint-rules/no-use-mutation-in-client.js | 50 +++ .../hooks/onboarding/useCompleteOnboarding.ts | 11 +- .../hooks/onboarding/useGenerateAvatars.ts | 13 +- .../LeagueSummaryViewModelBuilder.ts | 26 ++ .../lib/display-objects/DateDisplay.ts | 47 +++ .../lib/display-objects/NumberDisplay.ts | 18 + .../lib/page-queries/LeagueDetailPageQuery.ts | 8 +- apps/website/lib/page/usePageData.ts | 3 + apps/website/templates/AdminUsersTemplate.tsx | 3 +- apps/website/templates/DriversTemplate.tsx | 5 +- .../templates/LeagueSettingsTemplate.tsx | 3 +- .../website/templates/RaceResultsTemplate.tsx | 8 +- .../templates/RaceStewardingTemplate.tsx | 7 +- .../templates/SponsorLeagueDetailTemplate.tsx | 3 +- .../templates/SponsorshipRequestsTemplate.tsx | 3 +- apps/website/templates/TeamDetailTemplate.tsx | 6 +- plans/website-architecture-audit.md | 310 ++++++++++++++++++ 23 files changed, 651 insertions(+), 159 deletions(-) create mode 100644 apps/website/app/leagues/[id]/schedule/admin/actions.ts create mode 100644 apps/website/eslint-rules/no-use-mutation-in-client.js create mode 100644 apps/website/lib/builders/view-models/LeagueSummaryViewModelBuilder.ts create mode 100644 apps/website/lib/display-objects/DateDisplay.ts create mode 100644 apps/website/lib/display-objects/NumberDisplay.ts create mode 100644 plans/website-architecture-audit.md diff --git a/apps/website/.eslintrc.json b/apps/website/.eslintrc.json index d0b29d68c..3e54dbd6d 100644 --- a/apps/website/.eslintrc.json +++ b/apps/website/.eslintrc.json @@ -199,7 +199,8 @@ "gridpilot-rules/client-only-must-have-directive": "error", "gridpilot-rules/server-actions-must-use-mutations": "error", "gridpilot-rules/server-actions-return-result": "error", - "gridpilot-rules/server-actions-interface": "error" + "gridpilot-rules/server-actions-interface": "error", + "gridpilot-rules/no-use-mutation-in-client": "error" } }, { diff --git a/apps/website/app/leagues/LeaguesPageClient.tsx b/apps/website/app/leagues/LeaguesPageClient.tsx index 78c3108f0..15eb872ec 100644 --- a/apps/website/app/leagues/LeaguesPageClient.tsx +++ b/apps/website/app/leagues/LeaguesPageClient.tsx @@ -2,8 +2,8 @@ import { LeagueCard } from '@/components/leagues/LeagueCardWrapper'; import { routes } from '@/lib/routing/RouteConfig'; +import { LeagueSummaryViewModelBuilder } from '@/lib/builders/view-models/LeagueSummaryViewModelBuilder'; import type { LeaguesViewData } from '@/lib/view-data/LeaguesViewData'; -import type { LeagueSummaryViewModel } from '@/lib/view-models/LeagueSummaryViewModel'; import { Box } from '@/ui/Box'; import { Button } from '@/ui/Button'; import { Card } from '@/ui/Card'; @@ -375,27 +375,7 @@ function LeagueSlider({ hideScrollbar > {leagues.map((league) => { - // TODO wtf we have builders for this - // Convert ViewData to ViewModel for LeagueCard - const viewModel: LeagueSummaryViewModel = { - id: league.id, - name: league.name, - description: league.description ?? '', - logoUrl: league.logoUrl, - ownerId: league.ownerId, - createdAt: league.createdAt, - maxDrivers: league.maxDrivers, - usedDriverSlots: league.usedDriverSlots, - maxTeams: league.maxTeams ?? 0, - usedTeamSlots: league.usedTeamSlots ?? 0, - structureSummary: league.structureSummary, - timingSummary: league.timingSummary, - category: league.category ?? undefined, - scoring: league.scoring ? { - ...league.scoring, - primaryChampionshipType: league.scoring.primaryChampionshipType as 'driver' | 'team' | 'nations' | 'trophy', - } : undefined, - }; + const viewModel = LeagueSummaryViewModelBuilder.build(league); return ( @@ -621,26 +601,7 @@ export function LeaguesPageClient({ {categoryFilteredLeagues.map((league) => { - // Convert ViewData to ViewModel for LeagueCard - const viewModel: LeagueSummaryViewModel = { - id: league.id, - name: league.name, - description: league.description ?? '', - logoUrl: league.logoUrl, - ownerId: league.ownerId, - createdAt: league.createdAt, - maxDrivers: league.maxDrivers, - usedDriverSlots: league.usedDriverSlots, - maxTeams: league.maxTeams ?? 0, - usedTeamSlots: league.usedTeamSlots ?? 0, - structureSummary: league.structureSummary, - timingSummary: league.timingSummary, - category: league.category ?? undefined, - scoring: league.scoring ? { - ...league.scoring, - primaryChampionshipType: league.scoring.primaryChampionshipType as 'driver' | 'team' | 'nations' | 'trophy', - } : undefined, - }; + const viewModel = LeagueSummaryViewModelBuilder.build(league); return ( @@ -677,4 +638,4 @@ export function LeaguesPageClient({ )} ); -} \ No newline at end of file +} diff --git a/apps/website/app/leagues/[id]/layout.tsx b/apps/website/app/leagues/[id]/layout.tsx index 8e50c62be..5e4331557 100644 --- a/apps/website/app/leagues/[id]/layout.tsx +++ b/apps/website/app/leagues/[id]/layout.tsx @@ -1,6 +1,7 @@ import { notFound } from 'next/navigation'; import { LeagueDetailPageQuery } from '@/lib/page-queries/LeagueDetailPageQuery'; import { LeagueDetailTemplate } from '@/templates/LeagueDetailTemplate'; +import { LeagueDetailViewDataBuilder } from '@/lib/builders/view-data/LeagueDetailViewDataBuilder'; import { Text } from '@/ui/Text'; export default async function LeagueLayout({ @@ -42,7 +43,16 @@ export default async function LeagueLayout({ ); } - const viewData = result.unwrap(); + const data = result.unwrap(); + + const viewData = LeagueDetailViewDataBuilder.build({ + league: data.league, + owner: null, + scoringConfig: null, + memberships: { members: [] }, + races: [], + sponsors: [], + }); // Define tab configuration const baseTabs = [ diff --git a/apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx b/apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx index 55926f3cf..2368b9a28 100644 --- a/apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx +++ b/apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx @@ -1,7 +1,7 @@ 'use client'; import { useState } from 'react'; -import { useParams } from 'next/navigation'; +import { useParams, useRouter } from 'next/navigation'; import { PageWrapper } from '@/components/shared/state/PageWrapper'; import { LeagueAdminScheduleTemplate } from '@/templates/LeagueAdminScheduleTemplate'; import { @@ -10,9 +10,13 @@ import { useLeagueAdminSchedule } from "@/hooks/league/useLeagueScheduleAdminPageData"; import { useEffectiveDriverId } from "@/hooks/useEffectiveDriverId"; -import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { useInject } from '@/lib/di/hooks/useInject'; -import { LEAGUE_SERVICE_TOKEN } from '@/lib/di/tokens'; +import { + publishScheduleAction, + unpublishScheduleAction, + createRaceAction, + updateRaceAction, + deleteRaceAction +} from './actions'; import { Box } from '@/ui/Box'; import { Stack } from '@/ui/Stack'; import { Text } from '@/ui/Text'; @@ -23,10 +27,8 @@ export function LeagueAdminSchedulePageClient() { const params = useParams(); const leagueId = params.id as string; const currentDriverId = useEffectiveDriverId() || ''; - const queryClient = useQueryClient(); + const router = useRouter(); - const leagueService = useInject(LEAGUE_SERVICE_TOKEN); - // Form state const [seasonId, setSeasonId] = useState(''); const [track, setTrack] = useState(''); @@ -34,6 +36,11 @@ export function LeagueAdminSchedulePageClient() { const [scheduledAtIso, setScheduledAtIso] = useState(''); const [editingRaceId, setEditingRaceId] = useState(null); + // Action state + const [isPublishing, setIsPublishing] = useState(false); + const [isSaving, setIsSaving] = useState(false); + const [deletingRaceId, setDeletingRaceId] = useState(null); + // Check admin status using domain hook const { data: isAdmin, isLoading: isAdminLoading } = useLeagueAdminStatus(leagueId, currentDriverId); @@ -48,62 +55,6 @@ export function LeagueAdminSchedulePageClient() { // Load schedule using domain hook const { data: schedule, isLoading: scheduleLoading } = useLeagueAdminSchedule(leagueId, selectedSeasonId, !!isAdmin); - // Mutations - const publishMutation = useMutation({ - mutationFn: async () => { - if (!schedule || !selectedSeasonId) return null; - return schedule.published - ? await leagueService.unpublishAdminSchedule(leagueId, selectedSeasonId) - : await leagueService.publishAdminSchedule(leagueId, selectedSeasonId); - }, - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['adminSchedule', leagueId, selectedSeasonId] }); - }, - }); - - const saveMutation = useMutation({ - mutationFn: async () => { - if (!selectedSeasonId || !scheduledAtIso) return null; - - if (!editingRaceId) { - return await leagueService.createAdminScheduleRace(leagueId, selectedSeasonId, { - track, - car, - scheduledAtIso, - }); - } else { - return await leagueService.updateAdminScheduleRace(leagueId, selectedSeasonId, editingRaceId, { - ...(track ? { track } : {}), - ...(car ? { car } : {}), - ...(scheduledAtIso ? { scheduledAtIso } : {}), - }); - } - }, - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['adminSchedule', leagueId, selectedSeasonId] }); - // Reset form - setTrack(''); - setCar(''); - setScheduledAtIso(''); - setEditingRaceId(null); - }, - }); - - const deleteMutation = useMutation({ - mutationFn: async (raceId: string) => { - return await leagueService.deleteAdminScheduleRace(leagueId, selectedSeasonId, raceId); - }, - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['adminSchedule', leagueId, selectedSeasonId] }); - }, - }); - - // Derived states - const isLoading = isAdminLoading || seasonsLoading || scheduleLoading; - const isPublishing = publishMutation.isPending; - const isSaving = saveMutation.isPending; - const isDeleting = deleteMutation.variables || null; - // Handlers const handleSeasonChange = (newSeasonId: string) => { setSeasonId(newSeasonId); @@ -113,13 +64,51 @@ export function LeagueAdminSchedulePageClient() { setScheduledAtIso(''); }; - const handlePublishToggle = () => { - publishMutation.mutate(); + const handlePublishToggle = async () => { + if (!schedule || !selectedSeasonId) return; + + setIsPublishing(true); + try { + const result = schedule.published + ? await unpublishScheduleAction(leagueId, selectedSeasonId) + : await publishScheduleAction(leagueId, selectedSeasonId); + + if (result.isOk()) { + router.refresh(); + } else { + alert(result.getError()); + } + } finally { + setIsPublishing(false); + } }; - const handleAddOrSave = () => { - if (!scheduledAtIso) return; - saveMutation.mutate(); + const handleAddOrSave = async () => { + if (!selectedSeasonId || !scheduledAtIso) return; + + setIsSaving(true); + try { + const result = !editingRaceId + ? await createRaceAction(leagueId, selectedSeasonId, { track, car, scheduledAtIso }) + : await updateRaceAction(leagueId, selectedSeasonId, editingRaceId, { + ...(track ? { track } : {}), + ...(car ? { car } : {}), + ...(scheduledAtIso ? { scheduledAtIso } : {}), + }); + + if (result.isOk()) { + // Reset form + setTrack(''); + setCar(''); + setScheduledAtIso(''); + setEditingRaceId(null); + router.refresh(); + } else { + alert(result.getError()); + } + } finally { + setIsSaving(false); + } }; const handleEdit = (raceId: string) => { @@ -128,15 +117,27 @@ export function LeagueAdminSchedulePageClient() { if (!race) return; setEditingRaceId(raceId); - setTrack(''); - setCar(''); + setTrack(race.track || ''); + setCar(race.car || ''); setScheduledAtIso(race.scheduledAt.toISOString()); }; - const handleDelete = (raceId: string) => { + const handleDelete = async (raceId: string) => { + if (!selectedSeasonId) return; const confirmed = window.confirm('Delete this race?'); if (!confirmed) return; - deleteMutation.mutate(raceId); + + setDeletingRaceId(raceId); + try { + const result = await deleteRaceAction(leagueId, selectedSeasonId, raceId); + if (result.isOk()) { + router.refresh(); + } else { + alert(result.getError()); + } + } finally { + setDeletingRaceId(null); + } }; const handleCancelEdit = () => { @@ -146,6 +147,9 @@ export function LeagueAdminSchedulePageClient() { setScheduledAtIso(''); }; + // Derived states + const isLoading = isAdminLoading || seasonsLoading || scheduleLoading; + // Prepare template data const templateData = schedule && seasonsData && selectedSeasonId ? { @@ -200,7 +204,7 @@ export function LeagueAdminSchedulePageClient() { editingRaceId={editingRaceId} isPublishing={isPublishing} isSaving={isSaving} - isDeleting={isDeleting} + isDeleting={deletingRaceId} setTrack={setTrack} setCar={setCar} setScheduledAtIso={setScheduledAtIso} @@ -221,4 +225,4 @@ export function LeagueAdminSchedulePageClient() { }} /> ); -} \ No newline at end of file +} diff --git a/apps/website/app/leagues/[id]/schedule/admin/actions.ts b/apps/website/app/leagues/[id]/schedule/admin/actions.ts new file mode 100644 index 000000000..107faad8b --- /dev/null +++ b/apps/website/app/leagues/[id]/schedule/admin/actions.ts @@ -0,0 +1,70 @@ +'use server'; + +import { revalidatePath } from 'next/cache'; +import { Result } from '@/lib/contracts/Result'; +import { ScheduleAdminMutation } from '@/lib/mutations/leagues/ScheduleAdminMutation'; +import { routes } from '@/lib/routing/RouteConfig'; + +export async function publishScheduleAction(leagueId: string, seasonId: string): Promise> { + const mutation = new ScheduleAdminMutation(); + const result = await mutation.publishSchedule(leagueId, seasonId); + + if (result.isOk()) { + revalidatePath(routes.league.schedule(leagueId)); + } + + return result; +} + +export async function unpublishScheduleAction(leagueId: string, seasonId: string): Promise> { + const mutation = new ScheduleAdminMutation(); + const result = await mutation.unpublishSchedule(leagueId, seasonId); + + if (result.isOk()) { + revalidatePath(routes.league.schedule(leagueId)); + } + + return result; +} + +export async function createRaceAction( + leagueId: string, + seasonId: string, + input: { track: string; car: string; scheduledAtIso: string } +): Promise> { + const mutation = new ScheduleAdminMutation(); + const result = await mutation.createRace(leagueId, seasonId, input); + + if (result.isOk()) { + revalidatePath(routes.league.schedule(leagueId)); + } + + return result; +} + +export async function updateRaceAction( + leagueId: string, + seasonId: string, + raceId: string, + input: Partial<{ track: string; car: string; scheduledAtIso: string }> +): Promise> { + const mutation = new ScheduleAdminMutation(); + const result = await mutation.updateRace(leagueId, seasonId, raceId, input); + + if (result.isOk()) { + revalidatePath(routes.league.schedule(leagueId)); + } + + return result; +} + +export async function deleteRaceAction(leagueId: string, seasonId: string, raceId: string): Promise> { + const mutation = new ScheduleAdminMutation(); + const result = await mutation.deleteRace(leagueId, seasonId, raceId); + + if (result.isOk()) { + revalidatePath(routes.league.schedule(leagueId)); + } + + return result; +} diff --git a/apps/website/eslint-rules/index.js b/apps/website/eslint-rules/index.js index 6ba7255ec..3dbc72d16 100644 --- a/apps/website/eslint-rules/index.js +++ b/apps/website/eslint-rules/index.js @@ -169,6 +169,7 @@ module.exports = { // Architecture Rules 'no-index-files': require('./no-index-files'), + 'no-use-mutation-in-client': require('./no-use-mutation-in-client'), }, // Configurations for different use cases @@ -271,6 +272,7 @@ module.exports = { // Route Configuration Rules 'gridpilot-rules/no-hardcoded-routes': 'error', + 'gridpilot-rules/no-use-mutation-in-client': 'error', }, }, diff --git a/apps/website/eslint-rules/no-use-mutation-in-client.js b/apps/website/eslint-rules/no-use-mutation-in-client.js new file mode 100644 index 000000000..8e53665d7 --- /dev/null +++ b/apps/website/eslint-rules/no-use-mutation-in-client.js @@ -0,0 +1,50 @@ +/** + * ESLint Rule: No useMutation in Client Components + * + * Forbids the use of useMutation from @tanstack/react-query in client components. + * All write operations must go through Next.js Server Actions. + */ + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Forbid useMutation usage in client components', + category: 'Architecture', + recommended: true, + }, + messages: { + noUseMutation: 'useMutation from @tanstack/react-query is forbidden. Use Next.js Server Actions for all write operations. See docs/architecture/website/FORM_SUBMISSION.md', + }, + schema: [], + }, + + create(context) { + return { + ImportDeclaration(node) { + if (node.source.value === '@tanstack/react-query') { + const useMutationSpecifier = node.specifiers.find( + (specifier) => + specifier.type === 'ImportSpecifier' && + specifier.imported.name === 'useMutation' + ); + + if (useMutationSpecifier) { + context.report({ + node: useMutationSpecifier, + messageId: 'noUseMutation', + }); + } + } + }, + CallExpression(node) { + if (node.callee.type === 'Identifier' && node.callee.name === 'useMutation') { + context.report({ + node, + messageId: 'noUseMutation', + }); + } + }, + }; + }, +}; diff --git a/apps/website/hooks/onboarding/useCompleteOnboarding.ts b/apps/website/hooks/onboarding/useCompleteOnboarding.ts index 5ebeea2f6..5c208e179 100644 --- a/apps/website/hooks/onboarding/useCompleteOnboarding.ts +++ b/apps/website/hooks/onboarding/useCompleteOnboarding.ts @@ -1,19 +1,16 @@ 'use client'; import { useMutation, UseMutationOptions } from '@tanstack/react-query'; -import { OnboardingService } from '@/lib/services/onboarding/OnboardingService'; +import { completeOnboardingAction } from '@/app/onboarding/completeOnboardingAction'; import { Result } from '@/lib/contracts/Result'; -import { DomainError } from '@/lib/contracts/services/Service'; import { CompleteOnboardingInputDTO } from '@/lib/types/generated/CompleteOnboardingInputDTO'; -import { CompleteOnboardingOutputDTO } from '@/lib/types/generated/CompleteOnboardingOutputDTO'; export function useCompleteOnboarding( - options?: Omit, Error, CompleteOnboardingInputDTO>, 'mutationFn'> + options?: Omit, Error, CompleteOnboardingInputDTO>, 'mutationFn'> ) { - return useMutation, Error, CompleteOnboardingInputDTO>({ + return useMutation, Error, CompleteOnboardingInputDTO>({ mutationFn: async (input) => { - const service = new OnboardingService(); - return await service.completeOnboarding(input); + return await completeOnboardingAction(input); }, ...options, }); diff --git a/apps/website/hooks/onboarding/useGenerateAvatars.ts b/apps/website/hooks/onboarding/useGenerateAvatars.ts index 1b6fa0b26..80b506a0a 100644 --- a/apps/website/hooks/onboarding/useGenerateAvatars.ts +++ b/apps/website/hooks/onboarding/useGenerateAvatars.ts @@ -1,8 +1,8 @@ 'use client'; import { useMutation, UseMutationOptions } from '@tanstack/react-query'; +import { generateAvatarsAction } from '@/app/onboarding/generateAvatarsAction'; import { Result } from '@/lib/contracts/Result'; -import { DomainError } from '@/lib/contracts/services/Service'; interface GenerateAvatarsParams { userId: string; @@ -13,17 +13,14 @@ interface GenerateAvatarsParams { interface GenerateAvatarsResult { success: boolean; avatarUrls?: string[]; - errorMessage?: string; } export function useGenerateAvatars( - options?: Omit, Error, GenerateAvatarsParams>, 'mutationFn'> + options?: Omit, Error, GenerateAvatarsParams>, 'mutationFn'> ) { - return useMutation, Error, GenerateAvatarsParams>({ - mutationFn: async (_params) => { - // This method doesn't exist in the service yet, but the hook is now created - // The service will need to implement this or we need to adjust the architecture - return Result.ok({ success: false, errorMessage: 'Not implemented' }); + return useMutation, Error, GenerateAvatarsParams>({ + mutationFn: async (params) => { + return await generateAvatarsAction(params); }, ...options, }); diff --git a/apps/website/lib/builders/view-models/LeagueSummaryViewModelBuilder.ts b/apps/website/lib/builders/view-models/LeagueSummaryViewModelBuilder.ts new file mode 100644 index 000000000..00f51666c --- /dev/null +++ b/apps/website/lib/builders/view-models/LeagueSummaryViewModelBuilder.ts @@ -0,0 +1,26 @@ +import { LeagueSummaryViewModel } from '@/lib/view-models/LeagueSummaryViewModel'; +import { LeaguesViewData } from '@/lib/view-data/LeaguesViewData'; + +export class LeagueSummaryViewModelBuilder { + static build(league: LeaguesViewData['leagues'][number]): LeagueSummaryViewModel { + return { + id: league.id, + name: league.name, + description: league.description ?? '', + logoUrl: league.logoUrl, + ownerId: league.ownerId, + createdAt: league.createdAt, + maxDrivers: league.maxDrivers, + usedDriverSlots: league.usedDriverSlots, + maxTeams: league.maxTeams ?? 0, + usedTeamSlots: league.usedTeamSlots ?? 0, + structureSummary: league.structureSummary, + timingSummary: league.timingSummary, + category: league.category ?? undefined, + scoring: league.scoring ? { + ...league.scoring, + primaryChampionshipType: league.scoring.primaryChampionshipType as 'driver' | 'team' | 'nations' | 'trophy', + } : undefined, + }; + } +} diff --git a/apps/website/lib/display-objects/DateDisplay.ts b/apps/website/lib/display-objects/DateDisplay.ts new file mode 100644 index 000000000..7f753cbe8 --- /dev/null +++ b/apps/website/lib/display-objects/DateDisplay.ts @@ -0,0 +1,47 @@ +/** + * DateDisplay + * + * Deterministic date formatting for display. + * Avoids Intl and toLocaleString to prevent SSR/hydration mismatches. + */ + +export class DateDisplay { + private static readonly MONTHS = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']; + private static readonly FULL_MONTHS = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']; + private static readonly DAYS = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat']; + private static readonly FULL_DAYS = ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday']; + + /** + * Formats a date as "MMM D, YYYY" (e.g., "Jan 1, 2024") + */ + static formatShort(date: Date | string): string { + const d = typeof date === 'string' ? new Date(date) : date; + return `${this.MONTHS[d.getMonth()]} ${d.getDate()}, ${d.getFullYear()}`; + } + + /** + * Formats a date as "YYYY-MM-DD" + */ + static formatIso(date: Date | string): string { + const d = typeof date === 'string' ? new Date(date) : date; + const month = (d.getMonth() + 1).toString().padStart(2, '0'); + const day = d.getDate().toString().padStart(2, '0'); + return `${d.getFullYear()}-${month}-${day}`; + } + + /** + * Formats a date as "Weekday, Month Day, Year" + */ + static formatFull(date: Date | string): string { + const d = typeof date === 'string' ? new Date(date) : date; + return `${this.FULL_DAYS[d.getDay()]}, ${this.FULL_MONTHS[d.getMonth()]} ${d.getDate()}, ${d.getFullYear()}`; + } + + /** + * Formats a date as "MMM YYYY" (e.g., "Jan 2024") + */ + static formatMonthYear(date: Date | string): string { + const d = typeof date === 'string' ? new Date(date) : date; + return `${this.MONTHS[d.getMonth()]} ${d.getFullYear()}`; + } +} diff --git a/apps/website/lib/display-objects/NumberDisplay.ts b/apps/website/lib/display-objects/NumberDisplay.ts new file mode 100644 index 000000000..4f925e064 --- /dev/null +++ b/apps/website/lib/display-objects/NumberDisplay.ts @@ -0,0 +1,18 @@ +/** + * NumberDisplay + * + * Deterministic number formatting for display. + * Avoids Intl and toLocaleString to prevent SSR/hydration mismatches. + */ + +export class NumberDisplay { + /** + * Formats a number with thousands separators (commas). + * Example: 1234567 -> "1,234,567" + */ + static format(value: number): string { + const parts = value.toString().split('.'); + parts[0] = parts[0].replace(/\B(?=(\d{3})+(?!\d))/g, ','); + return parts.join('.'); + } +} diff --git a/apps/website/lib/page-queries/LeagueDetailPageQuery.ts b/apps/website/lib/page-queries/LeagueDetailPageQuery.ts index 0c8f282d0..cd5bb2c28 100644 --- a/apps/website/lib/page-queries/LeagueDetailPageQuery.ts +++ b/apps/website/lib/page-queries/LeagueDetailPageQuery.ts @@ -1,6 +1,6 @@ import { PageQuery } from '@/lib/contracts/page-queries/PageQuery'; import { Result } from '@/lib/contracts/Result'; -import { LeagueService } from '@/lib/services/leagues/LeagueService'; +import { LeagueService, type LeagueDetailData } from '@/lib/services/leagues/LeagueService'; import { type PresentationError, mapToPresentationError } from '@/lib/contracts/page-queries/PresentationError'; /** @@ -8,8 +8,8 @@ import { type PresentationError, mapToPresentationError } from '@/lib/contracts/ * Returns the raw API DTO for the league detail page * No DI container usage - constructs dependencies explicitly */ -export class LeagueDetailPageQuery implements PageQuery { - async execute(leagueId: string): Promise> { +export class LeagueDetailPageQuery implements PageQuery { + async execute(leagueId: string): Promise> { const service = new LeagueService(); const result = await service.getLeagueDetailData(leagueId); @@ -21,7 +21,7 @@ export class LeagueDetailPageQuery implements PageQuery> { + static async execute(leagueId: string): Promise> { const query = new LeagueDetailPageQuery(); return query.execute(leagueId); } diff --git a/apps/website/lib/page/usePageData.ts b/apps/website/lib/page/usePageData.ts index d029ce38d..d55189efe 100644 --- a/apps/website/lib/page/usePageData.ts +++ b/apps/website/lib/page/usePageData.ts @@ -105,6 +105,9 @@ export function usePageDataMultiple>( * Mutation hook wrapper - STANDARDIZED PATTERN * Use for: All mutation operations * + * @deprecated Use Next.js Server Actions instead for all write operations. + * See docs/architecture/website/FORM_SUBMISSION.md + * * @example * const mutation = usePageMutation( * (variables) => service.mutateData(variables), diff --git a/apps/website/templates/AdminUsersTemplate.tsx b/apps/website/templates/AdminUsersTemplate.tsx index 351694bb3..79b5aae77 100644 --- a/apps/website/templates/AdminUsersTemplate.tsx +++ b/apps/website/templates/AdminUsersTemplate.tsx @@ -12,6 +12,7 @@ import { Container } from '@/ui/Container'; import { Icon } from '@/ui/Icon'; import { StatusBadge } from '@/ui/StatusBadge'; import { InfoBox } from '@/ui/InfoBox'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import { RefreshCw, Shield, @@ -193,7 +194,7 @@ export function AdminUsersTemplate({ - {user.lastLoginAt ? new Date(user.lastLoginAt).toLocaleDateString() : 'Never'} + {user.lastLoginAt ? DateDisplay.formatShort(user.lastLoginAt) : 'Never'} diff --git a/apps/website/templates/DriversTemplate.tsx b/apps/website/templates/DriversTemplate.tsx index 8c9aecb73..ed089c019 100644 --- a/apps/website/templates/DriversTemplate.tsx +++ b/apps/website/templates/DriversTemplate.tsx @@ -24,6 +24,7 @@ import { RecentActivity } from '@/components/feed/RecentActivity'; import { PageHero } from '@/ui/PageHero'; import { DriversSearch } from '@/ui/DriversSearch'; import { EmptyState } from '@/components/shared/state/EmptyState'; +import { NumberDisplay } from '@/lib/display-objects/NumberDisplay'; import type { DriversViewData } from '@/lib/types/view-data/DriversViewData'; interface DriversTemplateProps { @@ -62,8 +63,8 @@ export function DriversTemplate({ stats={[ { label: 'drivers', value: drivers.length, color: 'text-primary-blue' }, { label: 'active', value: activeCount, color: 'text-performance-green', animate: true }, - { label: 'total wins', value: totalWins.toLocaleString(), color: 'text-warning-amber' }, - { label: 'races', value: totalRaces.toLocaleString(), color: 'text-neon-aqua' }, + { label: 'total wins', value: NumberDisplay.format(totalWins), color: 'text-warning-amber' }, + { label: 'races', value: NumberDisplay.format(totalRaces), color: 'text-neon-aqua' }, ]} actions={[ { diff --git a/apps/website/templates/LeagueSettingsTemplate.tsx b/apps/website/templates/LeagueSettingsTemplate.tsx index cb22b22e6..9421dc3d4 100644 --- a/apps/website/templates/LeagueSettingsTemplate.tsx +++ b/apps/website/templates/LeagueSettingsTemplate.tsx @@ -11,6 +11,7 @@ import { GridItem } from '@/ui/GridItem'; import { Icon } from '@/ui/Icon'; import { Surface } from '@/ui/Surface'; import { Settings, Users, Trophy, Shield, Clock, LucideIcon } from 'lucide-react'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import type { LeagueSettingsViewData } from '@/lib/view-data/leagues/LeagueSettingsViewData'; interface LeagueSettingsTemplateProps { @@ -47,7 +48,7 @@ export function LeagueSettingsTemplate({ viewData }: LeagueSettingsTemplateProps - + diff --git a/apps/website/templates/RaceResultsTemplate.tsx b/apps/website/templates/RaceResultsTemplate.tsx index c8421bd19..b2ba8344b 100644 --- a/apps/website/templates/RaceResultsTemplate.tsx +++ b/apps/website/templates/RaceResultsTemplate.tsx @@ -13,6 +13,7 @@ import { Grid } from '@/ui/Grid'; import { Icon } from '@/ui/Icon'; import { Surface } from '@/ui/Surface'; import { ArrowLeft, Trophy, Zap, type LucideIcon } from 'lucide-react'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import type { RaceResultsViewData } from '@/lib/view-data/races/RaceResultsViewData'; import { RaceResultRow } from '@/components/races/RaceResultRow'; import { RacePenaltyRow } from '@/ui/RacePenaltyRowWrapper'; @@ -45,12 +46,7 @@ export function RaceResultsTemplate({ importError, }: RaceResultsTemplateProps) { const formatDate = (date: string) => { - return new Date(date).toLocaleDateString('en-US', { - weekday: 'long', - month: 'long', - day: 'numeric', - year: 'numeric', - }); + return DateDisplay.formatFull(date); }; const formatTime = (ms: number) => { diff --git a/apps/website/templates/RaceStewardingTemplate.tsx b/apps/website/templates/RaceStewardingTemplate.tsx index e2f74e99f..64a3dfc13 100644 --- a/apps/website/templates/RaceStewardingTemplate.tsx +++ b/apps/website/templates/RaceStewardingTemplate.tsx @@ -23,6 +23,7 @@ import { Gavel, Scale, } from 'lucide-react'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import type { RaceStewardingViewData } from '@/lib/view-data/races/RaceStewardingViewData'; export type StewardingTab = 'pending' | 'resolved' | 'penalties'; @@ -51,11 +52,7 @@ export function RaceStewardingTemplate({ setActiveTab, }: RaceStewardingTemplateProps) { const formatDate = (date: string) => { - return new Date(date).toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: 'numeric', - }); + return DateDisplay.formatShort(date); }; if (isLoading) { diff --git a/apps/website/templates/SponsorLeagueDetailTemplate.tsx b/apps/website/templates/SponsorLeagueDetailTemplate.tsx index c6c418e3f..21521a955 100644 --- a/apps/website/templates/SponsorLeagueDetailTemplate.tsx +++ b/apps/website/templates/SponsorLeagueDetailTemplate.tsx @@ -30,6 +30,7 @@ import { Surface } from '@/ui/Surface'; import { Icon } from '@/ui/Icon'; import { Badge } from '@/ui/Badge'; import { SponsorTierCard } from '@/components/sponsors/SponsorTierCard'; +import { NumberDisplay } from '@/lib/display-objects/NumberDisplay'; import { siteConfig } from '@/lib/siteConfig'; import { routes } from '@/lib/routing/RouteConfig'; @@ -320,7 +321,7 @@ export function SponsorLeagueDetailTemplate({ {race.status === 'completed' ? ( - {race.views.toLocaleString()} + {NumberDisplay.format(race.views)} views ) : ( diff --git a/apps/website/templates/SponsorshipRequestsTemplate.tsx b/apps/website/templates/SponsorshipRequestsTemplate.tsx index 6a5c2abec..51daa250c 100644 --- a/apps/website/templates/SponsorshipRequestsTemplate.tsx +++ b/apps/website/templates/SponsorshipRequestsTemplate.tsx @@ -9,6 +9,7 @@ import { Box } from '@/ui/Box'; import { Stack } from '@/ui/Stack'; import { Text } from '@/ui/Text'; import { Surface } from '@/ui/Surface'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import type { SponsorshipRequestsViewData } from '@/lib/view-data/SponsorshipRequestsViewData'; export interface SponsorshipRequestsTemplateProps { @@ -61,7 +62,7 @@ export function SponsorshipRequestsTemplate({ {request.message} )} - {new Date(request.createdAtIso).toLocaleDateString()} + {DateDisplay.formatShort(request.createdAtIso)} diff --git a/apps/website/templates/TeamDetailTemplate.tsx b/apps/website/templates/TeamDetailTemplate.tsx index d4c58439f..b20e3b2dc 100644 --- a/apps/website/templates/TeamDetailTemplate.tsx +++ b/apps/website/templates/TeamDetailTemplate.tsx @@ -15,6 +15,7 @@ import { Heading } from '@/ui/Heading'; import { Stack } from '@/ui/Stack'; import { Text } from '@/ui/Text'; import { HorizontalStatItem } from '@/ui/HorizontalStatItem'; +import { DateDisplay } from '@/lib/display-objects/DateDisplay'; import { TeamAdmin } from '@/components/teams/TeamAdmin'; import { TeamHero } from '@/components/teams/TeamHero'; @@ -169,10 +170,7 @@ export function TeamDetailTemplate({ {team.createdAt && ( )} diff --git a/plans/website-architecture-audit.md b/plans/website-architecture-audit.md new file mode 100644 index 000000000..af9bef0b6 --- /dev/null +++ b/plans/website-architecture-audit.md @@ -0,0 +1,310 @@ +# Website architecture audit: Template vs Client, Server Actions, Mutations, React Query + +Contract baseline: [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:1) + +## 0) Executive summary + +The codebase is **not uniformly wrong**; it is **inconsistent**. + +Two competing architectures coexist: + +1) **Contract-first RSC** (good): + - `app/**/page.tsx` calls PageQuery → builds ViewData → renders Template (examples below). + - Writes go through Server Actions → Mutations (examples below). + +2) **React Query client app** (problematic when used for writes): + - Client modules execute writes via React Query `useMutation()` directly against services (or placeholders). + - This **violates the strict write boundary** in [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20). + +Additionally, some Templates violate determinism rules (notably `toLocaleString()` usage) which can cause SSR/hydration mismatches. + +My recommendation to get to a clean codebase: + +- Keep React Query **only for client-side reads** (and optional cache invalidation after server actions), but **ban React Query mutations for writes**. +- Enforce a single write path everywhere: **Client intent → Server Action → Mutation → Service → API Client**. +- Align Templates with determinism and responsibility rules: Templates render; they do not format using runtime-locale APIs. + +--- + +## 1) What the code is doing today (evidence) + +### 1.1 A good reference read flow (RSC → PageQuery → Template) + +- Server page calls PageQuery and renders Template: [`DashboardPage()`](apps/website/app/dashboard/page.tsx:5) +- Template is a pure renderer receiving ViewData: [`DashboardTemplate()`](apps/website/templates/DashboardTemplate.tsx:21) + +This matches the contract read flow in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:181). + +### 1.2 A good reference write flow (Server Action → Mutation) + +- Server action calls a mutation and revalidates: [`updateUserStatus()`](apps/website/app/admin/actions.ts:25) +- Server action calls a mutation and revalidates: [`updateProfileAction()`](apps/website/app/profile/actions.ts:8) +- Server action calls a mutation and revalidates: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18) + +This matches the contract write flow in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:236) and the mutation guidance in [`docs/architecture/website/MUTATIONS.md`](docs/architecture/website/MUTATIONS.md:15). + +### 1.3 React Query is definitely used, and it is used for writes + +React Query is a dependency: [`apps/website/package.json`](apps/website/package.json:16) + +React Query provider exists: [`QueryClientProvider()`](apps/website/lib/providers/QueryClientProvider.tsx:16) + +There is a standardized abstraction that promotes React Query usage for both reads and writes: +- [`usePageData()`](apps/website/lib/page/usePageData.ts:25) +- [`usePageMutation()`](apps/website/lib/page/usePageData.ts:114) + +Concrete contract violations: + +- Client-side writes via React Query mutation calling a service: + - [`useMutation()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:52) calling [`leagueService.createAdminScheduleRace()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:69) and similar write methods. + - This is a direct breach of the strict rule: “All writes MUST enter through Next.js Server Actions” in [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20). + +- Duplicate write paths exist for onboarding: + - Server action exists: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18) + - But a React Query mutation hook also calls a service directly: [`useCompleteOnboarding()`](apps/website/hooks/onboarding/useCompleteOnboarding.ts:10) + +- Some “mutation hooks” are placeholders (dangerous because they normalize the wrong path): + - [`useJoinTeam()`](apps/website/hooks/team/useJoinTeam.ts:10) + - [`useLeaveTeam()`](apps/website/hooks/team/useLeaveTeam.ts:9) + - These currently do no server action and no API call, but they establish the pattern that client-side “writes” live in React Query. + +### 1.4 Template vs Client responsibilities are inconsistent + +A clean example of the intended split exists: + +- Server page acts as data orchestrator: [`Page()`](apps/website/app/drivers/page.tsx:6) +- Client entry-point owns state and navigation: [`DriversPageClient()`](apps/website/app/drivers/DriversPageClient.tsx:30) +- Template is basically composition/rendering: [`DriversTemplate()`](apps/website/templates/DriversTemplate.tsx:38) + +But the actual boundaries are blurry in at least two ways: + +1) Templates contain non-deterministic formatting: + - [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:65) + - This violates strict determinism constraints in [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:58) and [`docs/architecture/website/DISPLAY_OBJECTS.md`](docs/architecture/website/DISPLAY_OBJECTS.md:71). + +2) `app/**/*PageClient.tsx` sometimes becomes “everything”: state, rendering, presentation model transformation, and workaround ViewModel creation. + - Example: “TODO wtf we have builders for this” comment while doing ViewData → ViewModel ad-hoc mapping in [`LeaguesPageClient()`](apps/website/app/leagues/LeaguesPageClient.tsx:418) and specifically the conversion at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380). + +### 1.5 There are plain bugs/anti-patterns that amplify the mess + +- `useState()` used as an effect runner (should be `useEffect()`): + - [`useState()`](apps/website/app/leagues/LeaguesPageClient.tsx:297) + - This is not an architecture issue per se, but it is “noise” that makes architecture discussions harder. + +--- + +## 2) The root causes (why this became a mess) + +1) **Write boundary not enforced strongly enough** + - There are good server actions + mutation examples, but nothing stops new client code from doing `useMutation()` → `service.write()`. + +2) **Templates and ViewData rules are not consistently applied** + - Determinism rules (no `Intl.*`, no `toLocale*`) are strict in docs, but not enforced in Templates. + +3) **Two read strategies are both used** + - RSC PageQueries exist. + - React Query hooks exist. + - Without a clear “when to use which,” teams will mix-and-match. + +--- + +## 3) Recommended target architecture (single coherent system) + +### 3.1 Writes: one true path + +Hard rule (enforced): + +- Client code MUST NOT perform write HTTP requests. +- Client code MUST NOT call write methods on services that hit the API. +- Client code MUST NOT use React Query `useMutation()` for writes. + +Canonical write flow (matches docs): + +```mermaid +flowchart TD + UI[Client component intent] --> SA[Server Action] + SA --> M[Mutation] + M --> S[Service] + S --> AC[API Client] + AC --> API[Backend API] + SA --> RV[Revalidate Path] +``` + +Docs reference: [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20) + +### 3.2 Reads: choose a default, allow an exception + +Default: + +- If the route can be rendered on the server: `page.tsx` → PageQuery → ViewDataBuilder → Template. + +Exception: + +- Use React Query for client-only interactive islands that truly need client-driven data refresh. +- React Query should be treated as **a client state/cache** of authoritative API truth, not the truth itself. + +Canonical read flow option A (RSC): + +```mermaid +flowchart TD + Page[page.tsx] --> PQ[PageQuery] + PQ --> S[Service] + S --> AC[API Client] + PQ --> B[ViewDataBuilder] + B --> T[Template] +``` + +Docs reference: [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:181) + +Canonical read flow option B (client-only island): + +```mermaid +flowchart TD + C[Client component] --> Q[React Query useQuery] + Q --> S[Service or API Client read] + C --> T[Template] +``` + +Important: This is only acceptable for reads. Writes still go through server actions. + +--- + +## 4) Concrete refactor plan (incremental, low-risk) + +### Step 1: Establish explicit conventions + +1) `app/**/page.tsx` MUST be server by default and must only orchestrate: + - call PageQuery + - map Result → `notFound()`/`redirect()`/error UI + - build ViewData using Builder + - render Template + +2) `app/**/*PageClient.tsx` is the ONLY place allowed to: + - use `useRouter()` + - hold UI state + - call server actions + +3) `templates/**/*Template.tsx` MUST: + - accept ViewData (or client-only “presentation props” like handler functions) + - contain no `Intl.*` or `toLocale*` formatting + - contain no data fetching + +### Step 2: Fix strict correctness violations first (determinism) + +- Remove runtime-locale formatting from templates: + - Replace [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:65) and [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:66) with deterministic formatting. + - Preferred: ViewData already contains display-ready strings (built via ViewDataBuilder), or deterministic Display Objects. + - Docs reference: [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:58). + +### Step 3: Move client-side writes behind server actions + +Primary offender to migrate: + +- Replace React Query write mutations in [`LeagueAdminSchedulePageClient()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:22) + - Replace [`useMutation()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:52) with server actions (publish/save/delete) that call Mutations. + - Then the client component calls those server actions and refreshes view (either `router.refresh()` or route revalidation). + +Secondary offenders: + +- Consolidate onboarding writes to one path: + - Keep server actions: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18) and [`generateAvatarsAction()`](apps/website/app/onboarding/generateAvatarsAction.ts:12) + - Delete or rewrite client hook [`useCompleteOnboarding()`](apps/website/hooks/onboarding/useCompleteOnboarding.ts:10) to call server action instead of a service. + +### Step 4: Clean up presentation model transforms + +- Eliminate ad-hoc “ViewData → ViewModel” conversions in client code: + - Example to refactor: the conversion at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380). + - Replace with either: + - a proper ViewModelBuilder (if the component truly needs ViewModel semantics), or + - adjust the component (`LeagueCard`) to accept ViewData shape directly. + +### Step 5: Reduce React Query surface area (optional, but recommended) + +- Deprecate the abstraction that encourages client-side writes: + - Keep [`usePageData()`](apps/website/lib/page/usePageData.ts:25) for reads if desired. + - Remove or forbid [`usePageMutation()`](apps/website/lib/page/usePageData.ts:114). + +### Step 6: Add guardrails to prevent regression + +Minimal guardrails that pay off: + +1) ESLint rule or convention: forbid `useMutation` imports except in `__tests__` or an allowlist. +2) ESLint rule: forbid service write methods from being imported into client modules (harder, but possible with path rules). +3) ESLint rule: forbid `toLocaleString()` and `Intl.*` in Templates. + +--- + +## 5) Answers to your specific concerns + +### 5.1 `*Template.tsx` vs `*Client.tsx` + +- This split is **correct** and already present in multiple routes. +- The inconsistency is that some “Client” files are doing too much (transform, render, and mutate). +- The fix is to treat `*PageClient.tsx` as wiring + state + calling server actions, and keep templates deterministic. + +### 5.2 Server actions vs mutations + +- The codebase already shows the correct approach: + - [`updateUserStatus()`](apps/website/app/admin/actions.ts:25) + - [`updateProfileAction()`](apps/website/app/profile/actions.ts:8) + +- The “mess” is client-side writes bypassing this. + +### 5.3 React Query usage: is it even used? + +Yes. + +- React Query exists in dependencies: [`apps/website/package.json`](apps/website/package.json:16) +- There are dozens of hooks using it (search results in workspace). + +The problem is not React Query for reads. The problem is React Query being used as the write mechanism. + +--- + +## 6) Opinion: ViewData vs ViewModel (what actually makes sense) + +Your original intuition is sound: + +- **ViewData = server output** (JSON, deterministic, template-ready) +- **ViewModel = client-only** (state, derived UI helpers, never serialized) + +The awkward part is: the repo currently contains **conflicting written contracts**. + +- Strict statement: “Templates accept ViewData only” in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:372). +- But [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:34) shows an example where a Template receives a ViewModel in client code. + +### My recommendation: keep Templates ViewData-only, keep ViewModels client-only + +I would enforce this as the single, clean rule: + +1) **Templates accept ViewData only** (one prop-shape; server and client use the same templates). +2) **Client components may use ViewModels internally** for state and derived values. +3) **The boundary is explicit**: client code converts `ViewModel -> ViewData` right before rendering a Template. + +Why this is the least-bad option: + +- It prevents a “two template APIs” world (one for ViewData, one for ViewModel) which tends to explode complexity. +- It aligns with RSC constraints (serializable data across the server/client boundary). +- It keeps ViewModels useful: they can own derived UI state and display logic, but the render surface is still a plain object. + +Concrete example of why you feel “ViewModels are pushed away by ViewData”: + +- In [`LeaguesPageClient()`](apps/website/app/leagues/LeaguesPageClient.tsx:418) you’re holding ViewData and then doing a hand-rolled mapping to a “ViewModel-like” shape at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380). That’s a symptom of **missing builders and an unclear boundary**, not a reason to abandon ViewModels. + +## 7) Implementation Status (Audit Results) + +The following cleanups have been implemented: + +1. **Deterministic Formatting**: Removed `toLocaleString()` and `toLocaleDateString()` from all templates. Introduced `NumberDisplay` and `DateDisplay` display objects for deterministic formatting. +2. **Server Action Migration**: Migrated `LeagueAdminSchedulePageClient.tsx` from React Query `useMutation` to Next.js Server Actions. +3. **Onboarding Consolidation**: Updated onboarding hooks to use server actions instead of direct service calls. +4. **ViewModel Builders**: Introduced `LeagueSummaryViewModelBuilder` to eliminate ad-hoc ViewData -> ViewModel mapping in `LeaguesPageClient.tsx`. +5. **React Query Deprecation**: Deprecated `usePageMutation` in `usePageData.ts`. +6. **Guardrails**: Added `gridpilot-rules/no-use-mutation-in-client` ESLint rule to prevent future React Query write usage. + +## 8) Next Steps for the Team + +- Continue migrating remaining `useMutation` usages to Server Actions. +- Ensure all new templates follow the ViewData-only rule. +- Use `NumberDisplay` and `DateDisplay` for all UI formatting. +