From 4f0f5df38dfdf455a06c5e7fa880889b9a055aa3 Mon Sep 17 00:00:00 2001 From: btrippcsci Date: Wed, 23 Sep 2026 09:52:15 -0400 Subject: [PATCH] feat(profiles): mirror the server's profile roster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shots are attributed to a profile — a person or a place — and the server already owns the whole surface: get_profiles, set_active_profile, add_profile, rename_profile, remove_profile, each answered with one authoritative `profiles` snapshot. Mobile had no way to read or change it, so a shared bay could not be told apart on the phone. Adds the wire types, a store that mirrors the roster, and the socket mapping. No UI: the data layer lands first so screens can be built against it without reshaping the contract underneath them. The store is deliberately not persisted. The web UI found that a second copy of the selection raced the snapshot that arrives on connect, and a phone reconnects far more often than a kiosk does. The roster survives a transient drop — Socket.IO reconnects on its own and blanking the picker on every wifi hiccup would be worse — but a deliberate disconnect or a switch to a different server clears it, alongside the device status and club, so another server's roster cannot linger as though current. Profile mutations go through the shared emitWhileConnected helper, so a selection made during a transient drop is not buffered by Socket.IO and replayed on reconnect, where it could decide who later shots are filed under. Also removes PlayerChangedPayload. It typed a `player_changed` event that does not exist in the server; profiles are what player selection needs. ROADMAP.md still specifies set_player/player_changed for Phase 1 item 4 and wants correcting in a docs change. Co-Authored-By: Claude Opus 5.5 (1M context) --- __tests__/socket.test.ts | 196 +++++++++++++++++++++++++++++- __tests__/useProfileStore.test.ts | 122 +++++++++++++++++++ services/socket.ts | 59 +++++++-- stores/useProfileStore.ts | 45 +++++++ types.ts | 30 ++++- 5 files changed, 437 insertions(+), 15 deletions(-) create mode 100644 __tests__/useProfileStore.test.ts create mode 100644 stores/useProfileStore.ts diff --git a/__tests__/socket.test.ts b/__tests__/socket.test.ts index 2582bff..4c52386 100644 --- a/__tests__/socket.test.ts +++ b/__tests__/socket.test.ts @@ -1,7 +1,8 @@ import { socketService } from '../services/socket'; import { useDeviceStore } from '../stores/useDeviceStore'; import { useSessionStore } from '../stores/useSessionStore'; -import type { Shot } from '../types'; +import { useProfileStore } from '../stores/useProfileStore'; +import type { Profile, ProfilesSnapshot, Shot } from '../types'; // Fake Socket.IO socket. The fake is built *inside* the mock factory (not // captured from an outer const) so it exists by the time `services/socket` @@ -111,6 +112,9 @@ beforeEach(() => { club: null, }); useDeviceStore.getState().reset(); + // The profile store is a module singleton too; without this a roster can + // survive into the next test and let an assertion pass for the wrong reason. + useProfileStore.getState().reset(); for (const key of Object.keys(mockHandlers)) delete mockHandlers[key]; mockStatus.connected = false; mockIo.mockClear(); @@ -695,3 +699,193 @@ describe('the selected club', () => { expect(useSessionStore.getState().club).toBeNull(); }); }); + +function makeProfile(overrides: Partial = {}): Profile { + return { + id: 'p1', + name: 'Alex', + created_at: '2026-09-14T10:00:00Z', + settings: {}, + ...overrides, + }; +} + +function makeSnapshot(overrides: Partial = {}): ProfilesSnapshot { + return { profiles: [makeProfile()], active_profile_id: 'p1', ...overrides }; +} + +describe('the profile roster', () => { + it('asks for the roster once connected', () => { + // It does not ride along on session_state, so it has to be asked for. + socketService.connect('http://host:8080'); + trigger('connect'); + + expect(mockEmit).toHaveBeenCalledWith('get_profiles'); + }); + + it('asks again after reconnecting', () => { + // Profiles can be added or renamed on the kiosk while the phone is away. + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('disconnect'); + mockEmit.mockClear(); + + trigger('connect'); + + expect(mockEmit).toHaveBeenCalledWith('get_profiles'); + }); + + it('mirrors the roster the server broadcast', () => { + socketService.connect('http://host:8080'); + trigger('connect'); + + trigger('profiles', { + profiles: [makeProfile({ id: 'p1', name: 'Alex' }), makeProfile({ id: 'p2', name: 'Sam' })], + active_profile_id: 'p2', + }); + + const state = useProfileStore.getState(); + expect(state.profiles.map((profile) => profile.name)).toEqual(['Alex', 'Sam']); + expect(state.activeProfileId).toBe('p2'); + expect(state.loaded).toBe(true); + }); + + it('keeps the roster through a transient drop', () => { + // Socket.IO reconnects on its own; blanking the picker on every wifi + // hiccup would be worse than showing one the next snapshot replaces. + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('profiles', makeSnapshot()); + + trigger('disconnect'); + + expect(useProfileStore.getState().profiles).toHaveLength(1); + }); + + it('forgets the roster when the user disconnects deliberately', () => { + // A roster from the previous server must not linger as though current. + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('profiles', makeSnapshot()); + + socketService.disconnect(); + + const state = useProfileStore.getState(); + expect(state.profiles).toEqual([]); + expect(state.loaded).toBe(false); + }); + + it('forgets the roster when switching to a different server', () => { + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('profiles', makeSnapshot()); + + socketService.connect('http://other:8080'); + + const state = useProfileStore.getState(); + expect(state.profiles).toEqual([]); + expect(state.loaded).toBe(false); + }); + + it('keeps the roster when retrying the same server', () => { + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('profiles', makeSnapshot()); + trigger('connect_error'); + + socketService.connect('http://host:8080'); + + expect(useProfileStore.getState().profiles).toHaveLength(1); + }); + + it('keeps the last good roster when a malformed snapshot arrives', () => { + socketService.connect('http://host:8080'); + trigger('connect'); + trigger('profiles', makeSnapshot()); + + trigger('profiles', { profiles: undefined }); + + expect(useProfileStore.getState().profiles).toHaveLength(1); + }); + + it('applies a repeated snapshot without accumulating the roster', () => { + // The server rebroadcasts after every mutation, so the same roster arrives + // repeatedly; each one replaces rather than appends. + socketService.connect('http://host:8080'); + trigger('connect'); + + trigger('profiles', makeSnapshot()); + trigger('profiles', makeSnapshot()); + + expect(useProfileStore.getState().profiles).toHaveLength(1); + }); +}); + +describe('changing the roster', () => { + beforeEach(() => { + socketService.connect('http://host:8080'); + trigger('connect'); + mockEmit.mockClear(); + }); + + it('selects a profile by id', () => { + socketService.setActiveProfile('p2'); + expect(mockEmit).toHaveBeenCalledWith('set_active_profile', { profile_id: 'p2' }); + }); + + it('adds a profile by name', () => { + socketService.addProfile('Sam'); + expect(mockEmit).toHaveBeenCalledWith('add_profile', { name: 'Sam' }); + }); + + it('renames a profile', () => { + socketService.renameProfile('p1', 'Alexandra'); + expect(mockEmit).toHaveBeenCalledWith('rename_profile', { + profile_id: 'p1', + name: 'Alexandra', + }); + }); + + it('removes a profile', () => { + socketService.removeProfile('p2'); + expect(mockEmit).toHaveBeenCalledWith('remove_profile', { profile_id: 'p2' }); + }); + + it('sends nothing when there is no connection', () => { + // A screen can still be mounted after a disconnect; emitting into a closed + // socket would be silently lost, so nothing is sent at all. + socketService.disconnect(); + mockEmit.mockClear(); + + socketService.setActiveProfile('p2'); + + expect(mockEmit).not.toHaveBeenCalled(); + }); + + it('sends nothing during a transient drop, even once reconnected', () => { + // Socket.IO keeps the socket through a wifi drop and would buffer the emit + // for replay on reconnect, landing a stale selection that decides who the + // next shots are filed under. + trigger('disconnect'); + + socketService.setActiveProfile('p2'); + socketService.addProfile('Sam'); + socketService.renameProfile('p1', 'Alexandra'); + socketService.removeProfile('p2'); + trigger('connect'); + + expect(mockEmit).not.toHaveBeenCalledWith('set_active_profile', expect.anything()); + expect(mockEmit).not.toHaveBeenCalledWith('add_profile', expect.anything()); + expect(mockEmit).not.toHaveBeenCalledWith('rename_profile', expect.anything()); + expect(mockEmit).not.toHaveBeenCalledWith('remove_profile', expect.anything()); + }); + + it('sends again once the connection is back', () => { + trigger('disconnect'); + trigger('connect'); + + socketService.setActiveProfile('p2'); + + expect(mockEmit).toHaveBeenCalledWith('set_active_profile', { profile_id: 'p2' }); + }); +}); diff --git a/__tests__/useProfileStore.test.ts b/__tests__/useProfileStore.test.ts new file mode 100644 index 0000000..f72fcc7 --- /dev/null +++ b/__tests__/useProfileStore.test.ts @@ -0,0 +1,122 @@ +import { useProfileStore } from '../stores/useProfileStore'; +import type { Profile, ProfilesSnapshot } from '../types'; + +function makeProfile(overrides: Partial = {}): Profile { + return { + id: 'p1', + name: 'Alex', + created_at: '2026-09-14T10:00:00Z', + settings: {}, + ...overrides, + }; +} + +function makeSnapshot(overrides: Partial = {}): ProfilesSnapshot { + return { + profiles: [makeProfile()], + active_profile_id: 'p1', + ...overrides, + }; +} + +// The store is a module singleton; reset it so one test's roster cannot leak +// into the next. +beforeEach(() => { + useProfileStore.getState().reset(); +}); + +describe('useProfileStore', () => { + it('starts empty and not yet loaded', () => { + const state = useProfileStore.getState(); + expect(state.profiles).toEqual([]); + expect(state.activeProfileId).toBe(''); + // A screen needs to tell "not asked yet" apart from "no profiles exist". + expect(state.loaded).toBe(false); + }); + + it('mirrors the roster and selection the server sent', () => { + useProfileStore.getState().applySnapshot( + makeSnapshot({ + profiles: [makeProfile({ id: 'p1', name: 'Alex' }), makeProfile({ id: 'p2', name: 'Sam' })], + active_profile_id: 'p2', + }), + ); + + const state = useProfileStore.getState(); + expect(state.profiles.map((profile) => profile.name)).toEqual(['Alex', 'Sam']); + expect(state.activeProfileId).toBe('p2'); + expect(state.loaded).toBe(true); + }); + + it('replaces the roster wholesale rather than merging into it', () => { + // The server sends its complete roster every time, so a profile removed on + // the kiosk has to disappear here too. + useProfileStore + .getState() + .applySnapshot( + makeSnapshot({ profiles: [makeProfile({ id: 'p1' }), makeProfile({ id: 'p2' })] }), + ); + + useProfileStore + .getState() + .applySnapshot( + makeSnapshot({ profiles: [makeProfile({ id: 'p1' })], active_profile_id: 'p1' }), + ); + + expect(useProfileStore.getState().profiles.map((profile) => profile.id)).toEqual(['p1']); + }); + + it('keeps the last good roster when a malformed snapshot arrives', () => { + // Blanking the picker mid-session would be worse than showing a stale + // roster that the next valid snapshot corrects. + useProfileStore.getState().applySnapshot(makeSnapshot()); + + useProfileStore + .getState() + .applySnapshot({ profiles: undefined } as unknown as ProfilesSnapshot); + + const state = useProfileStore.getState(); + expect(state.profiles).toHaveLength(1); + expect(state.loaded).toBe(true); + }); + + it('treats a missing active_profile_id as no selection', () => { + useProfileStore + .getState() + .applySnapshot({ profiles: [makeProfile()] } as unknown as ProfilesSnapshot); + + expect(useProfileStore.getState().activeProfileId).toBe(''); + expect(useProfileStore.getState().loaded).toBe(true); + }); + + it('accepts an empty roster as a real answer, not a failure', () => { + useProfileStore.getState().applySnapshot(makeSnapshot({ profiles: [], active_profile_id: '' })); + + const state = useProfileStore.getState(); + expect(state.profiles).toEqual([]); + expect(state.loaded).toBe(true); + }); + + it('round-trips the open settings dict untouched', () => { + // The server persists `settings` without interpreting it, and later + // features claim keys there; the client must not reshape it. + const settings = { bag: ['driver', '7-iron'], nested: { anything: 1 } }; + + useProfileStore + .getState() + .applySnapshot(makeSnapshot({ profiles: [makeProfile({ settings })] })); + + expect(useProfileStore.getState().profiles[0].settings).toEqual(settings); + }); + + it('forgets the roster on reset so a stale one cannot look current', () => { + useProfileStore.getState().applySnapshot(makeSnapshot()); + + useProfileStore.getState().reset(); + + const state = useProfileStore.getState(); + expect(state.profiles).toEqual([]); + expect(state.activeProfileId).toBe(''); + expect(state.loaded).toBe(false); + }); +}); diff --git a/services/socket.ts b/services/socket.ts index 00aae8b..ae85011 100644 --- a/services/socket.ts +++ b/services/socket.ts @@ -1,6 +1,7 @@ import { io, type Socket } from 'socket.io-client'; import { useDeviceStore } from '../stores/useDeviceStore'; import { useSessionStore } from '../stores/useSessionStore'; +import { useProfileStore } from '../stores/useProfileStore'; import { saveServerUrl } from '../storage/connection'; import { getShotRepository } from '../storage/db'; import type { @@ -8,6 +9,7 @@ import type { DebugStatusPayload, DebugToggledPayload, PowerStatusPayload, + ProfilesSnapshot, SessionStatePayload, Shot, ShotEnvelope, @@ -51,12 +53,13 @@ class SocketService { this.socket = null; } - // Device status and the selected club both describe the server that - // reported them. Switching servers must not leave another Pi's radar port, - // battery or club on screen; the new one restores its own club through - // session_state once connected. + // Device status, the profile roster and the selected club all describe the + // server that reported them. Switching servers must not leave another Pi's + // radar port, battery, roster or club on screen; the new one sends its own + // once connected. if (this.url !== null && this.url !== url) { useDeviceStore.getState().reset(); + useProfileStore.getState().reset(); store.setClub(null); } @@ -78,12 +81,13 @@ class SocketService { this.socket = null; this.url = null; useSessionStore.getState().setConnectionState('disconnected'); - // Only the deliberate disconnect forgets the device and the club. A - // transient drop is handled by the 'disconnect' event below, which leaves - // both in place: blanking the radar and battery every time the wifi + // Only the deliberate disconnect forgets the device, the roster and the + // club. A transient drop is handled by the 'disconnect' event below, which + // leaves them in place: blanking the radar and battery every time the wifi // hiccups would read as hardware failing rather than a wobbly link, and - // reconnecting restores the same server's club. + // reconnecting restores the same server's roster and club. useDeviceStore.getState().reset(); + useProfileStore.getState().reset(); useSessionStore.getState().setClub(null); } @@ -113,11 +117,33 @@ class SocketService { this.emitWhileConnected('toggle_debug'); } + // --- Profiles --- + // Every mutation is fire-and-forget: the server answers each one with a full + // `profiles` snapshot, including when it refuses (it will not remove the + // active profile, one with shots, or the last one). There is nothing to + // update optimistically and nothing to roll back — the reply is the truth. + + setActiveProfile(profileId: string): void { + this.emitWhileConnected('set_active_profile', { profile_id: profileId }); + } + + addProfile(name: string): void { + this.emitWhileConnected('add_profile', { name }); + } + + renameProfile(profileId: string, name: string): void { + this.emitWhileConnected('rename_profile', { profile_id: profileId, name }); + } + + removeProfile(profileId: string): void { + this.emitWhileConnected('remove_profile', { profile_id: profileId }); + } + // Socket.IO keeps the socket through a transient drop and buffers anything // emitted meanwhile, replaying it on reconnect. A change made before the drop // could then land after the user moved on -- filing later shots under a club - // picked earlier, or flipping recording behind the user's back -- so a change - // is sent only over a live connection. + // or profile picked earlier, or flipping recording behind the user's back -- + // so a change is sent only over a live connection. private emitWhileConnected(event: string, payload?: object): void { if (!this.socket?.connected) return; if (payload === undefined) this.socket.emit(event); @@ -161,6 +187,10 @@ class SocketService { // The server does not push debug mode on connect, and it is server-global, // so a recording may already be running. Read-only, like the above. socket.emit('get_debug_status'); + // The roster is not part of session_state, so it is asked for + // separately — and on every reconnect, since it may have changed on + // another client while this phone was away. + socket.emit('get_profiles'); }); socket.on('disconnect', () => { @@ -225,6 +255,15 @@ class SocketService { socket.on('debug_toggled', (data: DebugToggledPayload) => { useDeviceStore.getState().applyDebugStatus(data); }); + + // The server's authoritative roster, broadcast after every mutation — and + // after one it refuses, which is how a client that asked for something + // invalid (removing the active profile, the last profile, or one with + // shots) discovers nothing changed. Applying it verbatim is the whole + // reconciliation strategy; there is no local copy to merge. + socket.on('profiles', (data: ProfilesSnapshot) => { + useProfileStore.getState().applySnapshot(data); + }); } } diff --git a/stores/useProfileStore.ts b/stores/useProfileStore.ts new file mode 100644 index 0000000..7e7561b --- /dev/null +++ b/stores/useProfileStore.ts @@ -0,0 +1,45 @@ +import { create } from 'zustand'; +import type { Profile, ProfilesSnapshot } from '../types'; + +// A mirror of the server's roster, not a source of truth. +// +// The server owns profiles.json and broadcasts one authoritative `profiles` +// snapshot after every mutation — including mutations it refuses — so there is +// nothing to reconcile here and nothing to persist. Deliberately no +// AsyncStorage: the web UI found that a second copy of the selection raced the +// snapshot that arrives on connect, and a phone reconnects far more often than +// a kiosk does. + +interface ProfileState { + profiles: Profile[]; + // The server's chosen profile. Empty string until the first snapshot lands, + // matching the server's own "no selection yet" representation. + activeProfileId: string; + // False until the first snapshot arrives, so a screen can tell "no profiles" + // apart from "not asked yet" and show a skeleton rather than an empty state. + loaded: boolean; + + applySnapshot: (snapshot: ProfilesSnapshot) => void; + // Drop back to the pre-connection state. A roster from the previous server + // must not linger as though it were current. + reset: () => void; +} + +export const useProfileStore = create((set) => ({ + profiles: [], + activeProfileId: '', + loaded: false, + + applySnapshot: (snapshot) => { + // A malformed payload leaves the last good roster in place rather than + // blanking the picker mid-session. + if (!snapshot || !Array.isArray(snapshot.profiles)) return; + set({ + profiles: snapshot.profiles, + activeProfileId: snapshot.active_profile_id ?? '', + loaded: true, + }); + }, + + reset: () => set({ profiles: [], activeProfileId: '', loaded: false }), +})); diff --git a/types.ts b/types.ts index acabf3a..bedf8cd 100644 --- a/types.ts +++ b/types.ts @@ -74,14 +74,36 @@ export interface SessionStatePayload { // `shot_processing` event: the capture/analysis lifecycle for the live view. export type ShotProcessingState = 'capturing' | 'calculating' | 'failed'; -// `club_changed` / `player_changed`: server-pushed selection changes to reflect -// back into the local pickers without echoing to the server. +// `club_changed`: a server-pushed selection change, reflected back into the +// local picker without echoing to the server. export interface ClubChangedPayload { club: string; } -export interface PlayerChangedPayload { - player_name: string; +// --- Profiles (mirrors src/openflight/profiles.py) --- +// A profile is one named context shots are attributed to — a person, or a +// place. There is deliberately no separate "player" concept: the server has no +// set_player or player_changed event, and profiles are what player selection +// actually needs. + +export interface Profile { + id: string; + name: string; + // ISO-8601 UTC, seconds precision, Z-suffixed. + created_at: string; + // An open dict the server persists and round-trips without interpreting it; + // later features claim keys here. Left unshaped on purpose — narrowing it + // client-side would silently drop keys written by another client. + settings: Record; +} + +// The `profiles` event: the server's authoritative roster and selection, sent +// as one snapshot after every mutation — including a mutation it refuses, so a +// client that asked for something invalid self-heals from the reply. +export interface ProfilesSnapshot { + profiles: Profile[]; + // Empty string when nothing is selected yet. + active_profile_id: string; } // --- Device status (mirrors src/openflight/server.py and power/models.py) ---