diff --git a/.github/workflows/ci-infrastructure.yml b/.github/workflows/ci-infrastructure.yml index 84a332c..202ce73 100644 --- a/.github/workflows/ci-infrastructure.yml +++ b/.github/workflows/ci-infrastructure.yml @@ -30,28 +30,31 @@ jobs: - name: Install dependencies run: npm ci - - name: Check required Supabase client env vars + - name: Check required Supabase client and smoke-test env vars run: | test -n "$VITE_SUPABASE_URL" test -n "$VITE_SUPABASE_ANON_KEY" test -n "$E2E_TEST_EMAIL" test -n "$E2E_TEST_PASSWORD" - - name: Smoke-test Supabase client startup + - name: Smoke-test Supabase client startup and runtime config run: | node --input-type=module -e "import { createServer } from 'vite'; const server = await createServer({ server: { middlewareMode: true }, optimizeDeps: { noDiscovery: true } }); try { await server.ssrLoadModule('/services/supabaseClient.ts'); console.log('Supabase client startup smoke test passed'); } finally { await server.close(); }" - - name: Type-check code + - name: Run client-visible database contract smoke check + run: npm run check:db-contract + + - name: Type-check app and tests run: npm run typecheck - - name: Run tests + - name: Run unit tests run: npm run test:run - - name: Build app + - name: Build production bundle run: npm run build - - name: Install Playwright Chromium + - name: Install Playwright browser dependencies run: npx playwright install --with-deps chromium - - name: Run E2E smoke tests + - name: Run browser smoke tests against live Supabase data run: npm run test:e2e diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7a6ee98..f868841 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -65,7 +65,7 @@ flowchart LR - `profiles` for app roles and user metadata - `members` for member records - `marks` for per-member attendance and scores -- `services/settings.ts` handles section-level settings. +- `services/settings.ts` handles section-level settings, updating the seeded `company` and `junior` rows in place. ## State Model diff --git a/App.tsx b/App.tsx index 09d967f..1f52199 100644 --- a/App.tsx +++ b/App.tsx @@ -145,8 +145,15 @@ const App: React.FC = () => { // Render main app content with header return ( - <> -
+ <> +
{renderMainContent()}
diff --git a/README.md b/README.md index 6e64afb..fe6a3cc 100644 --- a/README.md +++ b/README.md @@ -17,21 +17,25 @@ There is no in-repo Express server or Docker runtime. The app is built as a stat 1. Install dependencies with `npm install`. 2. Create a local `.env` from [`.env.example`](./.env.example). 3. Start the dev server with `npm run dev`. -4. Run `npm run typecheck`, `npm run test:run`, and `npm run build` before shipping changes. +4. Run `npm run check:db-contract`, `npm run typecheck`, `npm run test:run`, and `npm run build` before shipping changes. ## Testing +- `npm run check:db-contract` reads `.env` and `.env.local`, signs in with the smoke-test user, resolves `current_app_role()`, and confirms the seeded `settings` rows for `company` and `junior` are readable through the published client credentials. - `npm run test:run` runs the lean automated suite used by CI on every push and pull request. - `npm run test:coverage` reports coverage for the same suite. -- `npm run test:e2e` runs the browser smoke suite against a real Supabase-backed environment. It requires `E2E_TEST_EMAIL` and `E2E_TEST_PASSWORD`. -- `tests/e2e/` contains manual Supabase-backed smoke-test runbooks for auth, member CRUD, and marks workflows. +- `npm run test:e2e` reads `.env` and `.env.local` and runs the browser smoke suite against a real Supabase-backed environment. It requires `E2E_TEST_EMAIL` and `E2E_TEST_PASSWORD`. +- `tests/e2e/` contains manual Supabase-backed smoke-test runbooks for auth, section settings, member CRUD, and marks workflows. -The CI smoke suite expects the test account to have a valid app role and at least one member in the Company section so the weekly-marks save flow has real data to exercise. +The CI smoke suite expects the test account to have a valid app role, seeded `settings` rows for both sections, and at least one member in the Company section so the settings and weekly-marks save flows have real data to exercise. +It verifies the client-visible contract only; it does not prove every live RLS restriction without privileged Supabase inspection. ## Environment Variables - `VITE_SUPABASE_URL` - `VITE_SUPABASE_ANON_KEY` +- `E2E_TEST_EMAIL` +- `E2E_TEST_PASSWORD` ## Documentation diff --git a/components/AccountSettingsPage.tsx b/components/AccountSettingsPage.tsx index 60c7c23..7aa05a4 100644 --- a/components/AccountSettingsPage.tsx +++ b/components/AccountSettingsPage.tsx @@ -1,5 +1,3 @@ -"use client"; - import React, { useState } from 'react'; import { Section, ToastType } from '../types'; import * as supabaseAuth from '../services/supabaseAuth'; diff --git a/components/BoyMarksPage.tsx b/components/BoyMarksPage.tsx index 47e25f3..16f0c85 100644 --- a/components/BoyMarksPage.tsx +++ b/components/BoyMarksPage.tsx @@ -6,10 +6,14 @@ */ import React, { useState, useEffect, useCallback, useMemo } from 'react'; -import { fetchBoyById, updateBoy } from '../services/db'; import { Boy, Mark, Squad, Section, JuniorSquad, ToastType } from '../types'; import { TrashIcon, SaveIcon } from './Icons'; import { BoyMarksPageSkeleton } from './SkeletonLoaders'; +import { fetchBoyById, saveBoyMarks } from '../services/db'; +import { + areMarkListsEqual, + normalizeEditableMarksForSave, +} from './weeklyMarksSavePlan'; interface BoyMarksPageProps { boyId: string; @@ -70,8 +74,8 @@ const BoyMarksPage: React.FC = ({ boyId, refreshData, setHasU // Sort marks by date descending for display. boyData.marks.sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime()); setBoy(boyData); - // Create a deep copy of the marks for editing to avoid mutating the original state. - setEditedMarks(JSON.parse(JSON.stringify(boyData.marks))); + // Create a shallow copy of the marks for editing to avoid mutating the original state. + setEditedMarks(boyData.marks.map((mark) => ({ ...mark }))); } else { setError('Boy not found.'); } @@ -96,34 +100,9 @@ const BoyMarksPage: React.FC = ({ boyId, refreshData, setHasU useEffect(() => { if (boy) { // Create a canonical, sorted representation of the original marks to ensure consistent key order and data types for comparison. - const originalMarksCanonical = [...boy.marks] - .map(m => { - const cleanMark: any = { date: m.date, score: m.score }; - if (m.uniformScore !== undefined) cleanMark.uniformScore = m.uniformScore; - if (m.behaviourScore !== undefined) cleanMark.behaviourScore = m.behaviourScore; - return cleanMark; - }) - .sort((a, b) => a.date.localeCompare(b.date)); + const editedMarksCanonical = normalizeEditableMarksForSave(editedMarks, activeSection); - // Create a canonical, sorted representation of the edited marks. - // Empty string inputs are converted to 0, which mirrors the save logic. - const editedMarksCanonical = [...editedMarks] - .map(editableMark => { - if (isCompany || editableMark.uniformScore === undefined) { - // An empty string for a score is treated as 0 for comparison. - const score = editableMark.score === '' ? 0 : parseFloat(editableMark.score as string); // Use parseFloat - return { date: editableMark.date, score }; - } - // For Juniors, recalculate the total score from uniform and behaviour. - const uniformScore = editableMark.uniformScore === '' ? 0 : parseFloat(editableMark.uniformScore as string); // Use parseFloat - const behaviourScore = editableMark.behaviourScore === '' ? 0 : parseFloat(editableMark.behaviourScore as string); // Use parseFloat - const totalScore = Number(editableMark.score) < 0 ? -1 : uniformScore + behaviourScore; // Preserve absent status. - return { date: editableMark.date, score: totalScore, uniformScore, behaviourScore }; - }) - .sort((a, b) => a.date.localeCompare(b.date)); - - // Compare the stringified versions to check for differences. - setIsDirty(JSON.stringify(originalMarksCanonical) !== JSON.stringify(editedMarksCanonical)); + setIsDirty(!areMarkListsEqual(boy.marks, editedMarksCanonical)); } else { setIsDirty(false); } @@ -193,32 +172,26 @@ const BoyMarksPage: React.FC = ({ boyId, refreshData, setHasU const handleSaveChanges = async () => { if (!boy || !isDirty) return; + if (!boy.id) { + setError('Boy ID is required.'); + return; + } setIsSaving(true); setError(null); - // Convert the local `EditableMark[]` state back into the strict `Mark[]` type for saving. - const validMarks: Mark[] = editedMarks - .map(editableMark => { - if(isCompany || editableMark.uniformScore === undefined) { - // An empty string for a score should be saved as 0. - const score = editableMark.score === '' ? 0 : parseFloat(editableMark.score as string); // Use parseFloat - return { date: editableMark.date, score }; - } - // For Juniors, recalculate the total score from uniform and behaviour. - const uniformScore = editableMark.uniformScore === '' ? 0 : parseFloat(editableMark.uniformScore as string); // Use parseFloat - const behaviourScore = editableMark.behaviourScore === '' ? 0 : parseFloat(editableMark.behaviourScore as string); // Use parseFloat - const totalScore = Number(editableMark.score) < 0 ? -1 : uniformScore + behaviourScore; // Preserve absent status. - return { date: editableMark.date, score: totalScore, uniformScore, behaviourScore }; - }); - - const updatedBoyData = { ...boy, marks: validMarks }; + const nextMarks = normalizeEditableMarksForSave(editedMarks, activeSection); try { - await updateBoy(updatedBoyData, activeSection); - // Update local state to match the saved data. - updatedBoyData.marks.sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime()); - setBoy(updatedBoyData); - setEditedMarks(JSON.parse(JSON.stringify(updatedBoyData.marks))); + await saveBoyMarks(boy.id, activeSection, boy.marks, nextMarks); + + const savedBoy = await fetchBoyById(boy.id, activeSection); + if (!savedBoy) { + throw new Error('Failed to reload saved marks.'); + } + + savedBoy.marks.sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime()); + setBoy(savedBoy); + setEditedMarks(savedBoy.marks.map((mark) => ({ ...mark }))); showToast('Changes saved successfully!', 'success'); refreshData(); // Refresh data in the main App component. setIsDirty(false); diff --git a/components/DatePicker.tsx b/components/DatePicker.tsx index 858d850..276053b 100644 --- a/components/DatePicker.tsx +++ b/components/DatePicker.tsx @@ -1,8 +1,4 @@ -"use client"; - import React from 'react'; -// CalendarIcon is no longer needed as the custom button is removed. -// import { CalendarIcon } from './Icons'; interface DatePickerProps { value: string; // YYYY-MM-DD format @@ -19,38 +15,20 @@ const DatePicker: React.FC = ({ ariaLabel = "Select date", accentRingClass = "focus:ring-junior-blue focus:border-junior-blue" }) => { - // The inputRef and handleIconClick are no longer needed as showPicker() is removed. - // const inputRef = useRef(null); - // const handleIconClick = () => { - // if (inputRef.current && !disabled) { - // inputRef.current.showPicker(); // Programmatically open the date picker - // } - // }; + // The old inputRef/handleIconClick/showPicker path was removed because browsers blocked it inconsistently. return (
onChange(e.target.value)} disabled={disabled} className={`block w-full px-3 py-2 pr-3 bg-white border border-slate-300 rounded-md shadow-sm focus:outline-none sm:text-sm ${accentRingClass} disabled:bg-slate-100 disabled:text-slate-500 disabled:cursor-not-allowed`} aria-label={ariaLabel} /> - {/* The custom button to trigger the date picker is removed to avoid the cross-origin error. - Users can still click directly on the input field to open the native date picker. */} - {/* */}
); }; -export default DatePicker; \ No newline at end of file +export default DatePicker; diff --git a/components/Header.tsx b/components/Header.tsx index f31bc3f..cebac0e 100644 --- a/components/Header.tsx +++ b/components/Header.tsx @@ -7,8 +7,7 @@ import React, { useState, useRef, useEffect } from 'react'; import { MenuIcon, XIcon, CogIcon, UserCircleIcon, SwitchHorizontalIcon, LogOutIcon } from './Icons'; -import { Page, Section } from '../types'; -import { useAuthAndRole } from '../hooks/useAuthAndRole'; +import { AppUser, Page, Section, UserRole } from '../types'; interface HeaderProps { /** Function to change the current view/page. */ @@ -19,10 +18,13 @@ interface HeaderProps { activeSection: Section; /** Callback function to handle switching between sections. */ onSwitchSection: () => void; + /** The current authenticated user from the app root. */ + currentUser: AppUser | null; + /** The current authenticated user's application role. */ + userRole: UserRole | null; } -const Header: React.FC = ({ setView, onSignOut, activeSection, onSwitchSection }) => { - const { currentUser: user, userRole } = useAuthAndRole(); +const Header: React.FC = ({ setView, onSignOut, activeSection, onSwitchSection, currentUser, userRole }) => { // State to manage the visibility of the mobile menu. const [isMenuOpen, setIsMenuOpen] = useState(false); // State to manage the visibility of the desktop profile dropdown. @@ -99,7 +101,7 @@ const Header: React.FC = ({ setView, onSignOut, activeSection, onSw {/* Desktop Menu */}
- {user && ( + {currentUser && ( <> @@ -126,7 +128,7 @@ const Header: React.FC = ({ setView, onSignOut, activeSection, onSw {isProfileMenuOpen && (
-

{user?.email || user?.id}

+

{currentUser.email || currentUser.id}

@@ -173,7 +175,7 @@ const Header: React.FC = ({ setView, onSignOut, activeSection, onSw )} {/* Profile-related options in mobile menu */}
-

{user?.email || user?.id}

+

{currentUser.email || currentUser.id}

diff --git a/components/HomePage.tsx b/components/HomePage.tsx index 2f249fb..1cd0d26 100644 --- a/components/HomePage.tsx +++ b/components/HomePage.tsx @@ -1,5 +1,3 @@ -"use client"; - import React, { useState, useMemo, useEffect } from 'react'; import { Boy, Squad, View, Section, JuniorSquad, ToastType, SortByType, SchoolYear, JuniorYear } from '../types'; import Modal from './Modal'; diff --git a/components/SettingsPage.tsx b/components/SettingsPage.tsx index d6eeda9..981b3e1 100644 --- a/components/SettingsPage.tsx +++ b/components/SettingsPage.tsx @@ -1,5 +1,3 @@ -"use client"; - import React, { useState, useEffect } from 'react'; import { Section, SectionSettings, ToastType, UserRole } from '../types'; import { saveSettings } from '../services/settings'; diff --git a/components/WeeklyMarksPage.tsx b/components/WeeklyMarksPage.tsx index 247fd67..50f0d6e 100644 --- a/components/WeeklyMarksPage.tsx +++ b/components/WeeklyMarksPage.tsx @@ -1,14 +1,16 @@ -import React, { useState, useEffect, useMemo, useCallback } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import { Boy, Squad, Section, JuniorSquad, SectionSettings, ToastType } from '../types'; -import { updateBoy } from '../services/db'; +import { saveWeeklyMarksSnapshot } from '../services/db'; import { SaveIcon, LockClosedIcon, LockOpenIcon, ClipboardDocumentListIcon, ChevronLeftIcon, ChevronRightIcon } from './Icons'; import DatePicker from './DatePicker'; // Import the new DatePicker component +import Modal from './Modal'; import { getNearestMeetingDay } from './weeklyMarksDates'; import { - buildUpdatedMarksForBoy, CompanyMarkState, JuniorMarkState, + buildWeeklyMarksSnapshot, } from './weeklyMarksSavePlan'; +import { shouldConfirmWeeklyMarksDateChange } from './weeklyMarksDateChange'; interface WeeklyMarksPageProps { boys: Boy[]; @@ -34,15 +36,19 @@ const JUNIOR_SQUAD_COLORS: Record = { 4: 'text-yellow-600', }; +const getTodayString = () => new Date().toISOString().split('T')[0]; + const WeeklyMarksPage: React.FC = ({ boys, refreshData, setHasUnsavedChanges, activeSection, settings, showToast }) => { // --- STATE MANAGEMENT --- const [selectedDate, setSelectedDate] = useState(''); const [marks, setMarks] = useState>({}); const [attendance, setAttendance] = useState>({}); const [isSaving, setIsSaving] = useState(false); - const [isDirty, setIsDirty] = useState(false); // Tracks if there are unsaved changes. + const [today, setToday] = useState(getTodayString); const [isLocked, setIsLocked] = useState(false); // Read-only state for past dates. const [markErrors, setMarkErrors] = useState>({}); + const [pendingDate, setPendingDate] = useState(''); + const [isDateChangeConfirmOpen, setIsDateChangeConfirmOpen] = useState(false); const isCompany = activeSection === 'company'; @@ -56,15 +62,34 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se setSelectedDate(getNearestMeetingDay(settings.meetingDay)); } }, [settings, selectedDate]); + + useEffect(() => { + const refreshToday = () => { + setToday(currentToday => { + const nextToday = getTodayString(); + return currentToday === nextToday ? currentToday : nextToday; + }); + }; + + refreshToday(); + window.addEventListener('focus', refreshToday); + document.addEventListener('visibilitychange', refreshToday); + const intervalId = window.setInterval(refreshToday, 60_000); + + return () => { + window.removeEventListener('focus', refreshToday); + document.removeEventListener('visibilitychange', refreshToday); + window.clearInterval(intervalId); + }; + }, []); /** * EFFECT: Automatically locks the page if the selected date is in the past. */ useEffect(() => { if (!selectedDate) return; - const todayString = new Date().toISOString().split('T')[0]; - setIsLocked(selectedDate < todayString); - }, [selectedDate]); + setIsLocked(selectedDate < today); + }, [selectedDate, today]); /** * EFFECT: Populates the marks and attendance state based on the selected date and boys data. @@ -97,13 +122,28 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se }); setMarks(newMarks); setAttendance(newAttendance); - setIsDirty(false); // Reset dirty state on date change. setMarkErrors({}); // Clear errors on date change. }, [selectedDate, boys, isCompany]); + const pendingSnapshot = useMemo( + () => + selectedDate + ? buildWeeklyMarksSnapshot({ + boys, + selectedDate, + attendance, + marks, + activeSection, + }) + : [], + [boys, selectedDate, attendance, marks, activeSection], + ); + + const hasPendingChanges = pendingSnapshot.length > 0; + useEffect(() => { - setHasUnsavedChanges(isDirty); - }, [isDirty, setHasUnsavedChanges]); + setHasUnsavedChanges(hasPendingChanges); + }, [hasPendingChanges, setHasUnsavedChanges]); useEffect(() => { return () => { @@ -140,7 +180,6 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se return { ...prev, [boyId]: { ...currentMark, [type]: scoreStr } }; } }); - setIsDirty(true); } }; @@ -172,7 +211,6 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se setMarks(prev => ({ ...prev, [boyId]: presentMark ? { uniform: markForDate.uniformScore ?? '', behaviour: markForDate.behaviourScore ?? '' } : { uniform: '', behaviour: '' } })); } } - setIsDirty(true); }; const handleClearAllMarks = () => { @@ -191,29 +229,53 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se }); setMarks(newMarks); setAttendance(newAttendance); - setIsDirty(true); setMarkErrors({}); // Clear errors showToast('All marks cleared for present members.', 'info'); }; + const requestDateChange = (nextDate: string) => { + if (!nextDate || nextDate === selectedDate) { + return; + } + + if (shouldConfirmWeeklyMarksDateChange({ + currentDate: selectedDate, + nextDate, + isDirty: hasPendingChanges, + })) { + setPendingDate(nextDate); + setIsDateChangeConfirmOpen(true); + return; + } + + setSelectedDate(nextDate); + }; + + const confirmDateChange = () => { + if (pendingDate) { + setSelectedDate(pendingDate); + } + setPendingDate(''); + setIsDateChangeConfirmOpen(false); + }; + + const cancelDateChange = () => { + setPendingDate(''); + setIsDateChangeConfirmOpen(false); + }; + const handlePreviousWeek = () => { const currentDate = new Date(selectedDate + 'T00:00:00'); currentDate.setDate(currentDate.getDate() - 7); - setSelectedDate(currentDate.toISOString().split('T')[0]); - setIsDirty(true); // Mark as dirty to prompt save if date changes + requestDateChange(currentDate.toISOString().split('T')[0]); }; const handleNextWeek = () => { const currentDate = new Date(selectedDate + 'T00:00:00'); currentDate.setDate(currentDate.getDate() + 7); - setSelectedDate(currentDate.toISOString().split('T')[0]); - setIsDirty(true); // Mark as dirty to prompt save if date changes + requestDateChange(currentDate.toISOString().split('T')[0]); }; - /** - * Core save logic. This function processes all local state changes, determines which boys - * need updating, and bundles these updates into a single transaction. - */ const handleSaveMarks = async () => { // Check for any active errors before saving const hasErrors = Object.values(markErrors).some(boyErrors => @@ -225,32 +287,17 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se return; } - setIsSaving(true); - - // Map over all boys to create an array of update promises. - const updates = boys.map(boy => { - if (!boy.id) return Promise.resolve(null); - - const updatedMarks = buildUpdatedMarksForBoy({ - boy, - selectedDate, - attendanceStatus: attendance[boy.id], - markState: marks[boy.id], - activeSection, - }); - - if (!updatedMarks) { - return Promise.resolve(null); - } + if (!hasPendingChanges) { + showToast('No changes to save.', 'info'); + return; + } - return updateBoy({ ...boy, marks: updatedMarks }, activeSection); - }); + setIsSaving(true); try { - await Promise.all(updates); + await saveWeeklyMarksSnapshot(activeSection, selectedDate, pendingSnapshot); showToast('Marks saved successfully!', 'success'); refreshData(); - setIsDirty(false); } catch (error) { console.error('Failed to save marks', error); showToast('Failed to save marks. Please try again.', 'error'); @@ -260,8 +307,6 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se }; // --- MEMOIZED COMPUTATIONS for rendering --- - const today = useMemo(() => new Date().toISOString().split('T')[0], []); - const boysBySquad = useMemo(() => { const grouped: Record = {}; boys.forEach(boy => { @@ -315,7 +360,7 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se const total = squadBoys.length; if (total === 0) { stats[squad] = { present: 0, total: 0, percentage: 0 }; - continue;; + continue; } const present = squadBoys.filter(boy => boy.id && attendance[boy.id] === 'present').length; const percentage = Math.round((present / total) * 100); @@ -369,7 +414,7 @@ const WeeklyMarksPage: React.FC = ({ boys, refreshData, se = ({ boys, refreshData, se
{/* Floating Action Button for saving */} - {isDirty && ( + {hasPendingChanges && ( - )} + )} + + +
+

+ Changing the date will discard the unsaved changes on this sheet. Continue? +

+
+ + +
+
+
); }; diff --git a/components/weeklyMarksDateChange.test.ts b/components/weeklyMarksDateChange.test.ts new file mode 100644 index 0000000..9712519 --- /dev/null +++ b/components/weeklyMarksDateChange.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest'; + +import { shouldConfirmWeeklyMarksDateChange } from './weeklyMarksDateChange'; + +describe('shouldConfirmWeeklyMarksDateChange', () => { + it('requires confirmation when the sheet is dirty and the date changes', () => { + expect( + shouldConfirmWeeklyMarksDateChange({ + currentDate: '2026-03-20', + nextDate: '2026-03-27', + isDirty: true, + }), + ).toBe(true); + }); + + it('does not require confirmation when the date stays the same', () => { + expect( + shouldConfirmWeeklyMarksDateChange({ + currentDate: '2026-03-20', + nextDate: '2026-03-20', + isDirty: true, + }), + ).toBe(false); + }); + + it('does not require confirmation when the sheet is clean', () => { + expect( + shouldConfirmWeeklyMarksDateChange({ + currentDate: '2026-03-20', + nextDate: '2026-03-27', + isDirty: false, + }), + ).toBe(false); + }); +}); diff --git a/components/weeklyMarksDateChange.ts b/components/weeklyMarksDateChange.ts new file mode 100644 index 0000000..eccf299 --- /dev/null +++ b/components/weeklyMarksDateChange.ts @@ -0,0 +1,11 @@ +export type WeeklyMarksDateChangeRequest = { + currentDate: string; + nextDate: string; + isDirty: boolean; +}; + +export const shouldConfirmWeeklyMarksDateChange = ({ + currentDate, + nextDate, + isDirty, +}: WeeklyMarksDateChangeRequest) => isDirty && currentDate !== nextDate; diff --git a/components/weeklyMarksSavePlan.test.ts b/components/weeklyMarksSavePlan.test.ts index ddd5f92..a8a678d 100644 --- a/components/weeklyMarksSavePlan.test.ts +++ b/components/weeklyMarksSavePlan.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; import type { Boy } from '../types'; -import { buildUpdatedMarksForBoy } from './weeklyMarksSavePlan'; +import { + areMarkListsEqual, + buildWeeklyMarksSnapshot, + normalizeEditableMarksForSave, +} from './weeklyMarksSavePlan'; const baseBoy: Boy = { id: 'member-1', @@ -12,87 +16,200 @@ const baseBoy: Boy = { isSquadLeader: false, }; -describe('buildUpdatedMarksForBoy', () => { - it('creates an absent mark when attendance is toggled off', () => { +describe('buildWeeklyMarksSnapshot', () => { + it('creates an absent mark entry when attendance is toggled off', () => { expect( - buildUpdatedMarksForBoy({ - boy: baseBoy, + buildWeeklyMarksSnapshot({ + boys: [baseBoy], selectedDate: '2026-03-27', - attendanceStatus: 'absent', - markState: undefined, + attendance: { 'member-1': 'absent' }, + marks: {}, activeSection: 'company', }), - ).toEqual([{ date: '2026-03-27', score: -1 }]); + ).toEqual([{ memberId: 'member-1', mark: { date: '2026-03-27', score: -1 } }]); }); - it('skips company updates when no score has been entered', () => { + it('creates a delete entry when a saved company mark is cleared', () => { expect( - buildUpdatedMarksForBoy({ - boy: baseBoy, + buildWeeklyMarksSnapshot({ + boys: [{ ...baseBoy, marks: [{ date: '2026-03-27', score: 9 }] }], selectedDate: '2026-03-27', - attendanceStatus: 'present', - markState: '', + attendance: { 'member-1': 'present' }, + marks: { 'member-1': '' }, activeSection: 'company', }), - ).toBeNull(); + ).toEqual([{ memberId: 'member-1', mark: null }]); }); - it('normalizes company updates back to a single score mark', () => { - const boy: Boy = { - ...baseBoy, - marks: [{ date: '2026-03-27', score: 9, uniformScore: 5, behaviourScore: 4 }], - }; - + it('builds a changed-entry snapshot for the selected date', () => { expect( - buildUpdatedMarksForBoy({ - boy, + buildWeeklyMarksSnapshot({ + boys: [ + { ...baseBoy, marks: [{ date: '2026-03-27', score: 8 }] }, + { + ...baseBoy, + id: 'member-2', + name: 'Ben', + marks: [{ date: '2026-03-27', score: 6 }], + }, + { + ...baseBoy, + id: 'member-3', + name: 'Chris', + marks: [], + }, + ], selectedDate: '2026-03-27', - attendanceStatus: 'present', - markState: '9', + attendance: { + 'member-1': 'present', + 'member-2': 'present', + 'member-3': 'absent', + }, + marks: { + 'member-1': 9, + 'member-2': '', + 'member-3': -1, + }, activeSection: 'company', }), - ).toEqual([{ date: '2026-03-27', score: 9 }]); + ).toEqual([ + { memberId: 'member-1', mark: { date: '2026-03-27', score: 9 } }, + { memberId: 'member-2', mark: null }, + { memberId: 'member-3', mark: { date: '2026-03-27', score: -1 } }, + ]); }); - it('builds junior totals from partial entries', () => { + it('keeps absent state in the snapshot', () => { expect( - buildUpdatedMarksForBoy({ - boy: baseBoy, + buildWeeklyMarksSnapshot({ + boys: [{ ...baseBoy, marks: [{ date: '2026-03-27', score: -1 }] }], selectedDate: '2026-03-27', - attendanceStatus: 'present', - markState: { uniform: 4, behaviour: '' }, - activeSection: 'junior', + attendance: { 'member-1': 'present' }, + marks: { 'member-1': '' }, + activeSection: 'company', }), - ).toEqual([{ date: '2026-03-27', score: 4, uniformScore: 4, behaviourScore: 0 }]); + ).toEqual([{ memberId: 'member-1', mark: null }]); }); - it('skips junior updates when the existing mark is unchanged', () => { - const boy: Boy = { - ...baseBoy, - year: 'P7', - marks: [{ date: '2026-03-27', score: 7, uniformScore: 5, behaviourScore: 2 }], - }; - + it('builds junior totals from partial entries', () => { expect( - buildUpdatedMarksForBoy({ - boy, + buildWeeklyMarksSnapshot({ + boys: [{ ...baseBoy, year: 'P6' }], selectedDate: '2026-03-27', - attendanceStatus: 'present', - markState: { uniform: 5, behaviour: 2 }, + attendance: { 'member-1': 'present' }, + marks: { 'member-1': { uniform: 4, behaviour: '' } }, activeSection: 'junior', }), - ).toBeNull(); + ).toEqual([ + { + memberId: 'member-1', + mark: { date: '2026-03-27', score: 4, uniformScore: 4, behaviourScore: 0 }, + }, + ]); }); - it('skips junior updates when a score cannot be parsed', () => { + it('omits unchanged weekly rows from the snapshot', () => { expect( - buildUpdatedMarksForBoy({ - boy: { ...baseBoy, year: 'P6' }, + buildWeeklyMarksSnapshot({ + boys: [{ ...baseBoy, marks: [{ date: '2026-03-27', score: -1 }] }], selectedDate: '2026-03-27', - attendanceStatus: 'present', - markState: { uniform: 'oops' as never, behaviour: 2 }, - activeSection: 'junior', + attendance: { 'member-1': 'absent' }, + marks: {}, + activeSection: 'company', }), - ).toBeNull(); + ).toEqual([]); + }); +}); + +describe('normalizeEditableMarksForSave', () => { + it('normalizes editable company marks', () => { + expect( + normalizeEditableMarksForSave([{ date: '2026-03-27', score: 8 }], 'company'), + ).toEqual([{ date: '2026-03-27', score: 8 }]); + }); + + it('preserves absent junior marks', () => { + expect( + normalizeEditableMarksForSave( + [{ date: '2026-03-27', score: -1, uniformScore: '', behaviourScore: '' }], + 'junior', + ), + ).toEqual([{ date: '2026-03-27', score: -1 }]); + }); + + it('normalizes junior component scores into numeric totals', () => { + expect( + normalizeEditableMarksForSave( + [{ date: '2026-03-27', score: 5, uniformScore: 2, behaviourScore: 3 }], + 'junior', + ), + ).toEqual([ + { date: '2026-03-27', score: 5, uniformScore: 2, behaviourScore: 3 }, + ]); + }); + + it('normalizes empty junior component scores to zero', () => { + expect( + normalizeEditableMarksForSave( + [{ date: '2026-03-27', score: 0, uniformScore: '', behaviourScore: '' }], + 'junior', + ), + ).toEqual([ + { date: '2026-03-27', score: 0, uniformScore: 0, behaviourScore: 0 }, + ]); + }); +}); + +describe('areMarkListsEqual', () => { + it('treats reordered equivalent lists as equal', () => { + expect( + areMarkListsEqual( + [ + { date: '2026-03-27', score: 8 }, + { date: '2026-03-20', score: -1 }, + ], + [ + { date: '2026-03-20', score: -1 }, + { date: '2026-03-27', score: 8 }, + ], + ), + ).toBe(true); + }); + + it('returns false when list lengths differ', () => { + expect( + areMarkListsEqual( + [{ date: '2026-03-27', score: 8 }], + [ + { date: '2026-03-20', score: -1 }, + { date: '2026-03-27', score: 8 }, + ], + ), + ).toBe(false); + }); + + it('returns false when scores differ for the same date', () => { + expect( + areMarkListsEqual( + [{ date: '2026-03-27', score: 5 }], + [{ date: '2026-03-27', score: 6 }], + ), + ).toBe(false); + }); + + it('compares junior mark components as part of equality', () => { + expect( + areMarkListsEqual( + [{ date: '2026-03-27', score: 5, uniformScore: 2, behaviourScore: 3 }], + [{ date: '2026-03-27', score: 5, uniformScore: 2, behaviourScore: 3 }], + ), + ).toBe(true); + + expect( + areMarkListsEqual( + [{ date: '2026-03-27', score: 5, uniformScore: 2, behaviourScore: 3 }], + [{ date: '2026-03-27', score: 5, uniformScore: 1, behaviourScore: 4 }], + ), + ).toBe(false); }); }); diff --git a/components/weeklyMarksSavePlan.ts b/components/weeklyMarksSavePlan.ts index fca90d3..89594dc 100644 --- a/components/weeklyMarksSavePlan.ts +++ b/components/weeklyMarksSavePlan.ts @@ -1,71 +1,71 @@ -import { Boy, Mark, Section } from '../types'; +import { Boy, Mark, Section, WeeklyMarksSnapshotEntry } from '../types'; export type JuniorMarkState = { uniform: number | ''; behaviour: number | '' }; export type CompanyMarkState = number | string; export type WeeklyMarkState = CompanyMarkState | JuniorMarkState | undefined; export type AttendanceStatus = 'present' | 'absent' | undefined; - -type BuildWeeklyMarkUpdateArgs = { - boy: Boy; - selectedDate: string; - attendanceStatus: AttendanceStatus; - markState: WeeklyMarkState; - activeSection: Section; +type EditableMarkLike = { + date: string; + score: number | ''; + uniformScore?: number | ''; + behaviourScore?: number | ''; }; -export const buildUpdatedMarksForBoy = ({ - boy, - selectedDate, - attendanceStatus, - markState, - activeSection, -}: BuildWeeklyMarkUpdateArgs): Mark[] | null => { - const markIndex = boy.marks.findIndex((mark) => mark.date === selectedDate); - const updatedMarks = [...boy.marks]; +const sortMarksByDate = (marks: Mark[]) => + [...marks].sort((a, b) => a.date.localeCompare(b.date)); - if (attendanceStatus === 'absent') { - if (markIndex > -1) { - if (updatedMarks[markIndex].score !== -1) { - updatedMarks[markIndex] = { date: selectedDate, score: -1 }; - return updatedMarks; - } +const marksEqual = (left: Mark | null, right: Mark | null) => { + if (!left || !right) { + return left === right; + } - return null; - } + return ( + left.date === right.date && + left.score === right.score && + left.uniformScore === right.uniformScore && + left.behaviourScore === right.behaviourScore + ); +}; - updatedMarks.push({ date: selectedDate, score: -1 }); - return updatedMarks; +const normalizeCompanySnapshotMark = ( + selectedDate: string, + attendanceStatus: AttendanceStatus, + markState: WeeklyMarkState, +) => { + if (attendanceStatus === 'absent') { + return { date: selectedDate, score: -1 }; } - if (activeSection === 'company') { - if (markState === '' || markState === undefined) { - return null; - } - - if (typeof markState !== 'string' && typeof markState !== 'number') { - return null; - } + if (markState === '' || markState === undefined) { + return null; + } - const parsedScore = typeof markState === 'string' ? parseFloat(markState) : markState; + if (typeof markState !== 'string' && typeof markState !== 'number') { + return null; + } - if (Number.isNaN(parsedScore)) { - return null; - } + const parsedScore = typeof markState === 'string' ? parseFloat(markState) : markState; - if (markIndex > -1) { - if (updatedMarks[markIndex].score !== parsedScore || updatedMarks[markIndex].uniformScore !== undefined) { - updatedMarks[markIndex] = { date: selectedDate, score: parsedScore }; - return updatedMarks; - } + if (Number.isNaN(parsedScore)) { + return null; + } - return null; - } + return { date: selectedDate, score: parsedScore }; +}; - updatedMarks.push({ date: selectedDate, score: parsedScore }); - return updatedMarks; +const normalizeJuniorSnapshotMark = ( + selectedDate: string, + attendanceStatus: AttendanceStatus, + markState: WeeklyMarkState, +) => { + if (attendanceStatus === 'absent') { + return { date: selectedDate, score: -1 }; } - const juniorState = (markState as JuniorMarkState | undefined) ?? { uniform: '', behaviour: '' }; + const juniorState = + typeof markState === 'object' && markState !== null && 'uniform' in markState && 'behaviour' in markState + ? markState + : { uniform: '', behaviour: '' }; const noUniformScore = juniorState.uniform === '' || juniorState.uniform === undefined; const noBehaviourScore = juniorState.behaviour === '' || juniorState.behaviour === undefined; @@ -80,34 +80,78 @@ export const buildUpdatedMarksForBoy = ({ return null; } - const finalScore = uniformScore + behaviourScore; - - if (markIndex > -1) { - const existingMark = updatedMarks[markIndex]; - - if ( - existingMark.score !== finalScore || - existingMark.uniformScore !== uniformScore || - existingMark.behaviourScore !== behaviourScore - ) { - updatedMarks[markIndex] = { - date: selectedDate, - score: finalScore, - uniformScore, - behaviourScore, - }; - return updatedMarks; + return { + date: selectedDate, + score: uniformScore + behaviourScore, + uniformScore, + behaviourScore, + }; +}; + +export const normalizeEditableMarksForSave = ( + editedMarks: EditableMarkLike[], + activeSection: Section, +): Mark[] => { + const normalizedMarks = editedMarks.map((editableMark) => { + if (activeSection === 'company' || editableMark.uniformScore === undefined) { + const score = editableMark.score === '' ? 0 : parseFloat(String(editableMark.score)); + return { date: editableMark.date, score }; } - return null; + if (Number(editableMark.score) < 0) { + return { date: editableMark.date, score: -1 }; + } + + const uniformScore = editableMark.uniformScore === '' ? 0 : parseFloat(String(editableMark.uniformScore)); + const behaviourScore = editableMark.behaviourScore === '' ? 0 : parseFloat(String(editableMark.behaviourScore)); + const score = uniformScore + behaviourScore; + return { date: editableMark.date, score, uniformScore, behaviourScore }; + }); + + return sortMarksByDate(normalizedMarks); +}; + +export const areMarkListsEqual = (left: Mark[], right: Mark[]) => { + const normalizedLeft = sortMarksByDate(left); + const normalizedRight = sortMarksByDate(right); + + if (normalizedLeft.length !== normalizedRight.length) { + return false; } - updatedMarks.push({ - date: selectedDate, - score: finalScore, - uniformScore, - behaviourScore, + return normalizedLeft.every((mark, index) => marksEqual(mark, normalizedRight[index])); +}; + +export const buildWeeklyMarksSnapshot = ({ + boys, + selectedDate, + attendance, + marks, + activeSection, +}: { + boys: Boy[]; + selectedDate: string; + attendance: Record; + marks: Record; + activeSection: Section; +}): WeeklyMarksSnapshotEntry[] => { + const snapshot: WeeklyMarksSnapshotEntry[] = []; + + boys.forEach((boy) => { + if (!boy.id) { + return; + } + + const desiredMark = + activeSection === 'company' + ? normalizeCompanySnapshotMark(selectedDate, attendance[boy.id], marks[boy.id]) + : normalizeJuniorSnapshotMark(selectedDate, attendance[boy.id], marks[boy.id]); + + const existingMark = boy.marks.find((mark) => mark.date === selectedDate) || null; + if (!marksEqual(existingMark, desiredMark)) { + snapshot.push({ memberId: boy.id, mark: desiredMark }); + } }); - return updatedMarks; + return snapshot; }; diff --git a/docs/03-getting-started.md b/docs/03-getting-started.md index cf7ae36..5a0f21a 100644 --- a/docs/03-getting-started.md +++ b/docs/03-getting-started.md @@ -23,7 +23,7 @@ VITE_SUPABASE_URL="https://.supabase.co" VITE_SUPABASE_ANON_KEY="" ``` -Optional browser smoke-test credentials for `npm run test:e2e`: +Smoke-test credentials for `npm run check:db-contract` and `npm run test:e2e`: ```bash E2E_TEST_EMAIL="" @@ -40,6 +40,8 @@ The live app expects these tables to exist: - `marks` For local development, create users manually in Supabase Auth and make sure each user has a row in `profiles` with a valid `role` such as `admin`, `captain`, or `officer`. +Seed `settings` with one row for `company` and one row for `junior` before running the app; section settings are updated in place and are not created on demand. +The Playwright smoke suite depends on those seeded rows so it can verify settings writes and restore the original value after each run. New-user handover material lives in [`docs/user-guide.md`](./user-guide.md). @@ -54,6 +56,7 @@ Open the printed local URL, usually `http://localhost:5173`. ## 5. Pre-Ship Checks ```bash +npm run check:db-contract npm run typecheck npm run test:coverage npm run build @@ -61,4 +64,8 @@ npm run build `npm run test:run` is the same automated suite CI runs on each push and pull request. The suite is intentionally small and focuses on business-critical logic rather than browser automation. -`npm run test:e2e` runs the small Playwright smoke suite. It uses a real browser and real Supabase auth, so it requires a dedicated test user with a valid role and at least one Company-section member already present. +`npm run check:db-contract` is the fast live-backend smoke check. It reads `.env` and `.env.local`, requires `E2E_TEST_EMAIL` and `E2E_TEST_PASSWORD`, signs in with that test user, verifies `current_app_role()` resolves to a valid app role, and confirms the seeded `settings` rows for `company` and `junior` are readable through the published client credentials. + +`npm run test:e2e` also reads `.env` and `.env.local` and uses the same `E2E_TEST_*` credentials. It uses a real browser and real Supabase auth, so it requires a dedicated test user with a valid role, seeded `settings` rows, and at least one Company-section member already present. + +Neither check proves the entire live RLS policy graph. They confirm only the client-visible contract the SPA depends on. diff --git a/docs/04-deployment.md b/docs/04-deployment.md index 7e160b1..3773de0 100644 --- a/docs/04-deployment.md +++ b/docs/04-deployment.md @@ -30,6 +30,9 @@ In Supabase Auth URL configuration: 1. Confirm Vercel env vars are present for the target environment. 2. Deploy from the main branch or the intended release branch. -3. Verify sign-in works for manually provisioned users. -4. Verify the production users have the expected roles in `profiles`. -5. Smoke-test member CRUD, marks entry, settings, and dashboard. +3. Run `npm run check:db-contract` against the target environment credentials or confirm the CI run passed with the intended deployment inputs. +4. Verify sign-in works for manually provisioned users. +5. Verify the production users have the expected roles in `profiles`. +6. Verify `settings` has seeded rows for both `company` and `junior`. +7. Smoke-test auth, section settings persistence, member CRUD, marks entry, and dashboard behavior against the live backend. +8. Treat CI smoke results as client-contract checks only; use Supabase inspection for live RLS policy verification. diff --git a/docs/06-data-and-services.md b/docs/06-data-and-services.md index cf1d183..20da0bf 100644 --- a/docs/06-data-and-services.md +++ b/docs/06-data-and-services.md @@ -7,14 +7,14 @@ This document describes how the app talks to Supabase. - `services/supabaseClient.ts`: shared Supabase client - `services/supabaseAuth.ts`: sign-in, sign-out, password update, auth subscription - `services/db.ts`: members, marks, profiles, and role-guardrail helpers -- `services/settings.ts`: section settings +- `services/settings.ts`: section settings for the seeded `company` and `junior` rows ## Live Table Mapping The current app talks to these tables: - `profiles`: email and application role -- `settings`: section-level settings +- `settings`: section-level settings, one seeded row per section - `members`: core member records - `marks`: normalized attendance and score rows @@ -29,4 +29,5 @@ The current app talks to these tables: - The UI-facing `Boy` model is assembled from `members` and `marks`. - Role information is loaded from `profiles`, not from a separate `user_roles` table. +- Section settings are updated in place; missing `settings` rows are a bootstrap problem, not a normal runtime case. - The active UI is limited to member management, marks entry, dashboard reporting, and section settings. diff --git a/docs/09-database-and-migrations.md b/docs/09-database-and-migrations.md index 3e67512..22b47c0 100644 --- a/docs/09-database-and-migrations.md +++ b/docs/09-database-and-migrations.md @@ -4,7 +4,7 @@ This repo relies on a live Supabase project as the database source of truth. ## Verified Current Shape -Verified on 2026-03-21: +Verified on 2026-03-22: - Tables: `profiles`, `settings`, `members`, `marks`, `invite_codes`, `audit_logs` - RLS: enabled on all of them @@ -12,9 +12,10 @@ Verified on 2026-03-21: The current app only depends on `profiles`, `settings`, `members`, and `marks`. The live project also contains legacy invite-code and audit-log objects, but they are outside the current app surface. -Latest migration visible in the live project at the time of verification: +Latest migrations visible in the live project at the time of verification: -- `20260320190925 repair_live_schema_for_app_compatibility_v2` +- `20260322185938 replace_transactional_mark_write_rpcs` +- `20260322192041 fix_save_weekly_marks_snapshot_member_alias` ## Workflow @@ -22,6 +23,10 @@ Latest migration visible in the live project at the time of verification: - Inspect schema and policies with Supabase MCP tools before making assumptions. - Apply schema changes through Supabase migrations or MCP-driven database changes, not ad-hoc dashboard edits. - Update app code and docs in the same change when table names, functions, or permissions change. +- Keep `members`, `marks`, and `settings` policies tied to valid app roles from `profiles`, not merely `auth.role() = 'authenticated'`. +- Keep one seeded `settings` row per section and treat missing rows as a bootstrap error that should be corrected, not created lazily from the browser. +- `npm run check:db-contract` is the fast live-backend check for the client contract: sign-in, `current_app_role()`, and the seeded `settings` rows for `company` and `junior`. +- CI and browser smoke tests can validate the client contract against live data, but they cannot prove live RLS policy shape without privileged Supabase inspection. - The current app no longer exposes invite-code provisioning, recovery, or audit-log flows. ## Important Historical Note diff --git a/docs/10-database-security-model.md b/docs/10-database-security-model.md index cf68c28..fe1a162 100644 --- a/docs/10-database-security-model.md +++ b/docs/10-database-security-model.md @@ -16,6 +16,8 @@ The live database also retains legacy invite-code and audit-log objects for comp - Browser code only receives public client credentials. - Authorization must be enforced in Supabase, not in React components. - App roles are derived from `profiles`. +- Access to `members`, `marks`, and `settings` requires a valid app role from `profiles`; authenticated Supabase users without a matching profile row should not be able to use core app tables. +- `npm run check:db-contract` and the Playwright smoke suite can catch broken client assumptions, missing seeded rows, and failed writes, but they are not a substitute for inspecting live RLS policies. - Manual account provisioning is the supported path; the UI no longer exposes invite-code signup or recovery flows. ## Role Model @@ -31,6 +33,7 @@ The UI uses those roles to shape workflows, but the database remains the enforce ## Sensitive Areas - `profiles` controls application access +- `settings` is seeded with one row for `company` and one row for `junior`, and settings updates modify those rows in place - `invite_codes` and `audit_logs` are legacy history data and are not written by the current app Changes that affect any of those areas should be treated as security-sensitive and reflected in both code and docs. diff --git a/hooks/useAppData.ts b/hooks/useAppData.ts index fa041d7..fbf07b8 100644 --- a/hooks/useAppData.ts +++ b/hooks/useAppData.ts @@ -1,5 +1,3 @@ -"use client"; - import { useState, useEffect, useCallback } from 'react'; import { fetchBoys } from '../services/db'; import { getSettings } from '../services/settings'; @@ -65,4 +63,4 @@ export const useAppData = ( }, [activeSection, currentUser, loadDataAndSettings]); return { boys, settings, dataLoading, dataError, refreshData, setSettings }; -}; \ No newline at end of file +}; diff --git a/hooks/useAuthAndRole.ts b/hooks/useAuthAndRole.ts index 8715adb..a133e7f 100644 --- a/hooks/useAuthAndRole.ts +++ b/hooks/useAuthAndRole.ts @@ -1,5 +1,3 @@ -"use client"; - import { useState, useEffect, useCallback, useRef } from 'react'; import { subscribeToAuth, signOut as supabaseSignOut, getCurrentUser } from '../services/supabaseAuth'; import { supabase } from '../services/supabaseClient'; @@ -107,18 +105,8 @@ export const useAuthAndRole = () => { setAuthLoading(false); }); - const handleUserRoleRefresh = (event: Event) => { - const customEvent = event as CustomEvent; - if (currentUserRef.current && customEvent.detail.uid === currentUserRef.current.id) { - loadUserRole(currentUserRef.current); - } - }; - - window.addEventListener('userrolerefresh', handleUserRoleRefresh); - return () => { subscription?.unsubscribe(); - window.removeEventListener('userrolerefresh', handleUserRoleRefresh); }; }, [loadUserRole, toAppUser, updateCurrentUser]); @@ -137,6 +125,5 @@ export const useAuthAndRole = () => { performSignOut, setCurrentUser, setUserRole, - user: currentUser, }; }; diff --git a/hooks/useToastNotifications.ts b/hooks/useToastNotifications.ts index 5ee3de4..ac326fb 100644 --- a/hooks/useToastNotifications.ts +++ b/hooks/useToastNotifications.ts @@ -1,5 +1,3 @@ -"use client"; - import { useState, useCallback } from 'react'; import { ToastMessage, ToastType } from '../types'; @@ -20,4 +18,4 @@ export const useToastNotifications = () => { }, []); return { toasts, showToast, removeToast }; -}; \ No newline at end of file +}; diff --git a/package.json b/package.json index d95afee..084015d 100644 --- a/package.json +++ b/package.json @@ -7,10 +7,11 @@ "dev": "vite", "build": "vite build", "preview": "vite preview", + "check:db-contract": "node --env-file-if-exists=.env --env-file-if-exists=.env.local scripts/check-db-contract.mjs", "test": "vitest", "test:run": "vitest run", "test:coverage": "vitest run --coverage", - "test:e2e": "playwright test", + "test:e2e": "node --env-file-if-exists=.env --env-file-if-exists=.env.local ./node_modules/@playwright/test/cli.js test", "typecheck": "tsc -p tsconfig.json --noEmit" }, "dependencies": { diff --git a/scripts/check-db-contract.mjs b/scripts/check-db-contract.mjs new file mode 100644 index 0000000..04fdadf --- /dev/null +++ b/scripts/check-db-contract.mjs @@ -0,0 +1,87 @@ +import { createClient } from '@supabase/supabase-js'; + +const requireEnv = (name) => { + const value = process.env[name]; + + if (!value) { + throw new Error(`${name} is required.`); + } + + return value; +}; + +const requireMeetingDay = (section, value) => { + if (!Number.isInteger(value) || value < 0 || value > 6) { + throw new Error(`Settings row for ${section} has invalid meeting_day: ${value}`); + } +}; + +const supabase = createClient(requireEnv('VITE_SUPABASE_URL'), requireEnv('VITE_SUPABASE_ANON_KEY'), { + auth: { + autoRefreshToken: false, + persistSession: false, + }, +}); + +const allowedRoles = new Set(['admin', 'captain', 'officer']); + +try { + const { data: signInData, error: signInError } = await supabase.auth.signInWithPassword({ + email: requireEnv('E2E_TEST_EMAIL'), + password: requireEnv('E2E_TEST_PASSWORD'), + }); + + if (signInError) { + throw new Error(`Failed to sign in smoke-test user: ${signInError.message}`); + } + + if (!signInData.user) { + throw new Error('Supabase sign-in succeeded without returning a user.'); + } + + const { data: role, error: roleError } = await supabase.rpc('current_app_role'); + + if (roleError) { + throw new Error(`Failed to resolve current_app_role(): ${roleError.message}`); + } + + if (typeof role !== 'string' || !allowedRoles.has(role)) { + throw new Error(`Unexpected app role for smoke-test user: ${String(role)}`); + } + + const { data: settingsRows, error: settingsError } = await supabase + .from('settings') + .select('section,meeting_day') + .in('section', ['company', 'junior']); + + if (settingsError) { + throw new Error(`Failed to read seeded settings rows: ${settingsError.message}`); + } + + const rows = settingsRows ?? []; + const rowsBySection = new Map(rows.map((row) => [row.section, row])); + + if (rows.length !== 2 || rowsBySection.size !== 2) { + throw new Error( + `Expected exactly one settings row for company and junior. Received ${rows.length} row(s).`, + ); + } + + for (const section of ['company', 'junior']) { + const row = rowsBySection.get(section); + + if (!row) { + throw new Error(`Missing seeded settings row for ${section}.`); + } + + requireMeetingDay(section, row.meeting_day); + } + + console.log(`Database contract smoke check passed for role ${role}.`); +} finally { + try { + await supabase.auth.signOut(); + } catch (error) { + console.warn('Failed to sign out after database contract check:', error); + } +} diff --git a/services/db.test.ts b/services/db.test.ts new file mode 100644 index 0000000..7f1ff54 --- /dev/null +++ b/services/db.test.ts @@ -0,0 +1,290 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const authMock = vi.hoisted(() => ({ + getCurrentUser: vi.fn(), +})); + +const supabaseMock = vi.hoisted(() => { + let rpcResponse: { error: { message?: string } | null } = { error: null }; + let insertSingleResponse: { data: unknown; error: { message?: string } | null } = { + data: null, + error: null, + }; + let updateResponse: { error: { message?: string } | null } = { error: null }; + let deleteResponse: { error: { message?: string } | null } = { error: null }; + let memberSelectSingleResponse: { data: unknown; error: { code?: string; message?: string } | null } = { + data: null, + error: { code: 'PGRST116', message: 'No rows found' }, + }; + let marksSelectResponse: { data: unknown[]; error: { message?: string } | null } = { + data: [], + error: null, + }; + + const rpc = vi.fn(() => Promise.resolve(rpcResponse)); + + const memberSelectSingle = vi.fn(() => Promise.resolve(memberSelectSingleResponse)); + const memberSelectEqSection = vi.fn(() => ({ single: memberSelectSingle })); + const memberSelectEqId = vi.fn(() => ({ eq: memberSelectEqSection })); + const memberSelect = vi.fn(() => ({ eq: memberSelectEqId })); + + const memberInsertSingle = vi.fn(() => Promise.resolve(insertSingleResponse)); + const memberInsertSelect = vi.fn(() => ({ single: memberInsertSingle })); + const memberInsert = vi.fn(() => ({ select: memberInsertSelect })); + + const memberUpdateEqSection = vi.fn(() => Promise.resolve(updateResponse)); + const memberUpdateEqId = vi.fn(() => ({ eq: memberUpdateEqSection })); + const memberUpdate = vi.fn(() => ({ eq: memberUpdateEqId })); + + const memberDeleteEqSection = vi.fn(() => Promise.resolve(deleteResponse)); + const memberDeleteEqId = vi.fn(() => ({ eq: memberDeleteEqSection })); + const memberDelete = vi.fn(() => ({ eq: memberDeleteEqId })); + + const marksSelectEqSection = vi.fn(() => Promise.resolve(marksSelectResponse)); + const marksSelectEqMember = vi.fn(() => ({ eq: marksSelectEqSection })); + const marksSelect = vi.fn(() => ({ eq: marksSelectEqMember })); + + const from = vi.fn((table: string) => { + if (table === 'members') { + return { + select: memberSelect, + insert: memberInsert, + update: memberUpdate, + delete: memberDelete, + }; + } + + if (table === 'marks') { + return { select: marksSelect }; + } + + throw new Error(`Unexpected table: ${table}`); + }); + + return { + from, + rpc, + memberUpdate, + memberUpdateEqId, + memberUpdateEqSection, + memberDelete, + memberDeleteEqId, + memberDeleteEqSection, + get rpcResponse() { + return rpcResponse; + }, + set rpcResponse(next) { + rpcResponse = next; + }, + get insertSingleResponse() { + return insertSingleResponse; + }, + set insertSingleResponse(next) { + insertSingleResponse = next; + }, + get updateResponse() { + return updateResponse; + }, + set updateResponse(next) { + updateResponse = next; + }, + get deleteResponse() { + return deleteResponse; + }, + set deleteResponse(next) { + deleteResponse = next; + }, + get memberSelectSingleResponse() { + return memberSelectSingleResponse; + }, + set memberSelectSingleResponse(next) { + memberSelectSingleResponse = next; + }, + get marksSelectResponse() { + return marksSelectResponse; + }, + set marksSelectResponse(next) { + marksSelectResponse = next; + }, + }; +}); + +vi.mock('./supabaseAuth', () => ({ + getCurrentUser: authMock.getCurrentUser, +})); + +vi.mock('./supabaseClient', () => ({ + supabase: { + from: supabaseMock.from, + rpc: supabaseMock.rpc, + }, +})); + +import { + deleteBoyById, + saveBoyMarks, + saveWeeklyMarksSnapshot, + updateBoy, +} from './db'; + +describe('db service write model', () => { + beforeEach(() => { + vi.clearAllMocks(); + authMock.getCurrentUser.mockResolvedValue({ id: 'user-1', email: 'captain@example.com' }); + supabaseMock.rpcResponse = { error: null }; + supabaseMock.insertSingleResponse = { data: null, error: null }; + supabaseMock.updateResponse = { error: null }; + supabaseMock.deleteResponse = { error: null }; + supabaseMock.memberSelectSingleResponse = { data: null, error: { code: 'PGRST116', message: 'No rows found' } }; + supabaseMock.marksSelectResponse = { data: [], error: null }; + }); + + it('sends a transactional member mark patch to the live patch RPC', async () => { + await expect( + saveBoyMarks( + 'member-1', + 'company', + [ + { date: '2026-03-13', score: 8 }, + { date: '2026-03-20', score: 9 }, + ], + [{ date: '2026-03-20', score: 7 }], + ), + ).resolves.toBeUndefined(); + + expect(supabaseMock.rpc).toHaveBeenCalledWith('save_member_marks_patch', { + p_member_id: 'member-1', + p_section: 'company', + p_delete_dates: ['2026-03-13'], + p_upsert_rows: [ + { + date: '2026-03-20', + section: 'company', + score: 7, + uniform_score: null, + behaviour_score: null, + present: true, + }, + ], + }); + }); + + it('skips the patch RPC when the member marks do not change', async () => { + await expect( + saveBoyMarks( + 'member-1', + 'junior', + [{ date: '2026-03-20', score: 7.5, uniformScore: 5, behaviourScore: 2.5 }], + [{ date: '2026-03-20', score: 7.5, uniformScore: 5, behaviourScore: 2.5 }], + ), + ).resolves.toBeUndefined(); + + expect(supabaseMock.rpc).not.toHaveBeenCalled(); + }); + + it('rejects duplicate mark dates before calling the patch RPC', async () => { + await expect( + saveBoyMarks( + 'member-1', + 'company', + [], + [ + { date: '2026-03-20', score: 7 }, + { date: '2026-03-20', score: 8 }, + ], + ), + ).rejects.toThrow(/duplicate mark date/i); + + expect(supabaseMock.rpc).not.toHaveBeenCalled(); + }); + + it('sends the whole selected-date snapshot to the live snapshot RPC', async () => { + await expect( + saveWeeklyMarksSnapshot('company', '2026-03-20', [ + { memberId: 'member-1', mark: { date: '2026-03-20', score: 7 } }, + { memberId: 'member-2', mark: null }, + ]), + ).resolves.toBeUndefined(); + + expect(supabaseMock.rpc).toHaveBeenCalledWith('save_weekly_marks_snapshot', { + p_section: 'company', + p_meeting_date: '2026-03-20', + p_snapshot: [ + { + memberId: 'member-1', + mark: { + section: 'company', + score: 7, + uniform_score: null, + behaviour_score: null, + present: true, + }, + }, + { + memberId: 'member-2', + mark: null, + }, + ], + }); + }); + + it('updates member fields without touching marks or validating mark payloads', async () => { + supabaseMock.memberSelectSingleResponse = { + data: { + id: 'member-1', + name: 'Alex', + squad: 2, + section: 'company', + school_year: '10', + is_squad_leader: false, + }, + error: null, + }; + supabaseMock.marksSelectResponse = { data: [], error: null }; + + await expect( + updateBoy( + { + id: 'member-1', + name: 'Alex Updated', + squad: 3, + year: 11, + marks: [{ date: '2026-03-20', score: 8, uniformScore: 4 }], + isSquadLeader: true, + }, + 'company', + ), + ).resolves.toEqual({ + id: 'member-1', + name: 'Alex', + squad: 2, + year: 10, + marks: [], + isSquadLeader: false, + }); + + expect(supabaseMock.from).toHaveBeenCalledWith('members'); + expect(supabaseMock.memberUpdate).toHaveBeenCalledWith({ + name: 'Alex Updated', + school_year: '11', + section: 'company', + squad: 3, + is_squad_leader: true, + }); + expect(supabaseMock.memberUpdateEqId).toHaveBeenCalledWith('id', 'member-1'); + expect(supabaseMock.memberUpdateEqSection).toHaveBeenCalledWith('section', 'company'); + expect(supabaseMock.rpc).not.toHaveBeenCalled(); + expect(supabaseMock.from.mock.calls.filter(([table]) => table === 'marks')).toHaveLength(1); + }); + + it('deletes only the members row and relies on cascade for marks', async () => { + await expect(deleteBoyById('member-1', 'junior')).resolves.toBeUndefined(); + + expect(supabaseMock.from).toHaveBeenCalledTimes(1); + expect(supabaseMock.from).toHaveBeenCalledWith('members'); + expect(supabaseMock.memberDelete).toHaveBeenCalled(); + expect(supabaseMock.memberDeleteEqId).toHaveBeenCalledWith('id', 'member-1'); + expect(supabaseMock.memberDeleteEqSection).toHaveBeenCalledWith('section', 'junior'); + expect(supabaseMock.rpc).not.toHaveBeenCalled(); + }); +}); diff --git a/services/db.ts b/services/db.ts index 1a4d742..ba66312 100644 --- a/services/db.ts +++ b/services/db.ts @@ -1,4 +1,4 @@ -import { Boy, Section, Mark } from '../types'; +import { Boy, Mark, Section, WeeklyMarksSnapshotEntry } from '../types'; import { supabase } from './supabaseClient'; import * as supabaseAuth from './supabaseAuth'; import { @@ -7,55 +7,130 @@ import { MemberRow, toStoredMark, validateBoyMarks, + validateMarksForSection, } from './dbModel'; -import { buildMemberMarkSyncPlan } from './dbSyncPlan'; -const syncMemberMarks = async (memberId: string, section: Section, marks: Mark[]) => { +export type WeeklyMarksSaveEntry = WeeklyMarksSnapshotEntry; + +type StoredMarkPatchRow = ReturnType; + +const storedMarksMatch = (left: StoredMarkPatchRow, right: StoredMarkPatchRow) => + left.date === right.date && + left.section === right.section && + left.score === right.score && + left.uniform_score === right.uniform_score && + left.behaviour_score === right.behaviour_score && + left.present === right.present; + +const buildMemberMarksPatch = ({ + originalMarks, + nextMarks, + section, +}: { + originalMarks: Mark[]; + nextMarks: Mark[]; + section: Section; +}) => { + const nextMarksByDate = new Map(nextMarks.map((mark) => [mark.date, mark])); + const originalMarksByDate = new Map(originalMarks.map((mark) => [mark.date, mark])); + + const deleteDates = originalMarks + .filter((mark) => !nextMarksByDate.has(mark.date)) + .map((mark) => mark.date); + + const upsertRows = nextMarks.reduce((rows, mark) => { + const nextStored = toStoredMark(mark, section); + const originalMark = originalMarksByDate.get(mark.date); + + if (!originalMark) { + rows.push(nextStored); + return rows; + } + + const originalStored = toStoredMark(originalMark, section); + + if (!storedMarksMatch(originalStored, nextStored)) { + rows.push(nextStored); + } + + return rows; + }, []); + + return { deleteDates, upsertRows }; +}; + +export const saveBoyMarks = async ( + memberId: string, + section: Section, + originalMarks: Mark[], + nextMarks: Mark[], +): Promise => { + if (!memberId) throw new Error('Member ID is required.'); const authUser = await supabaseAuth.getCurrentUser(); if (!authUser) throw new Error('User not authenticated'); - const { data: existingData, error: existingError } = await supabase - .from('marks') - .select('date') - .eq('member_id', memberId); - - if (existingError) { - throw new Error(existingError.message || 'Failed to fetch existing marks.'); - } + validateMarksForSection(nextMarks, section, 'Member'); - const { datesToDelete, markRows } = buildMemberMarkSyncPlan({ - existingDates: (existingData || []).map((row) => row.date as string), - marks, - memberId, - createdBy: authUser.id, + const { deleteDates, upsertRows } = buildMemberMarksPatch({ + originalMarks, + nextMarks, section, }); - if (datesToDelete.length > 0) { - const { error: deleteError } = await supabase - .from('marks') - .delete() - .eq('member_id', memberId) - .in('date', datesToDelete); + if (deleteDates.length === 0 && upsertRows.length === 0) { + return; + } - if (deleteError) { - throw new Error(deleteError.message || 'Failed to remove deleted marks.'); - } + const { error } = await supabase.rpc('save_member_marks_patch', { + p_member_id: memberId, + p_section: section, + p_delete_dates: deleteDates, + p_upsert_rows: upsertRows, + }); + + if (error) { + throw new Error(error.message || 'Failed to save member marks.'); } +}; - if (marks.length === 0) { +export const saveWeeklyMarksSnapshot = async ( + section: Section, + selectedDate: string, + snapshot: WeeklyMarksSnapshotEntry[], +): Promise => { + if (snapshot.length === 0) { return; } + const authUser = await supabaseAuth.getCurrentUser(); + if (!authUser) throw new Error('User not authenticated'); - const { error: upsertError } = await supabase - .from('marks') - .upsert(markRows, { onConflict: 'member_id,date' }); + validateMarksForSection( + snapshot.flatMap(({ mark }) => (mark ? [mark] : [])), + section, + 'Member', + ); + + const { error } = await supabase.rpc('save_weekly_marks_snapshot', { + p_section: section, + p_meeting_date: selectedDate, + p_snapshot: snapshot.map(({ memberId, mark }) => ({ + memberId, + mark: mark + ? (() => { + const { date: _date, ...storedMark } = toStoredMark(mark, section); + return storedMark; + })() + : null, + })), + }); - if (upsertError) { - throw new Error(upsertError.message || 'Failed to save marks.'); + if (error) { + throw new Error(error.message || 'Failed to save weekly marks.'); } }; +export const saveWeeklyMarksForSection = saveWeeklyMarksSnapshot; + export const createBoy = async (boy: Omit, section: Section): Promise => { validateBoyMarks(boy as Boy, section); const authUser = await supabaseAuth.getCurrentUser(); @@ -79,15 +154,18 @@ export const createBoy = async (boy: Omit, section: Section): Promise throw new Error(error?.message || 'Failed to create boy'); } - if (boy.marks.length > 0) { - await syncMemberMarks(data.id, section, boy.marks); + if (boy.marks.length === 0) { + return mapBoyRow(data as MemberRow, []); + } + + await saveBoyMarks(data.id, section, [], boy.marks); + + const savedBoy = await fetchBoyById(data.id, section); + if (!savedBoy) { + throw new Error('Failed to reload created boy.'); } - return mapBoyRow(data as MemberRow, boy.marks.map((mark, index) => ({ - id: `${index}`, - member_id: data.id, - ...toStoredMark(mark, section), - })) as MarkRow[]); + return savedBoy; }; export const fetchBoys = async (section: Section): Promise => { @@ -159,29 +237,25 @@ export const fetchBoyById = async (id: string, section: Section): Promise => { - validateBoyMarks(boy, section); - const { id, ...boyData } = boy; - if (!id) throw new Error('Boy ID is required.'); + if (!boy.id) throw new Error('Boy ID is required.'); const { error } = await supabase .from('members') .update({ - name: boyData.name, - school_year: String(boyData.year), + name: boy.name, + school_year: String(boy.year), section, - squad: boyData.squad, - is_squad_leader: boyData.isSquadLeader ?? false, + squad: boy.squad, + is_squad_leader: boy.isSquadLeader ?? false, }) - .eq('id', id) + .eq('id', boy.id) .eq('section', section); if (error) { throw new Error(error.message || 'Failed to update boy'); } - await syncMemberMarks(id, section, boyData.marks); - - const updatedBoy = await fetchBoyById(id, section); + const updatedBoy = await fetchBoyById(boy.id, section); if (!updatedBoy) { throw new Error('Failed to reload updated boy.'); } @@ -190,16 +264,6 @@ export const updateBoy = async (boy: Boy, section: Section): Promise => { }; export const deleteBoyById = async (id: string, section: Section): Promise => { - const { error: marksError } = await supabase - .from('marks') - .delete() - .eq('member_id', id) - .eq('section', section); - - if (marksError) { - throw new Error(marksError.message || 'Failed to delete member marks'); - } - const { error } = await supabase .from('members') .delete() diff --git a/services/dbModel.test.ts b/services/dbModel.test.ts index 6ef7d46..3708af5 100644 --- a/services/dbModel.test.ts +++ b/services/dbModel.test.ts @@ -115,4 +115,20 @@ describe('dbModel', () => { expect(() => validateBoyMarks(boy, 'junior')).toThrow(/does not match sum/i); }); + + it('rejects duplicate mark dates', () => { + const boy: Boy = { + id: 'member-1', + name: 'Euan', + squad: 2, + year: 10, + marks: [ + { date: '2026-03-20', score: 8 }, + { date: '2026-03-20', score: 7 }, + ], + isSquadLeader: false, + }; + + expect(() => validateBoyMarks(boy, 'company')).toThrow(/duplicate mark date/i); + }); }); diff --git a/services/dbModel.ts b/services/dbModel.ts index 9392620..2a63108 100644 --- a/services/dbModel.ts +++ b/services/dbModel.ts @@ -82,11 +82,13 @@ export const mapBoyRow = (member: MemberRow, marks: MarkRow[]): Boy => ({ isSquadLeader: member.is_squad_leader ?? false, }); -export const validateBoyMarks = (boy: Boy, section: Section) => { - if (!Array.isArray(boy.marks)) { +export const validateMarksForSection = (marks: Mark[], section: Section, subject = 'Member') => { + if (!Array.isArray(marks)) { throw new Error('Marks must be an array.'); } + const seenDates = new Set(); + const validateDecimalPlaces = (value: number, fieldName: string, date: string) => { if (value < 0) return; @@ -94,15 +96,21 @@ export const validateBoyMarks = (boy: Boy, section: Section) => { const decimalPart = valueString.split('.')[1]; if (decimalPart && decimalPart.length > 2) { - throw new Error(`${fieldName} for ${boy.name} on ${date} has more than 2 decimal places.`); + throw new Error(`${fieldName} for ${subject} on ${date} has more than 2 decimal places.`); } }; - for (const mark of boy.marks) { + for (const mark of marks) { if (typeof mark.date !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(mark.date)) { throw new Error(`Invalid date format for mark: ${mark.date}`); } + if (seenDates.has(mark.date)) { + throw new Error(`Duplicate mark date for ${subject} on ${mark.date}.`); + } + + seenDates.add(mark.date); + if (typeof mark.score !== 'number') { throw new Error(`Invalid score type for mark on ${mark.date}. Score must be a number.`); } @@ -115,19 +123,19 @@ export const validateBoyMarks = (boy: Boy, section: Section) => { if (section === 'company') { if (mark.score < 0 || mark.score > 10) { - throw new Error(`Company section score for ${boy.name} on ${mark.date} is out of range (0-10).`); + throw new Error(`Company section score for ${subject} on ${mark.date} is out of range (0-10).`); } if (mark.uniformScore !== undefined || mark.behaviourScore !== undefined) { - throw new Error(`Company section boy ${boy.name} on ${mark.date} has junior-specific scores.`); + throw new Error(`Company section boy ${subject} on ${mark.date} has junior-specific scores.`); } } else { if (typeof mark.uniformScore !== 'number' || mark.uniformScore < 0 || mark.uniformScore > 10) { - throw new Error(`Junior section uniform score for ${boy.name} on ${mark.date} is invalid or out of range (0-10).`); + throw new Error(`Junior section uniform score for ${subject} on ${mark.date} is invalid or out of range (0-10).`); } if (typeof mark.behaviourScore !== 'number' || mark.behaviourScore < 0 || mark.behaviourScore > 5) { - throw new Error(`Junior section behaviour score for ${boy.name} on ${mark.date} is invalid or out of range (0-5).`); + throw new Error(`Junior section behaviour score for ${subject} on ${mark.date} is invalid or out of range (0-5).`); } validateDecimalPlaces(mark.uniformScore, 'Uniform score', mark.date); @@ -136,8 +144,12 @@ export const validateBoyMarks = (boy: Boy, section: Section) => { const calculatedTotal = mark.uniformScore + mark.behaviourScore; if (Math.abs(mark.score - calculatedTotal) > 0.001) { - throw new Error(`Junior section total score for ${boy.name} on ${mark.date} does not match sum of uniform and behaviour scores.`); + throw new Error(`Junior section total score for ${subject} on ${mark.date} does not match sum of uniform and behaviour scores.`); } } } }; + +export const validateBoyMarks = (boy: Boy, section: Section) => { + validateMarksForSection(boy.marks, section, boy.name); +}; diff --git a/services/dbSyncPlan.test.ts b/services/dbSyncPlan.test.ts deleted file mode 100644 index db78400..0000000 --- a/services/dbSyncPlan.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -import { buildMemberMarkSyncPlan } from './dbSyncPlan'; - -describe('buildMemberMarkSyncPlan', () => { - it('identifies deleted dates and builds upsert rows', () => { - expect( - buildMemberMarkSyncPlan({ - existingDates: ['2026-03-13', '2026-03-20'], - marks: [{ date: '2026-03-20', score: 8 }], - memberId: 'member-1', - createdBy: 'user-1', - section: 'company', - }), - ).toEqual({ - datesToDelete: ['2026-03-13'], - markRows: [ - { - member_id: 'member-1', - created_by: 'user-1', - date: '2026-03-20', - section: 'company', - score: 8, - uniform_score: null, - behaviour_score: null, - present: true, - }, - ], - }); - }); - - it('maps absent marks correctly for junior rows', () => { - expect( - buildMemberMarkSyncPlan({ - existingDates: [], - marks: [{ date: '2026-03-20', score: -1 }], - memberId: 'member-1', - createdBy: 'user-1', - section: 'junior', - }), - ).toEqual({ - datesToDelete: [], - markRows: [ - { - member_id: 'member-1', - created_by: 'user-1', - date: '2026-03-20', - section: 'junior', - score: null, - uniform_score: null, - behaviour_score: null, - present: false, - }, - ], - }); - }); - - it('returns an empty upsert payload when there are no marks left', () => { - expect( - buildMemberMarkSyncPlan({ - existingDates: ['2026-03-20'], - marks: [], - memberId: 'member-1', - createdBy: 'user-1', - section: 'company', - }), - ).toEqual({ - datesToDelete: ['2026-03-20'], - markRows: [], - }); - }); -}); diff --git a/services/dbSyncPlan.ts b/services/dbSyncPlan.ts deleted file mode 100644 index 908bd26..0000000 --- a/services/dbSyncPlan.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Mark, Section } from '../types'; -import { toStoredMark } from './dbModel'; - -type BuildMemberMarkSyncPlanArgs = { - existingDates: Iterable; - marks: Mark[]; - memberId: string; - createdBy: string; - section: Section; -}; - -export const buildMemberMarkSyncPlan = ({ - existingDates, - marks, - memberId, - createdBy, - section, -}: BuildMemberMarkSyncPlanArgs) => { - const existingDateSet = new Set(existingDates); - const desiredDateSet = new Set(marks.map((mark) => mark.date)); - const datesToDelete = [...existingDateSet].filter((date) => !desiredDateSet.has(date)); - const markRows = marks.map((mark) => ({ - member_id: memberId, - created_by: createdBy, - ...toStoredMark(mark, section), - })); - - return { datesToDelete, markRows }; -}; diff --git a/services/settings.test.ts b/services/settings.test.ts index e06b05f..bc8381f 100644 --- a/services/settings.test.ts +++ b/services/settings.test.ts @@ -1,13 +1,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const supabaseMock = vi.hoisted(() => { - const single = vi.fn(); - const eq = vi.fn(() => ({ single })); - const select = vi.fn(() => ({ eq })); - const upsert = vi.fn(); - const from = vi.fn(() => ({ select, upsert })); + const readSingle = vi.fn(); + const readEq = vi.fn(() => ({ single: readSingle })); + const readSelect = vi.fn(() => ({ eq: readEq })); - return { from, select, eq, single, upsert }; + const writeSingle = vi.fn(); + const writeSelect = vi.fn(() => ({ single: writeSingle })); + const writeEq = vi.fn(() => ({ select: writeSelect })); + const update = vi.fn(() => ({ eq: writeEq })); + + const from = vi.fn(() => ({ select: readSelect, update })); + + return { from, readSelect, readEq, readSingle, update, writeEq, writeSelect, writeSingle }; }); vi.mock('./supabaseClient', () => ({ @@ -24,7 +29,7 @@ describe('settings', () => { }); it('returns the default meeting day when the settings row is missing', async () => { - supabaseMock.single.mockResolvedValueOnce({ + supabaseMock.readSingle.mockResolvedValueOnce({ data: null, error: { code: 'PGRST116' }, }); @@ -34,7 +39,7 @@ describe('settings', () => { }); it('returns the stored meeting day when the row exists', async () => { - supabaseMock.single.mockResolvedValueOnce({ + supabaseMock.readSingle.mockResolvedValueOnce({ data: { meeting_day: 2 }, error: null, }); @@ -42,8 +47,33 @@ describe('settings', () => { await expect(getSettings('junior')).resolves.toEqual({ meetingDay: 2 }); }); + it('updates the seeded settings row for admins and captains', async () => { + supabaseMock.writeSingle.mockResolvedValueOnce({ + data: { section: 'company', meeting_day: 4 }, + error: null, + }); + + await expect(saveSettings('company', { meetingDay: 4 }, 'captain')).resolves.toBeUndefined(); + expect(supabaseMock.update).toHaveBeenCalledWith({ + meeting_day: 4, + updated_at: expect.any(String), + }); + expect(supabaseMock.writeEq).toHaveBeenCalledWith('section', 'company'); + }); + it('rejects save attempts from non-admin roles before writing', async () => { await expect(saveSettings('company', { meetingDay: 4 }, 'officer')).rejects.toThrow(/permission denied/i); - expect(supabaseMock.upsert).not.toHaveBeenCalled(); + expect(supabaseMock.update).not.toHaveBeenCalled(); + }); + + it('surfaces a missing seeded row as an error', async () => { + supabaseMock.writeSingle.mockResolvedValueOnce({ + data: null, + error: { code: 'PGRST116', message: 'No rows found' }, + }); + + await expect(saveSettings('junior', { meetingDay: 2 }, 'admin')).rejects.toMatchObject({ + code: 'PGRST116', + }); }); }); diff --git a/services/settings.ts b/services/settings.ts index c240365..fc8233f 100644 --- a/services/settings.ts +++ b/services/settings.ts @@ -12,8 +12,8 @@ const DEFAULT_MEETING_DAY = 5; // Friday /** * Fetches the settings for a given section from Supabase. - * If no settings row exists for the section, it returns default values. - * This ensures the app always has valid settings to work with. + * The live project seeds one row per section; if a row is missing or unreadable, + * this falls back to the default meeting day so the UI can still render safely. * @param section The section ('company' or 'junior') to fetch settings for. * @returns A promise that resolves to the section's settings. */ @@ -45,6 +45,8 @@ export const getSettings = async (section: Section): Promise => /** * Saves the settings for a given section to Supabase. + * The live database contract expects one seeded row per section, so this updates + * the existing row in place rather than creating new rows. * @param section The section ('company' or 'junior') to save settings for. * @param settings The settings object to save. * @param userRole The role of the user attempting to save settings. @@ -59,11 +61,15 @@ export const saveSettings = async ( throw new Error('Permission denied: Only Admins and Captains can save settings.'); } - const { error } = await supabase.from('settings').upsert({ - section, - meeting_day: settings.meetingDay, - updated_at: new Date().toISOString(), - }); + const { error } = await supabase + .from('settings') + .update({ + meeting_day: settings.meetingDay, + updated_at: new Date().toISOString(), + }) + .eq('section', section) + .select('section,meeting_day') + .single(); if (error) { console.error(`Error saving settings for ${section}:`, error); diff --git a/tests/e2e/02-section-settings-workflow.md b/tests/e2e/02-section-settings-workflow.md new file mode 100644 index 0000000..26da0f2 --- /dev/null +++ b/tests/e2e/02-section-settings-workflow.md @@ -0,0 +1,23 @@ +# E2E Runbook: Section Settings Workflow + +Use this checklist to verify section settings persistence against a live Supabase-backed environment. + +## Preconditions + +- Signed in as a user allowed to edit section settings +- A target section is selected +- `settings` contains seeded rows for both `company` and `junior` + +## Checks + +1. Open the section settings page. +2. Change the weekly meeting day. +3. Save the change and confirm the UI reports success. +4. Reload the page and confirm the new value persists. +5. Restore the original meeting day value, save it, reload the page, and confirm the original value persisted before leaving the page. + +## Expected Outcome + +- Section settings update in place rather than creating new rows. +- Missing seeded rows or broken writes fail the smoke check. +- The test run leaves the live settings value unchanged after cleanup and proves that restore step persisted. diff --git a/tests/e2e/smoke.e2e.ts b/tests/e2e/smoke.e2e.ts index 3c29ae3..c81a7f0 100644 --- a/tests/e2e/smoke.e2e.ts +++ b/tests/e2e/smoke.e2e.ts @@ -35,6 +35,50 @@ const selectCompanySection = async (page: Page) => { await expect(page.getByRole('heading', { name: 'Members' })).toBeVisible(); }; +const openSectionSettings = async (page: Page) => { + await page.getByRole('button', { name: 'Section Settings' }).click(); + await expect(page.getByRole('heading', { name: 'Section Settings' })).toBeVisible(); +}; + +const ensureSectionSettingsOpen = async (page: Page) => { + const heading = page.getByRole('heading', { name: 'Section Settings' }); + + if (!(await heading.isVisible())) { + await openSectionSettings(page); + } +}; + +const saveSettingsAndWait = async (page: Page) => { + const saveResponsePromise = page.waitForResponse( + (response) => + response.request().method() === 'PATCH' && + response.url().includes('/rest/v1/settings'), + ); + + await page.getByRole('button', { name: 'Save' }).click(); + const saveResponse = await saveResponsePromise; + expect(saveResponse.ok()).toBeTruthy(); + await expect(page.getByText('Settings saved successfully!')).toBeVisible(); +}; + +const openWeeklyMarksPage = async (page: Page) => { + await page.getByRole('button', { name: 'Weekly Marks' }).click(); + await expect(page.getByRole('heading', { name: 'Weekly Marks' })).toBeVisible(); + await expect(page.getByText('No Members to Mark')).toHaveCount(0); +}; + +const saveWeeklyMarksAndWait = async (page: Page) => { + const saveResponsePromise = page.waitForResponse( + (response) => + response.request().method() === 'POST' && + response.url().includes('/rest/v1/rpc/save_weekly_marks_snapshot'), + ); + + await page.getByRole('button', { name: 'Save Marks' }).click(); + const saveResponse = await saveResponsePromise; + expect(saveResponse.ok()).toBeTruthy(); +}; + test.describe('E2E smoke tests', () => { test('auth session persists across reload and can sign out', async ({ page }) => { await signIn(page); @@ -51,30 +95,122 @@ test.describe('E2E smoke tests', () => { test('company weekly marks can be saved and reloaded', async ({ page }) => { const marksDate = getFutureMarksDate(); + let testError: unknown; + let restoreError: unknown; await signIn(page); await selectCompanySection(page); - await page.getByRole('button', { name: 'Weekly Marks' }).click(); - await expect(page.getByRole('heading', { name: 'Weekly Marks' })).toBeVisible(); - await expect(page.getByText('No Members to Mark')).toHaveCount(0); + await openWeeklyMarksPage(page); - await page.getByLabel('Select weekly marks date').fill(marksDate); + const dateInput = page.getByLabel('Select weekly marks date'); + await dateInput.fill(marksDate); const scoreInput = page.locator('input[aria-label^="Score for "]:not([disabled])').first(); await expect(scoreInput).toBeVisible(); - const currentValue = await scoreInput.inputValue(); - const nextValue = currentValue === '8' ? '9' : '8'; - - await scoreInput.fill(nextValue); - await page.getByRole('button', { name: 'Save Marks' }).click(); - await expect(page.getByText('Marks saved successfully!')).toBeVisible(); - - await page.reload(); - await page.getByRole('button', { name: 'Weekly Marks' }).click(); - await page.getByLabel('Select weekly marks date').fill(marksDate); + const originalValue = await scoreInput.inputValue(); + const nextValue = originalValue === '8' ? '9' : '8'; + + try { + await scoreInput.fill(nextValue); + await saveWeeklyMarksAndWait(page); + + await page.reload(); + await openWeeklyMarksPage(page); + await dateInput.fill(marksDate); + + const reloadedScoreInput = page.locator('input[aria-label^="Score for "]:not([disabled])').first(); + await expect(reloadedScoreInput).toHaveValue(nextValue); + } catch (error) { + testError = error; + } finally { + try { + await openWeeklyMarksPage(page); + await dateInput.fill(marksDate); + + const restoreScoreInput = page.locator('input[aria-label^="Score for "]:not([disabled])').first(); + await expect(restoreScoreInput).toBeVisible(); + + if ((await restoreScoreInput.inputValue()) !== originalValue) { + await restoreScoreInput.fill(originalValue); + await saveWeeklyMarksAndWait(page); + await page.reload(); + await openWeeklyMarksPage(page); + await dateInput.fill(marksDate); + await expect(page.locator('input[aria-label^="Score for "]:not([disabled])').first()).toHaveValue(originalValue); + } + } catch (error) { + restoreError = error; + } + } + + if (testError && restoreError) { + throw new AggregateError( + [testError, restoreError], + 'Weekly marks smoke test failed and cleanup could not restore the original score.', + ); + } + + if (testError) { + throw testError; + } + + if (restoreError) { + throw restoreError; + } + }); - const reloadedScoreInput = page.locator('input[aria-label^="Score for "]:not([disabled])').first(); - await expect(reloadedScoreInput).toHaveValue(nextValue); + test('company section settings can be saved and restored', async ({ page }) => { + await signIn(page); + await selectCompanySection(page); + await openSectionSettings(page); + + const meetingDaySelect = page.getByLabel('Weekly Meeting Day'); + const originalValue = Number(await meetingDaySelect.inputValue()); + const nextValue = (originalValue + 1) % 7; + + let testError: unknown; + let restoreError: unknown; + + try { + await meetingDaySelect.selectOption(String(nextValue)); + await saveSettingsAndWait(page); + await expect(meetingDaySelect).toHaveValue(String(nextValue)); + + await page.reload(); + await openSectionSettings(page); + await expect(page.getByLabel('Weekly Meeting Day')).toHaveValue(String(nextValue)); + } catch (error) { + testError = error; + } finally { + try { + await ensureSectionSettingsOpen(page); + const restoreSelect = page.getByLabel('Weekly Meeting Day'); + if ((await restoreSelect.inputValue()) !== String(originalValue)) { + await restoreSelect.selectOption(String(originalValue)); + await saveSettingsAndWait(page); + await page.reload(); + await openSectionSettings(page); + await expect(page.getByLabel('Weekly Meeting Day')).toHaveValue(String(originalValue)); + } + } catch (error) { + restoreError = error; + } + } + + if (testError && restoreError) { + throw new AggregateError( + [testError, restoreError], + 'Settings smoke test failed and cleanup could not restore the original meeting day.', + ); + } + + if (testError) { + throw testError; + } + + if (restoreError) { + throw restoreError; + } }); }); diff --git a/types.ts b/types.ts index 17cbe50..7155357 100644 --- a/types.ts +++ b/types.ts @@ -36,6 +36,14 @@ export interface Mark { behaviourScore?: number; } +/** + * Represents the weekly snapshot payload used for section-wide mark saves. + */ +export interface WeeklyMarksSnapshotEntry { + memberId: string; + mark: Mark | null; +} + /** * Represents a single member of the Boys' Brigade. */