diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 46c76f682..a19cbbc3b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,6 +11,13 @@ on: - 'client/**' - 'shared/**' - 'plugin-sdk/**' + # npm workspaces hoist every dependency into the single root lockfile, so + # a dependency bump touches none of the four directories above — and a + # native binding like better-sqlite3 is exactly what needs the suite. + - 'package.json' + - 'package-lock.json' + - 'Dockerfile' + - 'docker-compose.yml' - '.github/workflows/test.yml' jobs: diff --git a/client/scripts/check-page-pattern.mjs b/client/scripts/check-page-pattern.mjs index 5d7d789c9..05acc51a9 100644 --- a/client/scripts/check-page-pattern.mjs +++ b/client/scripts/check-page-pattern.mjs @@ -2,9 +2,12 @@ // src/pages/PATTERN.md). A *Page.tsx default-export component should wire a // co-located use() hook into JSX — it must not own state/effects itself. // -// We scan only the default-export component body (from `export default function` -// up to the next top-level `function` declaration or EOF), so presentational -// sub-components and helper hooks living in the same file are not flagged. +// We scan the default-export component body (from `export default function` up +// to the next top-level `function` declaration or EOF), and the same span of a +// top-level `*PageDesktop`/`*PageMobile` — since ViewportRoute started picking +// the branch, the default export is a two-line shim and the real container is +// one of those two. Everything else at top level stays exempt: presentational +// sub-components and helper hooks live in these files on purpose. // Context hooks like useTranslation/useParams are fine; the smell is stateful // logic — useState/useReducer/useEffect/useLayoutEffect/useMemo/useCallback/useRef. import { readdirSync, readFileSync } from 'node:fs' @@ -13,23 +16,38 @@ import { fileURLToPath } from 'node:url' const pagesDir = join(dirname(fileURLToPath(import.meta.url)), '..', 'src', 'pages') const BANNED = ['useState', 'useReducer', 'useEffect', 'useLayoutEffect', 'useMemo', 'useCallback', 'useRef'] -const bannedRe = new RegExp(`\\b(${BANNED.join('|')})\\s*\\(`) +// `[<(]` rather than `\(`: a generic call — `useRef<() => void>(null)` — is the +// same thing, and it slipped through for as long as the pattern only knew `(`. +const bannedRe = new RegExp(`\\b(${BANNED.join('|')})\\s*[<(]`) + +// Declarations the widened scan caught that are not this change's to move. +// Listed by the name they declare rather than by line, so an entry dies with +// its declaration instead of drifting onto whatever follows it. The rest of +// each listed file is still checked. +const KNOWN_ESCAPES = { + 'TripPlannerPage.tsx': ['glMap', 'bookingExpense'], +} const violations = [] for (const file of readdirSync(pagesDir)) { if (!file.endsWith('Page.tsx') || file.endsWith('.test.tsx')) continue const src = readFileSync(join(pagesDir, file), 'utf8') const lines = src.split('\n') - const start = lines.findIndex(l => /export default function/.test(l)) - if (start === -1) continue - // The page body ends at the next top-level declaration (a `function` at - // column 0) — everything after that is a sub-component or helper. - let end = lines.length - for (let i = start + 1; i < lines.length; i++) { - if (/^(function |const [A-Z]\w* = )/.test(lines[i])) { end = i; break } - } - for (let i = start; i < end; i++) { - if (bannedRe.test(lines[i])) { + const allowed = KNOWN_ESCAPES[file] ?? [] + const starts = [] + lines.forEach((line, i) => { + if (/export default function/.test(line) || /^function \w+Page(Desktop|Mobile)\b/.test(line)) starts.push(i) + }) + for (const start of starts) { + // The page body ends at the next top-level declaration (a `function` at + // column 0) — everything after that is a sub-component or helper. + let end = lines.length + for (let i = start + 1; i < lines.length; i++) { + if (/^(function |const [A-Z]\w* = )/.test(lines[i])) { end = i; break } + } + for (let i = start; i < end; i++) { + if (!bannedRe.test(lines[i])) continue + if (allowed.some(name => new RegExp(`\\b${name}\\b`).test(lines[i]))) continue violations.push(`${file}:${i + 1} ${lines[i].trim()}`) } } diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index 29124210c..ff9c2fe90 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -7,7 +7,8 @@ import { server } from '../tests/helpers/msw/server' import { useAuthStore } from './store/authStore' import { useSettingsStore } from './store/settingsStore' import { resetAllStores } from '../tests/helpers/store' -import { buildUser, buildSettings } from '../tests/helpers/factories' +import { buildUser, buildSettings, buildTrip } from '../tests/helpers/factories' +import { offlineDb } from './db/offlineDb' import { SETTINGS_WAIT_MS } from './utils/startDestination' import App from './App' @@ -142,6 +143,30 @@ describe('RootRedirect — startup destination', () => { await waitFor(() => expect(screen.getByText('Dashboard')).toBeInTheDocument()) }) + it('FE-COMP-APP-028b: opens the cached active trip when the launch is offline', async () => { + seedAuth({ isAuthenticated: true, user: buildUser() }) + useSettingsStore.setState({ + isLoaded: true, + settings: buildSettings({ start_page: 'active_trip' }), + }) + const today = new Date() + const iso = (d: Date) => d.toISOString().slice(0, 10) + await offlineDb.trips.put(buildTrip({ id: 42, title: 'Japan', start_date: iso(today), end_date: iso(today) })) + const onLine = Object.getOwnPropertyDescriptor(Navigator.prototype, 'onLine') + Object.defineProperty(navigator, 'onLine', { value: false, configurable: true }) + + // navigator and the Dexie table are shared with every other test in the + // file, so a failed assertion must not leave the rest of them offline. + try { + renderApp('/') + await waitFor(() => expect(screen.getByText('TripPlanner')).toBeInTheDocument()) + } finally { + if (onLine) Object.defineProperty(Navigator.prototype, 'onLine', onLine) + delete (navigator as unknown as { onLine?: boolean }).onLine + await offlineDb.trips.clear() + } + }) + // The whole point of the localStorage mirror: the default launch must not pay // for a lookup it doesn't need. it('FE-COMP-APP-029: never asks for the active trip when starting on the dashboard', async () => { diff --git a/client/src/App.tsx b/client/src/App.tsx index c9fc9edad..523e6150b 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -19,7 +19,8 @@ import ErrorBoundary from './components/shared/ErrorBoundary' import { lazyWithRetry } from './utils/lazyWithRetry' import { useIsPhone } from './mobile/useIsPhone' import { TranslationProvider, useTranslation } from './i18n' -import { authApi, tripsApi } from './api/client' +import { authApi } from './api/client' +import { tripRepo } from './repo/tripRepo' import { readStartDestination, tripStartPath, DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB, SETTINGS_WAIT_MS, START_DESTINATION_ROUTE } from './utils/startDestination' import { usePermissionsStore, PermissionLevel } from './store/permissionsStore' import { useInAppNotificationListener } from './hooks/useInAppNotificationListener.ts' @@ -244,7 +245,9 @@ function RootRedirect() { : (mirrored ?? { page: DEFAULT_START_PAGE, tab: DEFAULT_START_TRIP_TAB }) if (page !== 'active_trip') { setTarget('/dashboard'); return } let cancelled = false - tripsApi.active() + // Through the repo, not the api: offline this answers from Dexie instead of + // bouncing a cached trip to the dashboard. + tripRepo.active() .then(({ trip }) => { if (!cancelled) setTarget(trip ? tripStartPath(trip.id, tab) : '/dashboard') }) .catch(() => { if (!cancelled) setTarget('/dashboard') }) return () => { cancelled = true } diff --git a/client/src/api/client.surface.test.ts b/client/src/api/client.surface.test.ts index e3d76cb2b..bca591154 100644 --- a/client/src/api/client.surface.test.ts +++ b/client/src/api/client.surface.test.ts @@ -1,4 +1,4 @@ -// FE-APISURF-001 to FE-APISURF-052 +// FE-APISURF-001 to FE-APISURF-054 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import type { AxiosResponse } from 'axios' import { http, HttpResponse } from 'msw' @@ -9,7 +9,7 @@ import { tagsApi, categoriesApi, adminApi, addonsApi, pluginsApi, airtrailApi, journeyApi, mapsApi, airportsApi, budgetApi, filesApi, reservationsApi, healthApi, weatherApi, configApi, helpApi, settingsApi, accommodationsApi, dayNotesApi, collabApi, backupApi, - shareApi, transitApi, tripInviteApi, notificationsApi, inAppNotificationsApi, + shareApi, transitApi, tripInviteApi, notificationsApi, inAppNotificationsApi, memoriesApi, } from './client' interface Recorded { method: string; url: string; body: unknown } @@ -396,6 +396,20 @@ describe('client > endpoint wiring', () => { ]) }) + it('FE-APISURF-053: memoriesApi maps the photo-provider endpoints', async () => { + await assertCalls([ + { n: 'status', r: () => memoriesApi.status('immich'), e: 'GET /api/integrations/memories/immich/status' }, + { n: 'search', r: () => memoriesApi.search('immich', { from: '2026-01-01', to: '2026-01-02', page: 1, size: 50 }), e: 'POST /api/integrations/memories/immich/search' }, + { n: 'albums', r: () => memoriesApi.albums('immich'), e: 'GET /api/integrations/memories/immich/albums' }, + { n: 'albumPhotos', r: () => memoriesApi.albumPhotos('immich', 'alb-1'), e: 'GET /api/integrations/memories/immich/albums/alb-1/photos' }, + ]) + }) + + it('FE-APISURF-054: memoriesApi passes the album passphrase as a query parameter', async () => { + const rec = await traceOne(() => memoriesApi.albumPhotos('synologyphotos', 'alb-2', 'p/w?')) + expect(rec.url).toBe('/api/integrations/memories/synologyphotos/albums/alb-2/photos?passphrase=p%2Fw%3F') + }) + it('FE-APISURF-016: mapsApi and airportsApi map the geo endpoints', async () => { await assertCalls([ { n: 'maps.search', r: () => mapsApi.search('Rome'), e: 'POST /api/maps/search' }, diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 79aa8de2f..d3c60819c 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -55,6 +55,7 @@ import { } from '@trek/shared' import { getSocketId } from './websocket' import { probeNow } from '../sync/connectivity' +import { downloadBlob } from '../utils/fileDownload' /** * Validate a response payload against its @trek/shared Zod schema — but only in @@ -968,6 +969,32 @@ export const journeyApi = { getPublicJourney: (token: string) => apiClient.get(`/public/journey/${token}`).then(r => r.data), } +// Photo providers (Immich, Synology Photos, …) behind /api/integrations/memories. +// The provider sits on the user's own hardware and the server proxies through to +// it, so the 8s default does not apply — and neither does any number picked +// here. The server already holds the deadline, and its worst case is longer than +// anything that would look reasonable in this file: a Synology call is 30s, and +// an expired session makes it re-authenticate and try again. A client cap below +// that turns a slow NAS into a NAS that reads as disconnected, which is what +// these calls looked like before they moved off raw fetch (which had no timeout +// either). Callers that need to give up early pass an AbortSignal. +const MEMORIES_TIMEOUT = 0 + +export const memoriesApi = { + status: (provider: string): Promise<{ connected: boolean }> => + apiClient.get(`/integrations/memories/${provider}/status`, { timeout: MEMORIES_TIMEOUT }).then(r => r.data), + search: (provider: string, body: { from: string; to: string; page: number; size: number }, signal?: AbortSignal) => + apiClient.post(`/integrations/memories/${provider}/search`, body, { timeout: MEMORIES_TIMEOUT, signal }).then(r => r.data), + albums: (provider: string) => + apiClient.get(`/integrations/memories/${provider}/albums`, { timeout: MEMORIES_TIMEOUT }).then(r => r.data), + albumPhotos: (provider: string, albumId: string, passphrase?: string, signal?: AbortSignal) => + apiClient.get(`/integrations/memories/${provider}/albums/${albumId}/photos`, { + timeout: MEMORIES_TIMEOUT, + params: passphrase ? { passphrase } : undefined, + signal, + }).then(r => r.data), +} + export const mapsApi = { search: (query: string, lang?: string) => apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => checkInDev(mapsSearchResultSchema, r.data, 'maps.search')), autocomplete: (input: string, lang?: string, locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, signal?: AbortSignal, sessionToken?: string) => @@ -1144,13 +1171,7 @@ export const backupApi = { credentials: 'include', }) if (!res.ok) throw new Error('Download failed') - const blob = await res.blob() - const url = URL.createObjectURL(blob) - const a = document.createElement('a') - a.href = url - a.download = filename - a.click() - URL.revokeObjectURL(url) + downloadBlob(await res.blob(), filename) }, delete: (filename: string) => apiClient.delete(`/backup/${filename}`).then(r => r.data), restore: (filename: string) => apiClient.post(`/backup/restore/${filename}`).then(r => r.data), diff --git a/client/src/api/oauthScopes.test.ts b/client/src/api/oauthScopes.test.ts index 443c30b64..81712c63f 100644 --- a/client/src/api/oauthScopes.test.ts +++ b/client/src/api/oauthScopes.test.ts @@ -1,7 +1,20 @@ -// FE-OAUTH-SCOPES-001 to FE-OAUTH-SCOPES-010 +// FE-OAUTH-SCOPES-001 to FE-OAUTH-SCOPES-011 import { describe, it, expect } from 'vitest' +import { readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' import { SCOPE_GROUPS, ALL_SCOPES, SCOPE_GROUP_NAMES, getScopesByGroup } from './oauthScopes' +// The consent page mirrors the server's scope list by hand, so read the server +// file and compare. Parsing keeps this a client-only test (no server import). +function readServerScopes(): string[] { + const here = dirname(fileURLToPath(import.meta.url)) + const src = readFileSync(resolve(here, '../../../server/src/mcp/scopes.ts'), 'utf8') + const block = src.slice(src.indexOf('export const SCOPES = {')) + const body = block.slice(0, block.indexOf('} as const;')) + return [...body.matchAll(/^\s*[A-Z_]+:\s*'([a-z]+:[a-z]+)'/gm)].map(m => m[1]) +} + describe('SCOPE_GROUPS', () => { it('FE-OAUTH-SCOPES-001: contains all expected scope keys', () => { const expected = [ @@ -34,8 +47,16 @@ describe('SCOPE_GROUPS', () => { }) describe('ALL_SCOPES', () => { - it('FE-OAUTH-SCOPES-003: contains exactly 29 scopes', () => { - expect(ALL_SCOPES).toHaveLength(29) + it('FE-OAUTH-SCOPES-003: contains exactly as many scopes as the server defines', () => { + expect(ALL_SCOPES).toHaveLength(readServerScopes().length) + }) + + it('FE-OAUTH-SCOPES-011: matches server/src/mcp/scopes.ts exactly', () => { + const serverScopes = readServerScopes() + // Guards the parse itself: a moved file or a reformatted list must fail + // here rather than quietly compare two empty sets. + expect(serverScopes.length).toBeGreaterThan(0) + expect(new Set(ALL_SCOPES)).toEqual(new Set(serverScopes)) }) it('FE-OAUTH-SCOPES-004: matches Object.keys(SCOPE_GROUPS)', () => { diff --git a/client/src/components/Admin/AdminPluginsPanel.test.tsx b/client/src/components/Admin/AdminPluginsPanel.test.tsx index 7fff27480..371df1556 100644 --- a/client/src/components/Admin/AdminPluginsPanel.test.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.test.tsx @@ -1862,3 +1862,43 @@ describe('AdminPluginsPanel signature fingerprints and dismissals', () => { await waitFor(() => expect(uploaded).toBe(false)) }) }) + +// FE-W5PLG-030 — Update all drives the same single-slot busy flag as a per-row update, +// so the requests have to go out one after another. +describe('AdminPluginsPanel — update all', () => { + const updatableRow = (over: Record = {}) => plugin({ + id: 'a-widget', name: 'Alpha Widget', type: 'widget', version: '1.0.0', operatorEgress: false, ...over, + }) + + it('FE-W5PLG-030: Update all sends the second update only after the first came back', async () => { + const started: string[] = [] + let release = () => {} + const gate = new Promise(resolve => { release = resolve }) + server.use( + http.get('*/api/admin/plugins', () => HttpResponse.json({ + enabled: true, devLink: false, + plugins: [updatableRow(), updatableRow({ id: 'b-widget', name: 'Beta Widget' })], + })), + http.get('*/api/admin/plugins/registry', () => HttpResponse.json([ + { id: 'a-widget', name: 'Alpha Widget', author: 'acme', description: 'd', type: 'widget', latest: '2.0.0' }, + { id: 'b-widget', name: 'Beta Widget', author: 'acme', description: 'd', type: 'widget', latest: '2.0.0' }, + ])), + http.post('*/api/admin/plugins/:id/update', async ({ params }) => { + started.push(String(params.id)) + if (started.length === 1) await gate + return HttpResponse.json({ version: '2.0.0', activated: true, newPermissions: [], newEgress: [] }) + }), + ) + render() + + fireEvent.click(await screen.findByRole('button', { name: /update all/i })) + + await waitFor(() => expect(started).toHaveLength(1)) + // Give a parallel dispatch every chance to show up before releasing the first. + await new Promise(resolve => setTimeout(resolve, 20)) + expect(started).toEqual(['a-widget']) + + release() + await waitFor(() => expect(started).toEqual(['a-widget', 'b-widget'])) + }) +}) diff --git a/client/src/components/Admin/AdminPluginsPanel.tsx b/client/src/components/Admin/AdminPluginsPanel.tsx index 7342da053..4ca8ae1f1 100644 --- a/client/src/components/Admin/AdminPluginsPanel.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.tsx @@ -641,7 +641,7 @@ export default function AdminPluginsPanel() { const runUpdate = (p: PluginRow) => { setBusy(p.id); setMenu(null) - adminApi.pluginUpdate(p.id) + return adminApi.pluginUpdate(p.id) .then((r: { version: string; activated: boolean; newPermissions: string[]; newEgress: string[] }) => { if (r.activated || (r.newPermissions.length === 0 && r.newEgress.length === 0)) toast.success(t('admin.plugins.updated')) else setConsentQueue(qq => [...qq, { plugin: p, version: r.version, newPermissions: r.newPermissions, newEgress: r.newEgress }]) @@ -676,6 +676,12 @@ export default function AdminPluginsPanel() { // eslint-disable-next-line react-hooks/exhaustive-deps const updatable = useMemo(() => plugins.filter(updateAvailable), [plugins, latest]) + // One after another: busy is a single slot, so parallel updates would leave the + // other rows clickable mid-install and refresh once per plugin. + const updateAll = async () => { + for (const p of updatable) await runUpdate(p) + } + // Installed list after search / type / status filters + sort. const shownInstalled = useMemo(() => { const term = q.trim().toLowerCase() @@ -865,7 +871,7 @@ export default function AdminPluginsPanel() {
{t('admin.plugins.updatesAvailable', { count: updatable.length })} - diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx index b9ffcf8eb..c8ad05d7b 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx @@ -1,4 +1,4 @@ -// FE-W4BGT-001 to FE-W4BGT-020 +// FE-W4BGT-001 to FE-W4BGT-022 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { screen, act, waitFor } from '@testing-library/react' import { render, fireEvent } from '../../../tests/helpers/render' @@ -185,6 +185,37 @@ describe('BackgroundTasksWidget — rehydrate', () => { }) }) +describe('BackgroundTasksWidget — poll backstop', () => { + it('FE-W4BGT-021: a job the server drops mid-poll ends as an error instead of spinning on', async () => { + vi.useFakeTimers() + useBackgroundTasksStore.setState({ tasks: [task({ status: 'running', items: undefined })] }) + vi.mocked(reservationsApi.importJobStatus) + .mockResolvedValueOnce({ status: 'running', done: 0, total: 1 } as never) + .mockRejectedValue({ response: { status: 404 } }) + + render() + await act(async () => { await vi.advanceTimersByTimeAsync(5000) }) + + expect(useBackgroundTasksStore.getState().tasks[0].status).toBe('error') + // The file parsed fine — blaming it would send the user looking for a problem + // that isn't there. + expect(useBackgroundTasksStore.getState().tasks[0].error).toBe('Unknown error') + }) + + it('FE-W4BGT-022: a transient poll failure leaves the job running', async () => { + vi.useFakeTimers() + useBackgroundTasksStore.setState({ tasks: [task({ status: 'running', items: undefined })] }) + vi.mocked(reservationsApi.importJobStatus) + .mockResolvedValueOnce({ status: 'running', done: 0, total: 1 } as never) + .mockRejectedValue({ response: { status: 500 } }) + + render() + await act(async () => { await vi.advanceTimersByTimeAsync(5000) }) + + expect(useBackgroundTasksStore.getState().tasks[0].status).toBe('running') + }) +}) + describe('BackgroundTasksWidget — AI retry', () => { const withFiles = () => task({ items: [], sourceFiles: [new File(['%PDF'], 'voucher.pdf', { type: 'application/pdf' })], diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx index 9d3a5b79f..19c34b2a1 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx @@ -111,11 +111,16 @@ export default function BackgroundTasksWidget() { else if (s.status === 'error') setError(task.id, task.tripId, s.error ?? 'error') else setProgress(task.id, task.tripId, s.done, s.total) }) - .catch(() => {}) + .catch((err: { response?: { status?: number } }) => { + // The server 404s a job once it has expired (or after a restart). Nothing is + // ever coming, so end the card instead of spinning and re-polling forever. + // Not the parse-failure message — the file was fine, the job is just gone. + if (err?.response?.status === 404) setError(task.id, task.tripId, t('common.unknownError')) + }) } }, 5000) return () => clearInterval(iv) - }, [tasks, setProgress, setDone, setError]) + }, [tasks, setProgress, setDone, setError, t]) if (tasks.length === 0) return null diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index bba2289e5..3a5a8cff8 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -10,7 +10,7 @@ import { useSettingsStore } from '../../store/settingsStore' import { usePermissionsStore } from '../../store/permissionsStore' import { clearExchangeRateCache } from '../../hooks/useExchangeRates' import { resetAllStores, seedStore } from '../../../tests/helpers/store' -import { buildUser, buildTrip, buildBudgetItem } from '../../../tests/helpers/factories' +import { buildUser, buildTrip, buildBudgetItem, buildSettings } from '../../../tests/helpers/factories' import type { BudgetItem } from '../../types' import CostsPanel, { ExpenseModal } from './CostsPanel' import { splitEqualShares, calculateTicketShares, type TicketItem } from './CostsPanel.helpers' @@ -594,7 +594,7 @@ describe('CostsPanel — settlements in the ledger', () => { expect(screen.getByDisplayValue('100,00')).toBeDisabled() - expect(screen.getByText('Individual Shares Summary')).toBeInTheDocument() + expect(screen.getByText('Individual shares')).toBeInTheDocument() expect(screen.getByText(/75\.00/)).toBeInTheDocument() expect(screen.getByText(/25\.00/)).toBeInTheDocument() @@ -916,6 +916,50 @@ describe('CostsPanel — settle up', () => { delete window.__addToast }) + it('FE-W5COSTS-059: a settle-up that fails midway still reloads the transfers that went through', async () => { + const addToast = vi.fn() + window.__addToast = addToast as unknown as typeof window.__addToast + let posted = 0 + let reads = 0 + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [] })), + http.get('/api/trips/1/budget/settlement', () => { + reads += 1 + return HttpResponse.json({ balances: [], flows: posted > 0 ? [flows[1]] : flows, settlements: [] }) + }), + http.post('/api/trips/1/budget/settlements', () => { + posted += 1 + return posted === 1 ? HttpResponse.json({ settlement: { id: 1 } }) : HttpResponse.json({ error: 'no' }, { status: 500 }) + }), + ) + render() + + const settleAll = await screen.findByRole('button', { name: 'Settle up' }) + await waitFor(() => expect(settleAll).not.toBeDisabled()) + const readsBefore = reads + fireEvent.click(settleAll) + + await waitFor(() => expect(addToast).toHaveBeenCalledWith('Unknown error', 'error', undefined)) + // The failed second transfer must not swallow the refresh: bob's 45 € was + // recorded, and leaving it listed invites a second, doubled settle-up. + await waitFor(() => expect(reads).toBeGreaterThan(readsBefore)) + expect(posted).toBe(2) + delete window.__addToast + }) + + it('FE-W5COSTS-060: a settlement read that fails says so instead of claiming everyone is square', async () => { + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ error: 'no' }, { status: 500 })), + ) + render() + + await screen.findByText('Balances') + // Settle up and Balances both hang off the one failed request. + await waitFor(() => expect(screen.getAllByText('Unknown error')).toHaveLength(2)) + expect(screen.queryByText("Everyone's square")).toBeNull() + }) + it('FE-W5COSTS-013: the "you owe" card lists who I still have to pay', async () => { mount([], { flows: [{ from: { user_id: 1, username: 'alice' }, to: { user_id: 2, username: 'bob' }, amount: 20 }] }) @@ -1394,6 +1438,38 @@ describe('CostsPanel — expense modal', () => { expect(onSaved).toHaveBeenCalled() }) + // The mobile sheet already dates a new expense by the traveller's own clock; + // the desktop modal filed it under the UTC day, so the same expense entered + // late in Tokyo or early in Los Angeles landed on a different day per surface. + it('FE-W5COSTS-065: a new expense is dated by the local calendar day, not the UTC one', async () => { + vi.useFakeTimers({ toFake: ['Date'] }) + // A wall-clock time on whichever side of midnight puts the runner's UTC date + // on a different day. On UTC itself the two spellings agree and there is + // nothing here to catch. + const behindUtc = new Date(2026, 7, 12).getTimezoneOffset() > 0 + vi.setSystemTime(new Date(2026, 7, 12, behindUtc ? 23 : 1, 30, 0)) + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) + let posted: Record | null = null + server.use(http.post('/api/trips/1/budget', async ({ request }) => { + posted = await request.json() as Record + return HttpResponse.json({ item: dinner() }) + })) + render( + {}} onSaved={vi.fn()} /> + ) + + await user.click(screen.getByRole('button', { name: 'Add expense' })) + + try { + await waitFor(() => expect(posted).toBeTruthy()) + expect(posted!.expense_date).toBe('2026-08-12') + } finally { + vi.useRealTimers() + } + }) + it('FE-W5COSTS-038b: a prefill from a place links the expense to that place (#1298)', async () => { const user = userEvent.setup() let posted: Record | null = null @@ -1447,6 +1523,26 @@ describe('CostsPanel — remaining paths', () => { createObjURL.mockRestore(); revokeObjURL.mockRestore(); clickSpy.mockRestore() }) + it('FE-W5COSTS-061: the CSV export neutralises names a spreadsheet would run as a formula', async () => { + seedStore(useTripStore, { trip: buildTrip({ id: 1, currency: 'EUR', title: 'Rome' }) }) + let exported: Blob | null = null + const createObjURL = vi.spyOn(URL, 'createObjectURL').mockImplementation(b => { exported = b as Blob; return 'blob:mock' }) + const revokeObjURL = vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}) + const clickSpy = vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(() => {}) + mount([ + expense({ id: 204, name: '=HYPERLINK("http://evil","click")', category: 'food', total_price: 12, expense_date: '2025-06-15', note: '@SUM(A1:A9)' }), + expense({ id: 205, name: '-5 refund', category: 'misc', total_price: 5, expense_date: '2025-06-16', note: null }), + ]) + + await screen.findByText('-5 refund') + fireEvent.click(screen.getByTitle('Export CSV')) + + const lines = (await exported!.text()).replace(/^\uFEFF/, '').split('\r\n') + expect(lines[1]).toBe('06/15/2025;"\'=HYPERLINK(""http://evil"",""click"")";Food & drink;12.00;EUR;12.00;\'@SUM(A1:A9)') + expect(lines[2]).toBe('06/16/2025;\'-5 refund;Other;5.00;EUR;5.00;') + createObjURL.mockRestore(); revokeObjURL.mockRestore(); clickSpy.mockRestore() + }) + it('FE-W5COSTS-043: deleting an expense drops it from the ledger', async () => { let deleted = false server.use(http.delete('/api/trips/1/budget/101', () => { deleted = true; return HttpResponse.json({ success: true }) })) @@ -1615,7 +1711,7 @@ describe('CostsPanel — remaining paths', () => { await user.click(bob) await user.click(bob) // 10.99 across two people leaves the odd cent with the lower user id. - expect(screen.getByText('Individual Shares Summary')).toBeInTheDocument() + expect(screen.getByText('Individual shares')).toBeInTheDocument() expect(screen.getByText('€5.50')).toBeInTheDocument() expect(screen.getByText('€5.49')).toBeInTheDocument() }) @@ -1741,6 +1837,21 @@ describe('CostsPanel — mobile extras', () => { expect(await screen.findByPlaceholderText('0.00')).toBeInTheDocument() }) + it('FE-W5COSTS-062: the mobile search box keeps the caret across keystrokes', async () => { + const user = userEvent.setup() + mount([dinner(), taxi()]) + + await screen.findByText('Taxi') + const box = screen.getByPlaceholderText('Search expenses…') + await user.type(box, 'din') + + // The whole word has to land: if the body is remounted per keystroke the input + // loses focus and everything after the first character goes nowhere. + expect((box as HTMLInputElement).value).toBe('din') + expect(document.activeElement).toBe(box) + expect(screen.queryByText('Taxi')).not.toBeInTheDocument() + }) + it('FE-W5COSTS-054: an unknown currency degrades to a plain amount on mobile too', async () => { seedStore(useSettingsStore, { settings: { ...useSettingsStore.getState().settings, default_currency: '' } }) seedStore(useTripStore, { trip: buildTrip({ id: 1, currency: 'XX' }) }) @@ -1774,3 +1885,58 @@ describe('CostsPanel — split maths', () => { expect(calculateTicketShares(items)).toEqual({ shares: { 1: 7.5, 2: 7.5 }, total: 15 }) }) }) + +// The reworked expense modal shipped with English written straight into the JSX, +// so the receipt panel and the split summary stayed English on a German phone +// while everything around them translated. +describe('CostsPanel — expense modal in another language', () => { + beforeEach(() => { + seedAlice() + seedStore(useSettingsStore, { settings: buildSettings({ language: 'de', default_currency: 'EUR' }) }) + clearExchangeRateCache() + }) + afterEach(clearExchangeRateCache) + + const openModal = () => render( + {}} onSaved={vi.fn()} /> + ) + + it('FE-W5COSTS-063: the receipt panel is translated', async () => { + const user = userEvent.setup() + openModal() + + // The locale bundle is fetched, so the first paint is still English. + await user.click(await screen.findByRole('button', { name: 'Beleg' })) + await user.click(screen.getByRole('button', { name: /Artikel hinzufügen/ })) + + expect(screen.getByPlaceholderText('Artikelname')).toBeInTheDocument() + expect(screen.getByText('Aufteilen auf:')).toBeInTheDocument() + expect(screen.getByText('Anteil pro Person')).toBeInTheDocument() + expect(screen.queryByText('Individual shares')).not.toBeInTheDocument() + }) + + it('FE-W5COSTS-064: the split summary and the excluded marker are translated', async () => { + const user = userEvent.setup() + openModal() + + await user.type(await screen.findByPlaceholderText('0,00'), '90') + await user.click(screen.getByRole('button', { name: 'Individuell' })) + + // Nothing entered yet, so the whole 90 is still unaccounted for. + expect(screen.getByText('Summe der Anteile: €0.00 von €90.00 (es fehlen €90.00)')).toBeInTheDocument() + + const shares = screen.getAllByPlaceholderText('45,00') + await user.type(shares[0], '90') + expect(screen.getByText('Aufteilung passt zur Summe')).toBeInTheDocument() + + await user.type(shares[1], '10') + expect(screen.getByText('Summe der Anteile: €100.00 von €90.00 (€10.00 zu viel)')).toBeInTheDocument() + + // Back to the equal split: a member taken out of it is marked, not dropped. + await user.click(screen.getByRole('button', { name: 'Gleichmäßig' })) + await user.click(screen.getByRole('button', { name: /bob/i })) + expect(screen.getByText('Nicht dabei')).toBeInTheDocument() + expect(screen.queryByText('Excluded')).not.toBeInTheDocument() + }) +}) diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index b00d754ed..d1badfa7a 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useMemo, useCallback } from 'react' +import { Fragment, useState, useEffect, useMemo, useCallback } from 'react' import { useSearchParams } from 'react-router' import { ArrowDown, ArrowUp, BarChart3, Plus, Search, ArrowRight, ArrowLeftRight, Check, RotateCcw, Pencil, Trash2, AlertCircle, Download, StickyNote, ChevronDown } from 'lucide-react' import { useTripStore } from '../../store/tripStore' @@ -15,6 +15,7 @@ import { downloadBlob } from '../../utils/fileDownload' import Modal from '../shared/Modal' import CustomSelect from '../shared/CustomSelect' import { CustomDatePicker } from '../shared/CustomDateTimePicker' +import { localToday } from '../Planner/today' import { SYMBOLS, currenciesWith, SPLIT_COLORS } from './BudgetPanel.constants' import { calculateTicketShares, hasTicketSplit, NOTE_MAX, payersBalanced, readTicketItems, readUserNote, rebalancePayers, splitEqualShares, writeTicketItems, type TicketItem } from './CostsPanel.helpers' import { COST_CATEGORY_LIST, catMeta } from './costsCategories' @@ -74,6 +75,9 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const { convert } = useExchangeRates(base) const curOf = useCallback((e: BudgetItem) => (e.currency || tripCurrency), [tripCurrency]) const [settlement, setSettlement] = useState(null) + // A failed settlement read leaves `settlement` null, which the empty views would + // otherwise present as "everyone is square", a balance claim we cannot make. + const [settlementError, setSettlementError] = useState(false) const [filter, setFilter] = useState<'all' | 'mine' | 'owed'>('all') const [search, setSearch] = useState('') const [catFilter, setCatFilter] = useState('') // '' = all categories @@ -99,7 +103,9 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const fmt0 = useCallback((v: number, c = base) => formatMoney(v, c, locale, { decimals: 0 }), [base, locale]) const loadSettlement = useCallback(() => { - budgetApi.settlement(tripId, base).then(setSettlement).catch(() => {}) + budgetApi.settlement(tripId, base) + .then(s => { setSettlement(s); setSettlementError(false) }) + .catch(() => setSettlementError(true)) }, [tripId, base]) useEffect(() => { loadBudgetItems(tripId); loadSettlement() }, [tripId]) @@ -219,8 +225,10 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps if (!flows.length) return try { for (const f of flows) await budgetApi.createSettlement(tripId, { from_user_id: f.from.user_id, to_user_id: f.to.user_id, amount: f.amount, currency: base }) - loadSettlement() } catch { toast.error(t('common.unknownError')) } + // Refresh even when one transfer failed: the ones created before it are real, + // and leaving them in the flow list invites a second, doubled settle-up. + finally { loadSettlement() } } const dateMeta = useMemo(() => { @@ -241,7 +249,13 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps // Costs rework (#1500). One row per expense, oldest first. const handleExportCsv = () => { const sep = ';' - const esc = (v: unknown) => { const s = String(v ?? ''); return s.includes(sep) || s.includes('"') || s.includes('\n') ? '"' + s.replace(/"/g, '""') + '"' : s } + // A cell starting with =, +, -, @, TAB or CR is evaluated as a formula by Excel + // and Sheets, and the name/note columns are free text any trip member can write. + const esc = (v: unknown) => { + let s = String(v ?? '') + if (/^[=+\-@\t\r]/.test(s)) s = "'" + s + return s.includes(sep) || s.includes('"') || s.includes('\n') ? '"' + s.replace(/"/g, '""') + '"' : s + } const fmtDate = (iso: string) => { if (!iso) return ''; try { return new Date(iso + 'T00:00:00Z').toLocaleDateString(locale, { day: '2-digit', month: '2-digit', year: 'numeric', timeZone: 'UTC' }) } catch { return iso } } const header = ['Date', 'Name', 'Category', 'Amount', 'Currency', 'Amount (' + base + ')', 'Note'] @@ -310,7 +324,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps return (
- {isMobile ? : ( + {isMobile ? MobileBody() : (
{/* ── Header bar ── */}
@@ -422,8 +436,8 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps )}
{g.entries.map(en => en.kind === 'expense' - ? - : )} + ? {ExpenseRow({ e: en.e })} + : {SettlementRow({ s: en.s })})}
) @@ -444,19 +458,19 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps )}
- + {SettleFlows()}
{/* balances */}
{t('costs.balances')}
- + {BalancesList({ balances: settlement?.balances || [] })}
{/* by category */}
{t('costs.byCategory')}
- + {CategoryBreakdown()}
@@ -520,8 +534,15 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps ) + // Settle-up and Balances both come from the one settlement request, so they + // share the notice that says the numbers are missing rather than zero. + function loadFailed() { + return
{t('common.unknownError')}
+ } + // ── shared settle-flow list ────────────────────────────────────────────── function SettleFlows() { + if (settlementError) return loadFailed() const flows = settlement?.flows || [] if (flows.length === 0) return (
@@ -548,6 +569,14 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps } // ── mobile layout (Budget1Mobile.html): single flat column, total card on top ── + // + // Called as a plain function — `{MobileBody()}`, not `` — and the + // same goes for ExpenseRow, SettleFlows, BalancesList and CategoryBreakdown. + // A component declared inside this one is a fresh component type on every + // render, so React tears the old subtree down and mounts a new one instead of + // updating it; the search box then loses focus after every keystroke. Calling + // them keeps the markup inline where it always was. Do not "tidy" these back + // into elements. function MobileBody() { return (
@@ -602,7 +631,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps )}
- + {SettleFlows()}
{/* Expenses */} @@ -637,8 +666,8 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps
{!dayFilter &&
{g.day}{t('costs.spent', { amount: fmt(dtot) })}
}
{g.entries.map(en => en.kind === 'expense' - ? - : )}
+ ? {ExpenseRow({ e: en.e })} + : {SettlementRow({ s: en.s })})}
) })} @@ -647,13 +676,13 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps {/* Balances */}
{t('costs.balances')}
- + {BalancesList({ balances: settlement?.balances || [] })}
{/* By category */}
{t('costs.byCategory')}
- + {CategoryBreakdown()}
) @@ -682,6 +711,8 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps ) } + // Called, not rendered — see MobileBody: an element here would remount the + // list on every keystroke in the search box. function ExpenseRow({ e }: { e: BudgetItem }) { const c = catMeta(e.category) const Icon = c.Icon @@ -775,7 +806,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps - {canEdit && { setEditing(e); setModalOpen(true) }} onDelete={() => handleDelete(e.id)} deleteLabel={t('common.delete')} />} + {canEdit && RowActions({ onEdit: () => { setEditing(e); setModalOpen(true) }, onDelete: () => handleDelete(e.id), deleteLabel: t('common.delete') })} ) } @@ -817,6 +848,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps } function BalancesList({ balances }: { balances: SettlementData['balances'] }) { + if (settlementError) return loadFailed() const rows = people.map(p => balances.find(b => b.user_id === p.id) || { user_id: p.id, username: p.username, avatar_url: null, balance: 0 }) const max = Math.max(1, ...rows.map(r => Math.abs(r.balance))) return ( @@ -1017,7 +1049,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo const [name, setName] = useState(editing?.name || prefill?.name || '') const [cat, setCat] = useState(editing ? catMeta(editing.category).key : (prefill?.category || 'food')) const [currency, setCurrency] = useState((editing?.currency || base).toUpperCase()) - const [day, setDay] = useState(editing?.expense_date || new Date().toISOString().slice(0, 10)) + const [day, setDay] = useState(editing?.expense_date || localToday()) const [note, setNote] = useState(() => readUserNote(editing)) const [total, setTotal] = useState(() => { if (editing) return editing.total_price ? String(cleanAmount(editing.total_price)) : '' @@ -1445,7 +1477,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo
handleUpdateItemName(item.id, e.target.value)} className="bg-surface-input border border-edge text-content" @@ -1468,7 +1500,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo
- Splitting: + {t('costs.ticketSplitting')} {people.map((p, pIdx) => { const active = item.participants.has(p.id) return ( @@ -1492,12 +1524,12 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo
{ticketItems.length > 0 && (
-
Individual Shares Summary
+
{t('costs.ticketShares')}
{people.map(p => { const share = ticketInfo.shares[p.id] || 0 @@ -1532,7 +1564,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo {sym(currency)}{(equalShares[p.id] || 0).toFixed(2)} ) : ( - Excluded + {t('costs.excluded')} ) ) : ( on ? ( @@ -1557,9 +1589,13 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo ) : ( - {customBalanced - ? 'Split matches total' - : `Sum of splits: ${sym(currency)}${splitSum.toFixed(2)} of ${sym(currency)}${totalNum.toFixed(2)} (${(totalNum - splitSum) > 0 ? 'under by' : 'over by'} ${sym(currency)}${Math.abs(totalNum - splitSum).toFixed(2)})`} + {customBalanced + ? t('costs.splitBalanced') + : t(totalNum - splitSum > 0 ? 'costs.splitSumUnder' : 'costs.splitSumOver', { + sum: sym(currency) + splitSum.toFixed(2), + total: sym(currency) + totalNum.toFixed(2), + diff: sym(currency) + Math.abs(totalNum - splitSum).toFixed(2), + })} )}
diff --git a/client/src/components/Collab/CollabChatLinkPreview.tsx b/client/src/components/Collab/CollabChatLinkPreview.tsx index b8a722645..1f201750e 100644 --- a/client/src/components/Collab/CollabChatLinkPreview.tsx +++ b/client/src/components/Collab/CollabChatLinkPreview.tsx @@ -2,7 +2,12 @@ import { useState, useEffect } from 'react' import { collabApi } from '../../api/client' /* ── Link Preview ── */ -const previewCache = {} +// linkPreview is trip-scoped on the server, so the cache key has to be too. +// Capped as well: a long chat session would otherwise keep every preview it ever +// rendered for as long as the tab lives. +const previewCache = new Map() +const cacheKey = (tripId: number, url: string): string => `${tripId} ${url}` +const MAX_CACHED_PREVIEWS = 200 interface LinkPreviewProps { url: string @@ -12,17 +17,23 @@ interface LinkPreviewProps { } export function LinkPreview({ url, tripId, own, onLoad }: LinkPreviewProps) { - const [data, setData] = useState(previewCache[url] || null) - const [loading, setLoading] = useState(!previewCache[url]) + const [data, setData] = useState(previewCache.get(cacheKey(tripId, url)) || null) + const [loading, setLoading] = useState(!previewCache.has(cacheKey(tripId, url))) useEffect(() => { - if (previewCache[url]) return + const key = cacheKey(tripId, url) + const cached = previewCache.get(key) + if (cached) { setData(cached); setLoading(false); return } + let current = true collabApi.linkPreview(tripId, url).then(d => { - previewCache[url] = d + if (previewCache.size >= MAX_CACHED_PREVIEWS) previewCache.delete(previewCache.keys().next().value) + previewCache.set(key, d) + if (!current) return setData(d) setLoading(false) if (d?.title || d?.description || d?.image) onLoad?.() - }).catch(() => setLoading(false)) + }).catch(() => { if (current) setLoading(false) }) + return () => { current = false } }, [url, tripId]) if (loading || !data || (!data.title && !data.description && !data.image)) return null diff --git a/client/src/components/Collab/CollabChatSmall.test.tsx b/client/src/components/Collab/CollabChatSmall.test.tsx index 43269b718..2437ee9c7 100644 --- a/client/src/components/Collab/CollabChatSmall.test.tsx +++ b/client/src/components/Collab/CollabChatSmall.test.tsx @@ -1,4 +1,4 @@ -// FE-W4CCS-001 to FE-W4CCS-016 +// FE-W4CCS-001 to FE-W4CCS-019 import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor } from '../../../tests/helpers/render' import type { ChatReaction } from './CollabChat.types' @@ -11,6 +11,7 @@ import { TwemojiImg } from './CollabChatTwemojiImg' import { ReactionBadge } from './CollabChatReactionBadge' import { UserAvatar } from './CollabNotesUserAvatar' import { WebsiteThumbnail } from './CollabNotesWebsiteThumbnail' +import { LinkPreview } from './CollabChatLinkPreview' function reaction(overrides: Partial = {}): ChatReaction { return { emoji: '👍', count: 1, users: [{ id: 1, username: 'ada' }], ...overrides } as unknown as ChatReaction @@ -147,13 +148,26 @@ describe('WebsiteThumbnail', () => { expect(screen.getByText('example.com')).toBeInTheDocument() }) - it('FE-W4CCS-014: falls back to a link label for an unparseable url and a failing preview', async () => { + it('FE-W4CCS-014: keeps the domain chip when the preview request fails', async () => { linkPreview.mockRejectedValue(new Error('blocked')) - render() + render() - expect(screen.getByText('link')).toBeInTheDocument() + expect(screen.getByText('blocked.example')).toBeInTheDocument() await waitFor(() => expect(linkPreview).toHaveBeenCalled()) - expect(screen.getByRole('link')).toHaveAttribute('title', 'not a url') + expect(screen.getByRole('link')).toHaveAttribute('title', 'https://blocked.example/a') + }) + + it('FE-W4CCS-017: a url that is not http(s) is shown but neither linked nor previewed', async () => { + const { unmount } = render() + + expect(screen.queryByRole('link')).toBeNull() + expect(screen.getByTitle('javascript:alert(1)')).toBeInTheDocument() + unmount() + + render() + expect(screen.getByText('link')).toBeInTheDocument() + expect(screen.queryByRole('link')).toBeNull() + expect(linkPreview).not.toHaveBeenCalled() }) it('FE-W4CCS-015: caches per trip, because the preview endpoint is trip-scoped', async () => { @@ -185,3 +199,34 @@ describe('WebsiteThumbnail', () => { expect(container.querySelector('img')).toHaveAttribute('src', 'https://cdn.example/b.png') }) }) + +describe('LinkPreview', () => { + it('FE-W4CCS-018: caches per trip, because the preview endpoint is trip-scoped', async () => { + const shared = 'https://example.com/preview' + linkPreview.mockResolvedValue({ title: 'Trip 4', image: null }) + const first = render() + expect(await screen.findByText('Trip 4')).toBeInTheDocument() + first.unmount() + + linkPreview.mockResolvedValue({ title: 'Trip 9', image: null }) + render() + + expect(await screen.findByText('Trip 9')).toBeInTheDocument() + expect(linkPreview).toHaveBeenCalledTimes(2) + expect(linkPreview).toHaveBeenLastCalledWith(9, shared) + }) + + it('FE-W4CCS-019: a preview that resolves after the url changed is not shown', async () => { + let resolveFirst: (v: { title: string; image: string | null }) => void = () => {} + linkPreview.mockImplementationOnce(() => new Promise(res => { resolveFirst = res })) + const { rerender } = render() + + linkPreview.mockResolvedValue({ title: 'Second', image: null }) + rerender() + expect(await screen.findByText('Second')).toBeInTheDocument() + + resolveFirst({ title: 'First', image: null }) + await waitFor(() => expect(screen.queryByText('First')).toBeNull()) + expect(screen.getByText('Second')).toBeInTheDocument() + }) +}) diff --git a/client/src/components/Collab/CollabNotes.test.tsx b/client/src/components/Collab/CollabNotes.test.tsx index 618699e98..dcd9f9bbd 100644 --- a/client/src/components/Collab/CollabNotes.test.tsx +++ b/client/src/components/Collab/CollabNotes.test.tsx @@ -271,6 +271,7 @@ describe('CollabNotes', () => { const listener = calls[calls.length - 1][0]; act(() => { listener({ + tripId: 1, type: 'collab:note:created', note: { id: 50, trip_id: 1, user_id: 1, author_username: 'testuser', author_avatar: null, @@ -301,7 +302,7 @@ describe('CollabNotes', () => { const calls = (addListener as ReturnType).mock.calls; const listener = calls[calls.length - 1][0]; act(() => { - listener({ type: 'collab:note:deleted', noteId: 7 }); + listener({ tripId: 1, type: 'collab:note:deleted', noteId: 7 }); }); await waitFor(() => expect(screen.queryByText('WS Delete')).not.toBeInTheDocument()); }); @@ -463,6 +464,7 @@ describe('CollabNotes', () => { const listener = calls[calls.length - 1][0]; act(() => { listener({ + tripId: 1, type: 'collab:note:updated', note: { id: 5, trip_id: 1, user_id: 1, author_username: 'testuser', author_avatar: null, @@ -1390,7 +1392,7 @@ describe('CollabNotes details', () => { await screen.findByText('Already here'); const handler = wsHandler(); await act(async () => { - handler({ type: 'collab:note:created', note: buildNote({ id: 4, title: 'Already here' }) }); + handler({ tripId: 1, type: 'collab:note:created', note: buildNote({ id: 4, title: 'Already here' }) }); }); expect(screen.getAllByText('Already here')).toHaveLength(1); }); @@ -1403,7 +1405,7 @@ describe('CollabNotes details', () => { await screen.findByText('Second'); const handler = wsHandler(); await act(async () => { - handler({ type: 'collab:note:updated', note: { id: 2, title: 'Second renamed' } }); + handler({ tripId: 1, type: 'collab:note:updated', note: { id: 2, title: 'Second renamed' } }); }); expect(await screen.findByText('Second renamed')).toBeInTheDocument(); expect(screen.getByText('First')).toBeInTheDocument(); @@ -1414,9 +1416,9 @@ describe('CollabNotes details', () => { render(); await screen.findByText('Doomed'); const handler = wsHandler(); - await act(async () => { handler({ type: 'collab:note:deleted' }); }); + await act(async () => { handler({ tripId: 1, type: 'collab:note:deleted' }); }); expect(screen.getByText('Doomed')).toBeInTheDocument(); - await act(async () => { handler({ type: 'collab:note:deleted', id: 9 }); }); + await act(async () => { handler({ tripId: 1, type: 'collab:note:deleted', id: 9 }); }); await waitFor(() => expect(screen.queryByText('Doomed')).not.toBeInTheDocument()); }); @@ -1803,6 +1805,59 @@ describe('CollabNotes details', () => { expect(screen.getByText('Older note')).toBeInTheDocument(); }); + it('FE-W5CNT-030: a WebSocket event for another trip is ignored', async () => { + serveNotes({ notes: [] }); + render(); + await screen.findByText('No notes yet'); + const handler = wsHandler(); + await act(async () => { + handler({ tripId: 2, type: 'collab:note:created', note: buildNote({ id: 60, title: 'Other trip note' }) }); + }); + expect(screen.queryByText('Other trip note')).not.toBeInTheDocument(); + }); + + it('FE-W5CNT-031: a rejected rename still renames the rest and re-reads the list', async () => { + const user = userEvent.setup(); + const puts: string[] = []; + serveNotesSequence([ + { notes: [buildNote({ id: 1, title: 'First note', category: 'OldCat' }), buildNote({ id: 2, title: 'Second note', category: 'OldCat' })] }, + { notes: [buildNote({ id: 1, title: 'First note', category: 'OldCat' }), buildNote({ id: 2, title: 'Second note', category: 'NewCat' })] }, + ]); + server.use( + http.put('/api/trips/1/collab/notes/1', () => { + puts.push('1'); + return new HttpResponse(null, { status: 500 }); + }), + http.put('/api/trips/1/collab/notes/2', () => { + puts.push('2'); + return HttpResponse.json({ note: buildNote({ id: 2, title: 'Second note', category: 'NewCat' }) }); + }), + ); + render(); + await screen.findByText('First note'); + + await user.click(screen.getByTitle('Manage Categories')); + await screen.findByText('Manage Categories', { selector: 'h3' }); + const oldCat = screen.getAllByText('OldCat').find(el => el.tagName === 'SPAN' && el.title === 'Click to rename')!; + await user.click(oldCat); + const editInput = screen.getByDisplayValue('OldCat'); + await user.clear(editInput); + await user.type(editInput, 'NewCat'); + await user.keyboard('{Enter}'); + await user.click(screen.getByRole('button', { name: /^Save$/i })); + + // The rejected note must not stop the second one, but the modal stays open — + // half a rename is not a saved rename. + await waitFor(() => expect(puts).toEqual(['1', '2'])); + await waitFor(() => expect(addToast).toHaveBeenCalledWith('Error', 'error', undefined)); + expect(screen.getByText('Manage Categories', { selector: 'h3' })).toBeInTheDocument(); + // Re-read: the note the server refused is still shown under its old category. + const header = screen.getByText('Manage Categories', { selector: 'h3' }).parentElement!; + await user.click(within(header).getByRole('button')); + await waitFor(() => expect(screen.getAllByText('NewCat').length).toBeGreaterThan(0)); + expect(screen.getAllByText('OldCat').length).toBeGreaterThan(0); + }); + it('FE-W5CNT-029: a failing delete reports an error and keeps the note in the list', async () => { const user = userEvent.setup(); serveNotes({ notes: [buildNote({ id: 30, title: 'Stubborn note' })] }); @@ -1819,4 +1874,36 @@ describe('CollabNotes details', () => { await waitFor(() => expect(addToast).toHaveBeenCalledWith('Error', 'error', undefined)); expect(screen.getByText('Stubborn note')).toBeInTheDocument(); }); + + it('FE-W5CNT-032: a rejected category rename keeps the settings modal open and reports once', async () => { + const user = userEvent.setup(); + serveNotes({ + notes: [ + buildNote({ id: 1, title: 'First note', category: 'OldCat' }), + buildNote({ id: 2, title: 'Second note', category: 'OldCat' }), + ], + }); + server.use( + http.put('/api/trips/1/collab/notes/:id', () => new HttpResponse(null, { status: 500 })), + ); + render(); + await screen.findByText('First note'); + await user.click(screen.getByTitle('Manage Categories')); + await screen.findByText('Manage Categories', { selector: 'h3' }); + + const oldCat = screen.getAllByText('OldCat').find(el => el.tagName === 'SPAN' && el.title === 'Click to rename')!; + await user.click(oldCat); + const editInput = screen.getByDisplayValue('OldCat'); + await user.clear(editInput); + await user.type(editInput, 'NewCat'); + await user.keyboard('{Enter}'); + await screen.findByText('NewCat'); + await user.click(screen.getByRole('button', { name: /^Save$/i })); + + await waitFor(() => expect(addToast).toHaveBeenCalledWith('Error', 'error', undefined)); + // Nothing was renamed, so the modal must not act like it was saved… + expect(screen.getByText('Manage Categories', { selector: 'h3' })).toBeInTheDocument(); + // …and both rejected writes are one message, not one each. + expect(addToast).toHaveBeenCalledTimes(1); + }); }); diff --git a/client/src/components/Collab/CollabNotes.tsx b/client/src/components/Collab/CollabNotes.tsx index bf713d00a..db3a7b871 100644 --- a/client/src/components/Collab/CollabNotes.tsx +++ b/client/src/components/Collab/CollabNotes.tsx @@ -89,6 +89,9 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { if (!tripId) return const handler = (msg) => { + // The panel is not remounted on a trip change, so an event still in flight + // from the trip we just left must not land in this list. + if (String(msg?.tripId) !== String(tripId)) return if (msg.type === 'collab:note:created' && msg.note) { setNotes(prev => { if (prev.some(n => n.id === msg.note.id)) return prev @@ -145,12 +148,13 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { } }, [tripId, toast, t]) - const handleUpdateNote = useCallback(async (noteId, data) => { + const handleUpdateNote = useCallback(async (noteId, data, opts: { silent?: boolean } = {}) => { let result try { result = await collabApi.updateNote(tripId, noteId, data) } catch (err) { - toast.error(t('common.error')) + // A batch of writes reports once for the whole run instead of once per note. + if (!opts.silent) toast.error(t('common.error')) throw err } const updated = result?.note || result @@ -161,16 +165,28 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { } }, [tripId, toast, t]) + // A colour or a rename is N single-note writes; if one of them is rejected the + // rest still have to run, and the list has to be re-read so it stops showing a + // change the server never took. Reporting the failure is the caller's job. + const resyncNotes = useCallback(async () => { + try { + const fresh = await collabApi.getNotes(tripId) + setNotes(fresh?.notes || fresh || []) + } catch {} + }, [tripId]) + const saveCategoryColors = useCallback(async (newMap) => { + let failed = 0 // Update notes with changed colors for (const [cat, color] of Object.entries(newMap)) { const notesInCat = notes.filter(n => n.category === cat) if (notesInCat.length > 0 && categoryColors[cat] !== color) { for (const n of notesInCat) { - await handleUpdateNote(n.id, { color }) + try { await handleUpdateNote(n.id, { color }) } catch { failed++ } } } } + if (failed > 0) await resyncNotes() // Save all categories (including empty ones) to localStorage const emptyCats = {} for (const [cat, color] of Object.entries(newMap)) { @@ -179,7 +195,23 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { } } saveEmptyCategories(emptyCats) - }, [categoryColors, notes, handleUpdateNote]) + }, [categoryColors, notes, handleUpdateNote, resyncNotes]) + + const renameCategory = useCallback(async (oldName, newName) => { + // Update all notes with this category in DB + const toUpdate = notes.filter(n => n.category === oldName) + let failed = 0 + for (const n of toUpdate) { + try { await handleUpdateNote(n.id, { category: newName }, { silent: true }) } catch { failed++ } + } + // The rest still had to run, but a partial rename is not a saved rename: the + // settings modal has to stay open on the rejection instead of closing on it. + if (failed > 0) { + await resyncNotes() + toast.error(t('common.error')) + throw new Error(`rename failed for ${failed} of ${toUpdate.length} notes`) + } + }, [notes, handleUpdateNote, resyncNotes, toast, t]) const handleEditSubmit = useCallback(async (data) => { if (!editingNote) return @@ -232,7 +264,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { notes, loading, showNewModal, setShowNewModal, editingNote, setEditingNote, viewingNote, setViewingNote, previewFile, setPreviewFile, showSettings, setShowSettings, activeCategory, setActiveCategory, categoryColors, getCategoryColor, - handleCreateNote, handleUpdateNote, saveCategoryColors, handleEditSubmit, + handleCreateNote, handleUpdateNote, saveCategoryColors, renameCategory, handleEditSubmit, handleDeleteNoteFile, handleDeleteNote, categories, sortedNotes, pendingDeleteNoteId, setPendingDeleteNoteId, } @@ -462,7 +494,7 @@ export default function CollabNotes(props: CollabNotesProps) { loading, tripId, t, categories, categoryColors, getCategoryColor, notes, viewingNote, showNewModal, editingNote, previewFile, showSettings, setShowNewModal, setEditingNote, setPreviewFile, setShowSettings, - handleCreateNote, handleEditSubmit, handleDeleteNoteFile, saveCategoryColors, handleUpdateNote, + handleCreateNote, handleEditSubmit, handleDeleteNoteFile, saveCategoryColors, renameCategory, handleUpdateNote, handleDeleteNote, pendingDeleteNoteId, setPendingDeleteNoteId, } = S @@ -511,13 +543,7 @@ export default function CollabNotes(props: CollabNotesProps) { categories={categories} categoryColors={categoryColors} onSave={saveCategoryColors} - onRenameCategory={async (oldName, newName) => { - // Update all notes with this category in DB - const toUpdate = notes.filter(n => n.category === oldName) - for (const n of toUpdate) { - await handleUpdateNote(n.id, { category: newName }) - } - }} + onRenameCategory={renameCategory} t={t} /> )} diff --git a/client/src/components/Collab/CollabNotesCategorySettingsModal.tsx b/client/src/components/Collab/CollabNotesCategorySettingsModal.tsx index 3ffdb259e..c54a79ff7 100644 --- a/client/src/components/Collab/CollabNotesCategorySettingsModal.tsx +++ b/client/src/components/Collab/CollabNotesCategorySettingsModal.tsx @@ -48,9 +48,14 @@ export function CategorySettingsModal({ onClose, categories, categoryColors, onS } const handleSave = async () => { - // Apply renames to notes in DB - for (const [oldName, newName] of Object.entries(renames)) { - if (oldName !== newName) await onRenameCategory(oldName, newName) + // Apply renames to notes in DB. A rejected rename keeps the modal open — closing + // it would look like the rename went through; the caller has already reported it. + try { + for (const [oldName, newName] of Object.entries(renames)) { + if (oldName !== newName) await onRenameCategory(oldName, newName) + } + } catch { + return } await onSave(localColors) onClose() diff --git a/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx b/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx index 92e440a29..21f59cdb7 100644 Binary files a/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx and b/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx differ diff --git a/client/src/components/Collab/CollabPolls.test.tsx b/client/src/components/Collab/CollabPolls.test.tsx index 8666de8b6..5840333aa 100644 --- a/client/src/components/Collab/CollabPolls.test.tsx +++ b/client/src/components/Collab/CollabPolls.test.tsx @@ -12,7 +12,7 @@ vi.mock('../../api/websocket', () => ({ import { render, screen, waitFor, fireEvent, act } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; -import { http, HttpResponse } from 'msw'; +import { http, HttpResponse, delay } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; import { useAuthStore } from '../../store/authStore'; import { useTripStore } from '../../store/tripStore'; @@ -234,7 +234,7 @@ describe('CollabPolls', () => { // Get the WS listener that was registered const listener = (addListener as ReturnType).mock.calls[0][0]; - listener({ type: 'collab:poll:created', poll: buildPoll({ id: 77, question: 'Live poll?' }) }); + listener({ tripId: 1, type: 'collab:poll:created', poll: buildPoll({ id: 77, question: 'Live poll?' }) }); await screen.findByText('Live poll?'); }); @@ -249,7 +249,7 @@ describe('CollabPolls', () => { await screen.findByText('Best destination?'); const listener = (addListener as ReturnType).mock.calls[0][0]; - listener({ type: 'collab:poll:deleted', pollId: 3 }); + listener({ tripId: 1, type: 'collab:poll:deleted', pollId: 3 }); await waitFor(() => expect(screen.queryByText('Best destination?')).not.toBeInTheDocument(), @@ -658,8 +658,8 @@ describe('CollabPolls details', () => { const handler = wsHandler(); await act(async () => { handler({}); - handler({ type: 'collab:poll:deleted' }); - handler({ type: 'collab:poll:created', poll: { id: 40, question: 'Best destination?' } }); + handler({ tripId: 1, type: 'collab:poll:deleted' }); + handler({ tripId: 1, type: 'collab:poll:created', poll: { id: 40, question: 'Best destination?' } }); }); expect(screen.getAllByText('Best destination?')).toHaveLength(1); }); @@ -671,22 +671,52 @@ describe('CollabPolls details', () => { const handler = wsHandler(); await act(async () => { - handler({ type: 'collab:poll:voted', poll: buildPoll({ id: 41, question: 'Voted live' }) }); + handler({ tripId: 1, type: 'collab:poll:voted', poll: buildPoll({ id: 41, question: 'Voted live' }) }); }); expect(await screen.findByText('Voted live')).toBeInTheDocument(); expect(screen.getByText('Other poll')).toBeInTheDocument(); await act(async () => { - handler({ type: 'collab:poll:closed', poll: { id: 41 } }); + handler({ tripId: 1, type: 'collab:poll:closed', poll: { id: 41 } }); }); await waitFor(() => expect(screen.getAllByText('Closed')).toHaveLength(2)); await act(async () => { - handler({ type: 'collab:poll:deleted', poll: { id: 42 } }); + handler({ tripId: 1, type: 'collab:poll:deleted', poll: { id: 42 } }); }); await waitFor(() => expect(screen.queryByText('Other poll')).not.toBeInTheDocument()); }); + it('FE-W5CPL-027: a WebSocket event for another trip is ignored', async () => { + servePolls({ polls: [] }); + render(); + await screen.findByText(/no polls yet|collab.polls.empty/i); + const handler = wsHandler(); + await act(async () => { + handler({ tripId: 2, type: 'collab:poll:created', poll: buildPoll({ id: 88, question: 'Other trip poll?' }) }); + }); + expect(screen.queryByText('Other trip poll?')).not.toBeInTheDocument(); + }); + + it('FE-W5CPL-028: a slow load for the trip we left does not overwrite the new one', async () => { + server.use( + http.get('/api/trips/1/collab/polls', async () => { + await delay(80); + return HttpResponse.json({ polls: [buildPoll({ id: 1, question: 'Trip one poll?' })] }); + }), + http.get('/api/trips/2/collab/polls', () => + HttpResponse.json({ polls: [buildPoll({ id: 2, question: 'Trip two poll?' })] }), + ), + ); + const { rerender } = render(); + rerender(); + + await screen.findByText('Trip two poll?'); + await act(async () => { await delay(150); }); + expect(screen.queryByText('Trip one poll?')).not.toBeInTheDocument(); + expect(screen.getByText('Trip two poll?')).toBeInTheDocument(); + }); + it('FE-W5CPL-026: a poll with a live deadline starts the countdown ticker', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); try { diff --git a/client/src/components/Collab/CollabPolls.tsx b/client/src/components/Collab/CollabPolls.tsx index 0e7767f03..f2b181a59 100644 --- a/client/src/components/Collab/CollabPolls.tsx +++ b/client/src/components/Collab/CollabPolls.tsx @@ -359,15 +359,25 @@ export default function CollabPolls({ tripId, currentUser }: CollabPollsProps) { const [showForm, setShowForm] = useState(false) useEffect(() => { + let cancelled = false + setLoading(true) collabApi.getPolls(tripId).then(data => { - setPolls(Array.isArray(data) ? data : data.polls || []) - }).catch(() => {}).finally(() => setLoading(false)) + if (!cancelled) setPolls(Array.isArray(data) ? data : data.polls || []) + }).catch(() => { + if (!cancelled) setPolls([]) + }).finally(() => { + if (!cancelled) setLoading(false) + }) + return () => { cancelled = true } }, [tripId]) // WebSocket useEffect(() => { const handler = (msg) => { if (!msg?.type) return + // The panel is not remounted on a trip change, so an event still in flight + // from the trip we just left must not land in this list. + if (String(msg.tripId) !== String(tripId)) return if (msg.type === 'collab:poll:created' && msg.poll) { setPolls(prev => prev.some(p => p.id === msg.poll.id) ? prev : [msg.poll, ...prev]) } @@ -384,7 +394,7 @@ export default function CollabPolls({ tripId, currentUser }: CollabPollsProps) { } addListener(handler) return () => removeListener(handler) - }, []) + }, [tripId]) const handleCreate = useCallback(async (data) => { try { diff --git a/client/src/components/Collab/WhatsNextWidget.test.tsx b/client/src/components/Collab/WhatsNextWidget.test.tsx index 668bdd2b7..4527be802 100644 --- a/client/src/components/Collab/WhatsNextWidget.test.tsx +++ b/client/src/components/Collab/WhatsNextWidget.test.tsx @@ -3,21 +3,25 @@ import { resetAllStores, seedStore } from '../../../tests/helpers/store' import { useTripStore } from '../../store/tripStore' import { useSettingsStore } from '../../store/settingsStore' import WhatsNextWidget from './WhatsNextWidget' -import { afterEach, beforeEach, describe, it, expect } from 'vitest' +import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest' -// Dynamic date helpers -const today = new Date().toISOString().split('T')[0] +// Dynamic date helpers, in local calendar dates like the widget itself uses +function localDate(d: Date): string { + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}` +} + +const today = localDate(new Date()) function getFutureDate(daysAhead: number): string { const d = new Date() d.setDate(d.getDate() + daysAhead) - return d.toISOString().split('T')[0] + return localDate(d) } function getPastDate(daysBack: number): string { const d = new Date() d.setDate(d.getDate() - daysBack) - return d.toISOString().split('T')[0] + return localDate(d) } const tomorrow = getFutureDate(1) @@ -258,6 +262,29 @@ describe('WhatsNextWidget', () => { expect(screen.getByText('Lunch')).toBeInTheDocument() }) + it('FE-COMP-WHATSNEXT-016: keeps remaining events for the local day west of UTC after the UTC date rolls over', () => { + const prevTz = process.env.TZ + process.env.TZ = 'America/New_York' + vi.useFakeTimers({ shouldAdvanceTime: true }) + try { + // 20:00 in New York, already the next calendar day in UTC + vi.setSystemTime(new Date('2026-08-24T00:00:00Z')) + seedStore(useTripStore, { + days: [{ id: 1, trip_id: 1, date: '2026-08-23', title: null, day_number: 0, assignments: [], notes_items: [], notes: null }], + assignments: { + '1': [makeAssignment(80, { name: 'Late Show', place_time: '21:00' })], + }, + }) + render() + expect(screen.getByText('Late Show')).toBeInTheDocument() + expect(screen.getByText(/today/i)).toBeInTheDocument() + } finally { + vi.useRealTimers() + if (prevTz === undefined) delete process.env.TZ + else process.env.TZ = prevTz + } + }) + it('FE-COMP-WHATSNEXT-015: today past-time event is excluded', () => { // If it's not midnight, a past-time event today should not appear const now = new Date() diff --git a/client/src/components/Collab/WhatsNextWidget.tsx b/client/src/components/Collab/WhatsNextWidget.tsx index fad2d3d68..2a3b08cfa 100644 --- a/client/src/components/Collab/WhatsNextWidget.tsx +++ b/client/src/components/Collab/WhatsNextWidget.tsx @@ -5,6 +5,7 @@ import { useSettingsStore } from '../../store/settingsStore' import { useTranslation } from '../../i18n' import { MapPin, Clock, Users, Sparkles } from 'lucide-react' import EmptyState from '../shared/EmptyState' +import { localToday } from '../Planner/today' function formatTime(timeStr, is12h) { if (!timeStr) return '' @@ -19,9 +20,10 @@ function formatTime(timeStr, is12h) { function formatDayLabel(date, t, locale) { const now = new Date() - const nowDate = now.toISOString().split('T')[0] - const tomorrowUtc = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() + 1)) - const tomorrowDate = tomorrowUtc.toISOString().split('T')[0] + // Day dates are plain calendar strings, so "today"/"tomorrow" have to be + // compared against the local calendar day, not the UTC one. + const nowDate = localToday(now) + const tomorrowDate = localToday(new Date(now.getFullYear(), now.getMonth(), now.getDate() + 1)) if (date === nowDate) return t('collab.whatsNext.today') || 'Today' if (date === tomorrowDate) return t('collab.whatsNext.tomorrow') || 'Tomorrow' @@ -47,7 +49,7 @@ export default function WhatsNextWidget({ tripMembers = [] }: WhatsNextWidgetPro const upcoming = useMemo(() => { const now = new Date() - const nowDate = now.toISOString().split('T')[0] + const nowDate = localToday(now) const nowTime = `${String(now.getHours()).padStart(2, '0')}:${String(now.getMinutes()).padStart(2, '0')}` const items = [] diff --git a/client/src/components/Collections/ListEditorModal.test.tsx b/client/src/components/Collections/ListEditorModal.test.tsx index bac602ebd..71e1907b5 100644 --- a/client/src/components/Collections/ListEditorModal.test.tsx +++ b/client/src/components/Collections/ListEditorModal.test.tsx @@ -172,6 +172,21 @@ describe('ListEditorModal', () => { expect(actions.createCollection).not.toHaveBeenCalled() }) + it('FE-COMP-LISTEDITOR-026: a second Enter while the create is in flight does not make a second list', async () => { + let release: (value: { id: number; owner_id: number; name: string }) => void = () => {} + actions.createCollection.mockReturnValueOnce(new Promise(resolve => { release = resolve })) + const { props } = setup() + const nameInput = screen.getByPlaceholderText('e.g. Tokyo 2025') + fireEvent.change(nameInput, { target: { value: 'Lisbon' } }) + fireEvent.keyDown(nameInput, { key: 'Enter' }) + await waitFor(() => expect(actions.createCollection).toHaveBeenCalledTimes(1)) + fireEvent.keyDown(nameInput, { key: 'Enter' }) + release({ id: 77, owner_id: 1, name: 'Lisbon' }) + await waitFor(() => expect(props.onClose).toHaveBeenCalled()) + expect(actions.createCollection).toHaveBeenCalledTimes(1) + expect(props.onCreated).toHaveBeenCalledTimes(1) + }) + it('FE-COMP-LISTEDITOR-011: picking a swatch changes the saved colour', async () => { setup() fireEvent.change(screen.getByPlaceholderText('e.g. Tokyo 2025'), { target: { value: 'Oslo' } }) diff --git a/client/src/components/Collections/ListEditorModal.tsx b/client/src/components/Collections/ListEditorModal.tsx index 9b0609cb1..ec59dbd46 100644 --- a/client/src/components/Collections/ListEditorModal.tsx +++ b/client/src/components/Collections/ListEditorModal.tsx @@ -122,7 +122,9 @@ export default function ListEditorModal({ target, onClose, onCreated, onRequestD const save = async () => { const trimmed = name.trim() - if (!trimmed) return + // Enter fires while the create is still in flight, and `createdId` is only + // set once it comes back — without this the second one creates a second list. + if (!trimmed || saving) return // Normalise + keep only links with a url; drop blank rows. const cleanLinks = links .map(l => ({ label: l.label?.trim() || undefined, url: normalizeLinkUrl(l.url) })) diff --git a/client/src/components/Files/FileManagerImageLightbox.test.tsx b/client/src/components/Files/FileManagerImageLightbox.test.tsx index f0c470860..6721cfc61 100644 --- a/client/src/components/Files/FileManagerImageLightbox.test.tsx +++ b/client/src/components/Files/FileManagerImageLightbox.test.tsx @@ -1,7 +1,7 @@ -// FE-W4LBX-001 to FE-W4LBX-019 +// FE-W4LBX-001 to FE-W4LBX-020 import { describe, it, expect, vi, beforeEach } from 'vitest' import type { TripFile } from '../../types' -import { render, screen, fireEvent, waitFor } from '../../../tests/helpers/render' +import { render, screen, fireEvent, waitFor, act } from '../../../tests/helpers/render' const getAuthUrl = vi.fn(async (url: string, _kind: string) => `${url}?token=abc`) const openFile = vi.fn(async (_url: string, _name: string) => {}) @@ -222,6 +222,23 @@ describe('ImageLightbox', () => { expect(first).not.toHaveBeenCalled() }) + it('FE-W4LBX-020: a mint that lands after the gallery moved on does not paint', async () => { + const pending: Record void> = {} + getAuthUrl.mockImplementation((url: string) => new Promise(resolve => { pending[url] = resolve })) + const { container } = render( {}} />) + await waitFor(() => expect(getAuthUrl).toHaveBeenCalledWith('/f/a.jpg', 'download')) + + fireEvent.keyDown(window, { key: 'ArrowRight' }) + await waitFor(() => expect(getAuthUrl).toHaveBeenCalledWith('/f/b.jpg', 'download')) + // The token for the image we already left resolves last. + await act(async () => { + pending['/f/b.jpg']('/f/b.jpg?token=b') + pending['/f/a.jpg']('/f/a.jpg?token=a') + }) + + expect(container.querySelector('img[alt="b.jpg"]')).toHaveAttribute('src', '/f/b.jpg?token=b') + }) + it('FE-W4LBX-019: the strip mints thumbnail tokens only for thumbs in view', async () => { // Every thumbnail costs its own one-shot token, so an off-screen thumb must // stay quiet. The default observer stub never intersects. diff --git a/client/src/components/Files/FileManagerImageLightbox.tsx b/client/src/components/Files/FileManagerImageLightbox.tsx index 0b2b53911..aa497850b 100644 --- a/client/src/components/Files/FileManagerImageLightbox.tsx +++ b/client/src/components/Files/FileManagerImageLightbox.tsx @@ -27,7 +27,12 @@ export function ImageLightbox({ files, initialIndex, onClose }: ImageLightboxPro setImgSrc('') // Images use a one-shot signed URL; a video must use the plain same-origin // URL (cookie auth) so its many Range requests all authenticate (#823). - if (file && !isVideo(file.mime_type)) getAuthUrl(file.url, 'download').then(setImgSrc) + if (!file || isVideo(file.mime_type)) return + // Arrowing through the gallery leaves several mints in flight; only the one for + // the file still on screen may paint. + let current = true + getAuthUrl(file.url, 'download').then(u => { if (current) setImgSrc(u) }).catch(() => {}) + return () => { current = false } }, [file?.url, file?.mime_type]) const goPrev = () => setIndex(i => Math.max(0, i - 1)) diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx index 9a5f0e37e..cf51a6ba8 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx @@ -586,11 +586,11 @@ describe('EntryEditor', () => { expect(stripImg.getAttribute('src')).toBe('/api/photos/100/original') }) - it('FE-JRN-EDITOR-029: reorders photos locally even when persisting the order fails', async () => { + it('FE-JRN-EDITOR-029: rolls the order back when persisting it fails', async () => { let attempts = 0 server.use(http.patch('/api/journeys/photos/:id', () => { attempts += 1 - return new HttpResponse(null, { status: 500 }) + return HttpResponse.json({ error: 'sort rejected' }, { status: 500 }) })) const user = userEvent.setup() const { container } = mountEditor(buildEntry({ id: 10, photos: [buildPhoto(100), buildPhoto(101)] })) @@ -598,6 +598,27 @@ describe('EntryEditor', () => { await user.click(screen.getByRole('button', { name: 'Make 1st' })) await waitFor(() => expect(attempts).toBe(2)) + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('sort rejected', 'error', undefined)) + const order = Array.from(container.querySelectorAll('.w-20.h-20 img')).map(i => i.getAttribute('src')) + expect(order).toEqual(['/api/photos/100/thumbnail', '/api/photos/101/thumbnail']) + }) + + it('FE-JRN-EDITOR-047: a partly accepted order keeps what the server took', async () => { + const patched: number[] = [] + server.use(http.patch('/api/journeys/photos/:id', ({ params }) => { + const id = Number(params.id) + patched.push(id) + if (id === 100) return HttpResponse.json({ error: 'sort rejected' }, { status: 500 }) + return HttpResponse.json({ success: true }) + })) + const user = userEvent.setup() + const { container } = mountEditor(buildEntry({ id: 10, photos: [buildPhoto(100), buildPhoto(101)] })) + + await user.click(screen.getByRole('button', { name: 'Make 1st' })) + + await waitFor(() => expect(patched).toEqual([101, 100])) + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('sort rejected', 'error', undefined)) + // 101 is first on the server now; snapping the strip back would hide that. const order = Array.from(container.querySelectorAll('.w-20.h-20 img')).map(i => i.getAttribute('src')) expect(order).toEqual(['/api/photos/101/thumbnail', '/api/photos/100/thumbnail']) }) @@ -865,6 +886,32 @@ describe('EntryEditor', () => { expect(onSave.mock.calls[0][0]).toEqual(expect.objectContaining({ entry_time: null })) }) + it('FE-JRN-EDITOR-044: reports a rejected save and keeps the editor open', async () => { + const user = userEvent.setup() + const { onSave, onDone } = mountEditor(buildEntry({ id: 10, title: 'Old' })) + onSave.mockRejectedValueOnce(new Error('server said no')) + + await user.click(screen.getByRole('button', { name: 'Save' })) + + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('server said no', 'error', undefined)) + expect(onDone).not.toHaveBeenCalled() + expect(screen.getByRole('heading', { name: 'Edit Entry' })).toBeInTheDocument() + }) + + it('FE-JRN-EDITOR-046: gives the preview blob URLs back when the editor closes', async () => { + const revoke = vi.spyOn(URL, 'revokeObjectURL') + const { container, unmount } = mountEditor(buildEntry()) + + fireEvent.change(container.querySelector('input[type="file"]') as HTMLInputElement, { + target: { files: [new File(['a'], 'a.jpg', { type: 'image/jpeg' })] }, + }) + await waitFor(() => expect(container.querySelector('img[src="blob:preview"]')).toBeInTheDocument()) + + unmount() + expect(revoke).toHaveBeenCalledWith('blob:preview') + revoke.mockRestore() + }) + it('FE-JRN-EDITOR-043: the camera route is its own input, so the library picker survives', () => { const { container } = mountEditor(buildEntry({ id: 10 })) diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx index 19f57f0ab..0608df282 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx @@ -1,9 +1,9 @@ -import { useEffect, useState, useRef } from 'react' +import { useEffect, useMemo, useState, useRef } from 'react' import { X, Plus, Image, Minus, Check, MapPin, Locate, Camera } from 'lucide-react' import { normalizeImageFiles } from '../../utils/convertHeic' import { type ResilientResult, type UploadProgress } from '../../utils/uploadQueue' import { useTranslation } from '../../i18n' -import { journeyApi, mapsApi, addonsApi } from '../../api/client' +import { journeyApi, mapsApi, addonsApi, memoriesApi } from '../../api/client' import { useToast } from '../shared/Toast' import { getCurrentPositionOnce } from '../../hooks/useGeolocation' import { getApiErrorMessage } from '../../types' @@ -52,6 +52,10 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, const [uploadProgress, setUploadProgress] = useState<{ done: number; total: number } | null>(null) const [photos, setPhotos] = useState<(JourneyPhoto | GalleryPhoto)[]>(entry.photos || []) const [pendingFiles, setPendingFiles] = useState([]) + // Minting the preview URL inline in the JSX would hand out a fresh blob on + // every keystroke in the story field and never give one back. + const pendingUrls = useMemo(() => pendingFiles.map(f => URL.createObjectURL(f)), [pendingFiles]) + useEffect(() => () => { pendingUrls.forEach(u => URL.revokeObjectURL(u)) }, [pendingUrls]) const [pendingLinkIds, setPendingLinkIds] = useState([]) const [showGalleryPick, setShowGalleryPick] = useState(false) const [photoTab, setPhotoTab] = useState<'upload' | 'gallery' | 'external'>('upload') @@ -103,8 +107,7 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, const connected: { id: string; name: string }[] = [] for (const provider of enabled) { try { - const response = await fetch(`/api/integrations/memories/${provider.id}/status`, { credentials: 'include' }) - if (response.ok && (await response.json()).connected) connected.push({ id: provider.id, name: provider.name }) + if ((await memoriesApi.status(provider.id)).connected) connected.push({ id: provider.id, name: provider.name }) } catch {} } setAvailableProviders(connected) @@ -186,6 +189,11 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, setPendingProviderGroups([]) } onDone() + } catch (err) { + // Neither the page callback nor journeyStore toasts, so without this the + // whole entry just fails to save with no sign of it. + toast.error(getApiErrorMessage(err, t('journey.settings.saveFailed'))) + return } finally { setSaving(false) } @@ -436,13 +444,25 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, -
)} diff --git a/client/src/components/Plugins/PluginFrame.test.tsx b/client/src/components/Plugins/PluginFrame.test.tsx index 34b28c50a..151484793 100644 --- a/client/src/components/Plugins/PluginFrame.test.tsx +++ b/client/src/components/Plugins/PluginFrame.test.tsx @@ -1,4 +1,4 @@ -// FE-PLUGINS-FRAME-001 to 046 +// FE-PLUGINS-FRAME-001 to 061 import { render, cleanup, waitFor, fireEvent, screen, act } from '@testing-library/react'; import PluginFrame from './PluginFrame'; import { usePluginStore } from '../../store/pluginStore'; @@ -340,12 +340,12 @@ describe('PluginFrame', () => { function mount(granted: boolean) { grant(granted); - const { container, unmount } = render(); - const iframe = container.querySelector('iframe')!; + const view = render(); + const iframe = view.container.querySelector('iframe')!; const posted: Array> = []; (iframe.contentWindow as unknown as { postMessage: (m: unknown) => void }).postMessage = (m: unknown) => posted.push(m as Record); - return { iframe, posted, unmount }; + return { ...view, iframe, posted }; } it('FE-PLUGINS-FRAME-015: refuses an ungranted plugin without touching the browser API', () => { @@ -381,6 +381,33 @@ describe('PluginFrame', () => { expect(geo.clearWatch).toHaveBeenCalledWith(7); }); + it('FE-PLUGINS-FRAME-060: a host settings change re-bridges without dropping the watch', () => { + let tick: ((p: unknown) => void) | null = null; + geo.watchPosition.mockImplementation((ok: (p: unknown) => void) => { tick = ok; return 7; }); + const { iframe, posted, rerender } = mount(true); + act(() => { fromFrame(iframe, { type: 'trek:geolocation', requestId: 'g20', action: 'watch' }); }); + + // Any settings write re-runs the bridge effect; the watch must survive it. + host.settings = { ...DEFAULT_SETTINGS, distance_unit: 'imperial' }; + act(() => { rerender(); }); + + expect(geo.clearWatch).not.toHaveBeenCalled(); + const before = posted.length; + act(() => tick!({ coords: { latitude: 5, longitude: 6, accuracy: 5, heading: null, speed: null }, timestamp: 3 })); + expect(posted.length).toBe(before + 1); + }); + + it('FE-PLUGINS-FRAME-061: a frame that navigates itself has its watch released', () => { + const { iframe } = mount(true); + act(() => { fireEvent.load(iframe); }); + act(() => { fromFrame(iframe, { type: 'trek:geolocation', requestId: 'g21', action: 'watch' }); }); + + // Second load = the frame navigated away, so nothing receives the stream any more. + act(() => { fireEvent.load(iframe); }); + + expect(geo.clearWatch).toHaveBeenCalledWith(7); + }); + it('FE-PLUGINS-FRAME-018: clear stops the watch on request', () => { const { iframe, posted } = mount(true); fromFrame(iframe, { type: 'trek:geolocation', requestId: 'g4', action: 'watch' }); diff --git a/client/src/components/Plugins/PluginFrame.tsx b/client/src/components/Plugins/PluginFrame.tsx index f773d3731..e65af4390 100644 --- a/client/src/components/Plugins/PluginFrame.tsx +++ b/client/src/components/Plugins/PluginFrame.tsx @@ -650,8 +650,6 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d window.removeEventListener('message', onMessage) themeObserver.disconnect() if (wsForward) removeListener(wsForward) - // The frame is going away (or re-bridging) — never leave a live GPS watch behind. - if (geoWatchRef.current != null) { navigator.geolocation.clearWatch(geoWatchRef.current); geoWatchRef.current = null } } }, [pluginId, tripId, fill, navigate, postFrame, buildContext, userId]) @@ -664,6 +662,11 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d setHeight(null) confirmReqRef.current = null setConfirmReq(null) + // Only a real teardown may kill the watch: the bridge above re-runs on any + // settings, locale or breakpoint change, and nothing would re-arm it. + return () => { + if (geoWatchRef.current != null) { navigator.geolocation.clearWatch(geoWatchRef.current); geoWatchRef.current = null } + } }, [pluginId]) const answerConfirm = (confirmed: boolean) => { @@ -689,7 +692,14 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d // Deliver the context as soon as the document is parsed (the plugin sets up its // message listener during parse), closing the trek:ready race so the theme is // right on first paint. A 2nd load is a self-navigation — don't bridge to it. - onLoad={() => { setLoadFailed(false); loadsRef.current += 1; if (loadsRef.current === 1) postFrame(buildContext()) }} + onLoad={() => { + setLoadFailed(false) + loadsRef.current += 1 + if (loadsRef.current === 1) postFrame(buildContext()) + // A self-navigated frame is no longer bridged, so its watch is only + // burning battery for positions nobody receives. + else if (geoWatchRef.current != null) { navigator.geolocation.clearWatch(geoWatchRef.current); geoWatchRef.current = null } + }} onError={() => setLoadFailed(true)} sandbox="allow-scripts allow-forms" referrerPolicy="no-referrer" diff --git a/client/src/components/Settings/AppearanceSettingsTab.test.tsx b/client/src/components/Settings/AppearanceSettingsTab.test.tsx index b6b5cfde1..61ddaa6de 100644 --- a/client/src/components/Settings/AppearanceSettingsTab.test.tsx +++ b/client/src/components/Settings/AppearanceSettingsTab.test.tsx @@ -298,4 +298,35 @@ describe('AppearanceSettingsTab – dashboard widgets', () => { PERSIST, ); }); + + // The scheme is already on the DOM by then, so dropping the write would revert the + // choice on the next load without ever telling the user. + it('FE-COMP-APPEARANCE-023: leaving the tab before the debounce fires flushes the pending save once', async () => { + const user = userEvent.setup(); + const updateSetting = seedAppearance(); + const { unmount } = render(); + + await user.click(screen.getByText('Indigo')); + unmount(); + + const calls = updateSetting.mock.calls.filter(c => c[0] === 'appearance'); + expect(calls).toHaveLength(1); + expect(calls[0][1]).toMatchObject({ schemeId: 'indigo' }); + + // The timer is cancelled, so nothing lands a second time. + await new Promise(resolve => setTimeout(resolve, 500)); + expect(updateSetting.mock.calls.filter(c => c[0] === 'appearance')).toHaveLength(1); + }); + + it('FE-COMP-APPEARANCE-024: a debounce that already fired is not written again on unmount', async () => { + const user = userEvent.setup(); + const updateSetting = seedAppearance(); + const { unmount } = render(); + + await user.click(screen.getByText('Teal')); + await waitFor(() => expect(updateSetting.mock.calls.filter(c => c[0] === 'appearance')).toHaveLength(1), PERSIST); + + unmount(); + expect(updateSetting.mock.calls.filter(c => c[0] === 'appearance')).toHaveLength(1); + }); }); diff --git a/client/src/components/Settings/AppearanceSettingsTab.tsx b/client/src/components/Settings/AppearanceSettingsTab.tsx index 32608aecb..46893a1e8 100644 --- a/client/src/components/Settings/AppearanceSettingsTab.tsx +++ b/client/src/components/Settings/AppearanceSettingsTab.tsx @@ -81,6 +81,9 @@ export default function AppearanceSettingsTab(): React.ReactElement { const [cfg, setCfg] = useState(() => normalizeAppearance(settings.appearance)) const persistTimer = useRef(undefined) + // What the pending timer would have written, so leaving the tab inside the + // debounce window still saves instead of silently dropping the change. + const pendingWrite = useRef(null) // Re-sync when settings change elsewhere (e.g. server reconcile / another tab). useEffect(() => { @@ -89,8 +92,11 @@ export default function AppearanceSettingsTab(): React.ReactElement { // Flush any pending persist on unmount. useEffect(() => () => { - if (persistTimer.current) window.clearTimeout(persistTimer.current) - }, []) + if (!persistTimer.current) return + window.clearTimeout(persistTimer.current) + // The component is gone, so a failure has nowhere to be shown. + if (pendingWrite.current) updateSetting('appearance', pendingWrite.current).catch(() => {}) + }, [updateSetting]) const isDark = settings.dark_mode === true || @@ -103,7 +109,9 @@ export default function AppearanceSettingsTab(): React.ReactElement { setCfg(next) applyAppearance({ darkMode: settings.dark_mode, appearance: next, isSharedPage: false }) if (persistTimer.current) window.clearTimeout(persistTimer.current) + pendingWrite.current = next persistTimer.current = window.setTimeout(() => { + pendingWrite.current = null updateSetting('appearance', next).catch((e: unknown) => toast.error(e instanceof Error ? e.message : t('common.error')) ) diff --git a/client/src/components/Settings/NotificationsTab.test.tsx b/client/src/components/Settings/NotificationsTab.test.tsx index d5ceb72a6..b234b318a 100644 --- a/client/src/components/Settings/NotificationsTab.test.tsx +++ b/client/src/components/Settings/NotificationsTab.test.tsx @@ -755,6 +755,59 @@ describe('NotificationsTab', () => { expect(screen.getByText('Connection refused')).toBeInTheDocument(); }); }); + + it('FE-COMP-NOTIFICATIONS-030: a failing toggle surfaces the generic error', async () => { + const user = userEvent.setup(); + server.use( + http.put('/api/notifications/preferences', () => HttpResponse.json({ error: 'nope' }, { status: 500 })), + ); + render(<>); + await waitFor(() => expect(screen.queryByText('Loading...')).not.toBeInTheDocument()); + + const toggles = await screen.findAllByRole('button'); + await user.click(toggles[0]); + + expect(await screen.findByText('Error')).toBeInTheDocument(); + }); + + it('FE-COMP-NOTIFICATIONS-031: a failing toggle leaves a later toggle alone', async () => { + const user = userEvent.setup(); + const bodies: Record>[] = []; + let rejectEmail!: () => void; + server.use( + http.put('/api/notifications/preferences', async ({ request }) => { + const body = (await request.json()) as Record>; + bodies.push(body); + if (body.trip_invite?.email !== undefined) { + return new Promise(resolve => { + rejectEmail = () => resolve(HttpResponse.json({ error: 'nope' }, { status: 500 }) as unknown as Response); + }); + } + return HttpResponse.json({ success: true }); + }), + ); + render(<>); + await waitFor(() => expect(screen.queryByText('Loading...')).not.toBeInTheDocument()); + + // Email is off in this matrix, in-app is on. + const [email, inapp] = await screen.findAllByRole('button'); + await user.click(email); + await waitFor(() => expect(screen.getAllByRole('button')[0]).toHaveAttribute('aria-pressed', 'true')); + + // Flipped while the email write is still hanging. + await user.click(inapp); + await waitFor(() => expect(bodies).toHaveLength(2)); + await waitFor(() => expect(screen.getAllByRole('button')[1]).toHaveAttribute('aria-pressed', 'false')); + // The in-app write must not carry the email cell, or the server would keep the + // value the failing write is about to take back. + expect(bodies[1].trip_invite.email).toBeUndefined(); + + rejectEmail(); + + await waitFor(() => expect(screen.getAllByRole('button')[0]).toHaveAttribute('aria-pressed', 'false')); + // The in-app toggle the user made meanwhile is not undone. + expect(screen.getAllByRole('button')[1]).toHaveAttribute('aria-pressed', 'false'); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/client/src/components/Settings/NotificationsTab.tsx b/client/src/components/Settings/NotificationsTab.tsx index 56d34ca39..f59e60895 100644 --- a/client/src/components/Settings/NotificationsTab.tsx +++ b/client/src/components/Settings/NotificationsTab.tsx @@ -116,16 +116,24 @@ export default function NotificationsTab(): React.ReactElement { const toggle = async (eventType: string, channel: string) => { if (!matrix) return const current = matrix.preferences[eventType]?.[channel] ?? true - const updated = { - ...matrix.preferences, - [eventType]: { ...matrix.preferences[eventType], [channel]: !current }, - } - setMatrix(m => m ? { ...m, preferences: updated } : m) + setMatrix(m => m ? { + ...m, + preferences: { ...m.preferences, [eventType]: { ...m.preferences[eventType], [channel]: !current } }, + } : m) setSaving(true) try { - await notificationsApi.updatePreferences(updated) + // Only the toggled cell goes out. The server merges what it gets, and sending the + // whole matrix would carry this render's value for every other cell — a second + // toggle made while this request is in flight would be overwritten by it. + await notificationsApi.updatePreferences({ [eventType]: { [channel]: !current } }) } catch { - setMatrix(m => m ? { ...m, preferences: matrix.preferences } : m) + // Only this cell rolls back — restoring the whole snapshot would also undo a + // toggle the user made while this request was in flight. + setMatrix(m => m ? { + ...m, + preferences: { ...m.preferences, [eventType]: { ...m.preferences[eventType], [channel]: current } }, + } : m) + toast.error(t('common.error')) } finally { setSaving(false) } diff --git a/client/src/components/Studio/SaveIndicator.tsx b/client/src/components/Studio/SaveIndicator.tsx index 746880022..e0a9fe8a7 100644 --- a/client/src/components/Studio/SaveIndicator.tsx +++ b/client/src/components/Studio/SaveIndicator.tsx @@ -1,4 +1,4 @@ -import { AlertTriangle, Check, CloudOff, RefreshCw } from 'lucide-react' +import { AlertTriangle, Check, CloudOff, Eye, RefreshCw } from 'lucide-react' import type { BookRecord } from '@trek/shared' import type { SaveState } from './useBookStore' @@ -43,6 +43,20 @@ export function SaveIndicator({ ) } + if (state.status === 'readonly') { + /* + * Not an error and not retryable: this person was invited to read the + * journey, not to write it. Said once, plainly, and left standing — the + * alternative is an editor that looks like it is saving and is not. + */ + return ( +
+ + {t('journey.studio.saveReadOnly')} +
+ ) + } + if (state.status === 'error') { return ( {sel.length === 1 && ( <> - - @@ -405,13 +419,13 @@ export function StudioCanvas({ */} @@ -424,12 +438,16 @@ export function StudioCanvas({ elements: sp.elements.map(e => (selection.includes(e.id) ? { ...e, locked: !locked } : e)), })), }))} - title={locked ? 'Unlock' : 'Lock'} + title={t(locked ? 'journey.studio.unlock' : 'journey.studio.lock')} > {locked ? : } - diff --git a/client/src/components/Studio/StudioTravelPanel.tsx b/client/src/components/Studio/StudioTravelPanel.tsx index 29c0594cb..e3a4c61c2 100644 --- a/client/src/components/Studio/StudioTravelPanel.tsx +++ b/client/src/components/Studio/StudioTravelPanel.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react' import type { BookElement, BookMetric, BookPageSetup, JourneyStats } from '@trek/shared' import { BOOK_METRICS } from '@trek/shared' import { useStudioStore } from '../../store/studioStore' @@ -95,6 +96,19 @@ export function StudioTravelPanel({ const spread = doc?.spreads[active] const single = !!spread && spread.role !== 'inner' + /* + * The road lookups still running when the panel goes away. + * + * fetchRoads walks the legs at a deliberate pace, so a twenty-stop map keeps + * asking a public router for minutes after Studio is closed, and the answer + * lands in whatever document the store holds by then. + */ + const roadJobs = useRef(new Set()) + useEffect(() => () => { + roadJobs.current.forEach(c => c.abort()) + roadJobs.current.clear() + }, []) + const centre = (w: number, h: number) => { const W = single ? page.pageWidth : page.pageWidth * 2 return { x: (W - w) / 2, y: (page.pageHeight - h) / 2, w, h } @@ -304,13 +318,17 @@ export function StudioTravelPanel({ * free, and a failure leaves the leg exactly as it was. */ if (placed.kind === 'map' && placed.points.length > 1) { - void fetchRoads(placed.points.map(pt => ({ lat: pt.lat, lng: pt.lng }))) + const job = new AbortController() + roadJobs.current.add(job) + void fetchRoads(placed.points.map(pt => ({ lat: pt.lat, lng: pt.lng })), { signal: job.signal }) .then(roads => { + if (job.signal.aborted) return if (roads.some(r => r && r.length > 1)) { updateElement(active, placed.id, { roads } as Partial) } }) .catch(() => { /* No roads is a state the map already draws. */ }) + .finally(() => { roadJobs.current.delete(job) }) } } diff --git a/client/src/components/Studio/applyTemplate.ts b/client/src/components/Studio/applyTemplate.ts index 0472dba46..307079514 100644 --- a/client/src/components/Studio/applyTemplate.ts +++ b/client/src/components/Studio/applyTemplate.ts @@ -221,7 +221,7 @@ function fillBadge(el: BookElement & { kind: 'badge' }, entry: TemplateEntry, ct } } if (el.variant === 'distance' && ctx.stats?.distance) { - el.text = `${Math.round(ctx.stats.distance / 1000).toLocaleString('en-US')} km` + el.text = `${Math.round(ctx.stats.distance / 1000).toLocaleString(ctx.locale)} km` } } diff --git a/client/src/components/Studio/autoLayout.ts b/client/src/components/Studio/autoLayout.ts index 5d1889c13..15c45fc84 100644 --- a/client/src/components/Studio/autoLayout.ts +++ b/client/src/components/Studio/autoLayout.ts @@ -1369,7 +1369,7 @@ function summarySpread(input: AutoInput): BookSpread | null { els.push(badgeEl( { x: rx + 8 + lineW * 0.5 - 17, y: y - 11, w: 34, h: 8 }, 'distance', - { text: formatKm(stats.furthest) }, + { text: formatKm(stats.furthest, input.locale) }, { style: 'chip', textScale: 0.7 }, )) els.push(text({ x: rx + 4, y: y + 4, w: lineW * 0.5, h: 5 }, (home.label || '').toUpperCase(), { @@ -1396,9 +1396,9 @@ function furthestPoint(stats: JourneyStats) { return best } -/** Metres as the books print them: "1,189 km". */ -function formatKm(metres: number): string { - return `${Math.round(metres / 1000).toLocaleString('en-US')} km` +/** Metres as the books print them: "1,189 km", in the book's own language. */ +function formatKm(metres: number, locale: string): string { + return `${Math.round(metres / 1000).toLocaleString(locale)} km` } function countriesSpread(input: AutoInput, names: string[]): BookSpread | null { @@ -1477,7 +1477,7 @@ function backSpread(input: AutoInput): BookSpread { if (input.stats) { const closing: BookMetric[] = ([ ['days', input.stats.days], - ['places', input.stats.places || input.stats.steps], + ['places', input.stats.places], ['photos', input.stats.photos], ] as [BookMetric, number][]) .filter(([, v]) => v > 0) diff --git a/client/src/components/Studio/useBookStore.ts b/client/src/components/Studio/useBookStore.ts index 6358d7e66..7fc627946 100644 --- a/client/src/components/Studio/useBookStore.ts +++ b/client/src/components/Studio/useBookStore.ts @@ -55,6 +55,8 @@ export type SaveState = | { status: 'saving' } | { status: 'saved'; at: number } | { status: 'conflict'; current: BookRecord } + /** The journey is open to this user, but they may not write it (role 'viewer'). */ + | { status: 'readonly' } | { status: 'error' } export function useBookStore( @@ -73,6 +75,9 @@ export function useBookStore( const pending = useRef<{ document: BookDocument; title: string } | null>(null) const inFlight = useRef(false) + /** The title the last queued save carried, so a re-queue does not lose it. */ + const lastTitle = useRef('') + /** * The document the server and this client agree on, by identity. * @@ -173,6 +178,15 @@ export function useBookStore( if (res?.status === 409 && res.data?.current) { blocked.current = true setState({ status: 'conflict', current: res.data.current }) + } else if (res?.status === 403) { + /* + * A viewer can open Studio - the book is part of the journey they were + * invited to - but the server will not take their writes. Stop trying + * on the first refusal and say so: retrying forever would let someone + * lay out an entire book before finding out none of it was ever saved. + */ + blocked.current = true + setState({ status: 'readonly' }) } else { setState({ status: 'error' }) } @@ -193,6 +207,7 @@ export function useBookStore( */ const queueSave = useCallback((document: BookDocument, title: string) => { latest.current = document + lastTitle.current = title // The document we already agree on is not an edit — see `synced`. if (document === synced.current) return pending.current = { document, title } @@ -207,7 +222,10 @@ export function useBookStore( /** Write immediately — for closing the editor, or a save the user asked for. */ const saveNow = useCallback((document?: BookDocument, title?: string) => { - if (document) pending.current = { document, title: title ?? '' } + if (document) { + if (title != null) lastTitle.current = title + pending.current = { document, title: title ?? lastTitle.current } + } if (timer.current != null) window.clearTimeout(timer.current) return write() }, [write]) @@ -236,8 +254,20 @@ export function useBookStore( const keepMine = useCallback((current: BookRecord) => { version.current = current.version blocked.current = false + /* + * The refused document has to be queued again here, because write() cleared + * the queue before it made the request that came back 409. Without this, + * choosing "keep mine" left the work sitting in the tab: the autosave only + * fires on the next change, and closing the editor writes whatever is + * queued, which is nothing. + */ + if (latest.current && latest.current !== synced.current) { + pending.current = { document: latest.current, title: lastTitle.current } + void write() + return + } setState({ status: 'idle' }) - }, []) + }, [write]) /** * Someone else saved. diff --git a/client/src/components/SystemNotices/SystemNoticeModal.test.tsx b/client/src/components/SystemNotices/SystemNoticeModal.test.tsx index 5e9f1d766..63822b13d 100644 --- a/client/src/components/SystemNotices/SystemNoticeModal.test.tsx +++ b/client/src/components/SystemNotices/SystemNoticeModal.test.tsx @@ -298,6 +298,18 @@ describe('ModalRenderer', () => { expect(screen.getByText('Hello Alice, welcome to TREK')).toBeTruthy(); }); + it('FE-SN-MODAL-057: a body param is inserted literally, not as a replacement pattern', async () => { + const notice = makeNotice({ + bodyKey: 'Version {version} is here', + bodyParams: { version: '4.0.0 ($&)' }, + }); + render(); + + await flushGraceDelay(); + + expect(screen.getByText('Version 4.0.0 ($&) is here')).toBeTruthy(); + }); + it('FE-SN-MODAL-008: empty notices renders nothing', () => { const { container } = render(); expect(container.firstChild).toBeNull(); diff --git a/client/src/components/SystemNotices/SystemNoticeModal.tsx b/client/src/components/SystemNotices/SystemNoticeModal.tsx index e2fce3284..74bd3d98b 100644 --- a/client/src/components/SystemNotices/SystemNoticeModal.tsx +++ b/client/src/components/SystemNotices/SystemNoticeModal.tsx @@ -617,13 +617,17 @@ function useSystemNoticeModal(notices: SystemNoticeDTO[]) { type NoticeState = ReturnType; +const escapeRe = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // Build the NoticeContent props for a given notice + pager slot index. function makeContentProps(S: NoticeState, n: SystemNoticeDTO, slotIdx: number): ContentProps { const { t, isDark, canPage, notices, handleDismissAll, handleCTA, handleSecondaryCTA, handlePrev, handleNext, handleGoto } = S; const rawBody = t(n.bodyKey); + // The key is escaped into the pattern and the value goes in through a function, so a + // value carrying $&, $1 or $` is inserted as typed instead of being expanded. const body = n.bodyParams ? Object.entries(n.bodyParams).reduce( - (s, [k, v]) => s.replace(new RegExp(`\\{${k}\\}`, 'g'), v), + (s, [k, v]) => s.replace(new RegExp(`\\{${escapeRe(k)}\\}`, 'g'), () => v), rawBody ) : rawBody; diff --git a/client/src/components/Todo/TodoListPanel.test.tsx b/client/src/components/Todo/TodoListPanel.test.tsx index 3901be5a9..592d56d6e 100644 --- a/client/src/components/Todo/TodoListPanel.test.tsx +++ b/client/src/components/Todo/TodoListPanel.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-TODO-001 to FE-COMP-TODO-052 +// FE-COMP-TODO-001 to FE-COMP-TODO-079 import { render, screen, waitFor, fireEvent, within } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; @@ -24,6 +24,7 @@ beforeEach(() => { }); afterEach(() => { + vi.useRealTimers(); Object.defineProperty(window, 'innerWidth', { value: 0, writable: true, configurable: true }); }); @@ -202,6 +203,25 @@ describe('TodoListPanel', () => { expect(screen.queryByText('Future Task')).not.toBeInTheDocument(); }); + it('FE-COMP-TODO-079: Overdue uses the local calendar day, not the UTC one', () => { + // Freeze on an instant whose UTC date differs from the local one, so a UTC-derived + // "today" would move the cut-off a day in whichever direction this runner sits. + const offset = new Date(2026, 4, 15, 12).getTimezoneOffset(); + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date(2026, 4, 15, offset > 0 ? 23 : 0, 30)); + const items = [ + buildTodoItem({ id: 1, name: 'Due Today', checked: 0, due_date: '2026-05-15' }), + buildTodoItem({ id: 2, name: 'Due Yesterday', checked: 0, due_date: '2026-05-14' }), + ]; + render(); + const overdueBtn = screen.getAllByRole('button').find( + b => b.textContent?.includes('Overdue') || b.getAttribute('title') === 'Overdue' + ); + fireEvent.click(overdueBtn!); + expect(screen.getByText('Due Yesterday')).toBeInTheDocument(); + expect(screen.queryByText('Due Today')).not.toBeInTheDocument(); + }); + it('FE-COMP-TODO-017: My Tasks filter shows only items assigned to current user', async () => { // Use default current_user_id: 1 from beforeEach; assign one item to user 1 const items = [ diff --git a/client/src/components/Todo/useTodoList.ts b/client/src/components/Todo/useTodoList.ts index afda45b4b..79bd22739 100644 --- a/client/src/components/Todo/useTodoList.ts +++ b/client/src/components/Todo/useTodoList.ts @@ -6,6 +6,7 @@ import { useTranslation } from '../../i18n' import apiClient from '../../api/client' import { formatDate as fmtDate } from '../../utils/formatters' import type { TodoItem } from '../../types' +import { localToday } from '../Planner/today' import type { FilterType, Member } from './todoListModel' /** @@ -65,7 +66,9 @@ export function useTodoList(tripId: number, items: TodoItem[], addItemSignal: nu return Array.from(cats).sort() }, [items]) - const today = new Date().toISOString().split('T')[0] + // due_date is a bare calendar date, so "today" has to be the user's calendar + // day: toISOString() hands over the UTC one and shifts the overdue cut-off. + const today = localToday() const filtered = useMemo(() => { let result: TodoItem[] diff --git a/client/src/components/Trips/TripFormModal.test.tsx b/client/src/components/Trips/TripFormModal.test.tsx index ce298903a..75982b6cd 100644 --- a/client/src/components/Trips/TripFormModal.test.tsx +++ b/client/src/components/Trips/TripFormModal.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-TRIPFORM-001 to FE-COMP-TRIPFORM-082 +// FE-COMP-TRIPFORM-001 to FE-COMP-TRIPFORM-084 import type { Mock } from 'vitest'; import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; @@ -1214,4 +1214,37 @@ describe('TripFormModal', () => { await waitFor(() => expect(staleResolved).toBe(true)); expect(screen.queryByText('Stale failure')).not.toBeInTheDocument(); }); + + it('FE-COMP-TRIPFORM-083: a closed modal fetches nothing until it is opened', async () => { + // The trip planner keeps the modal mounted behind the page. + const seen: string[] = []; + server.use( + http.get('/api/auth/users', () => { seen.push('users'); return HttpResponse.json({ users: [] }); }), + http.get('/api/trips/:id/members', () => { seen.push('members'); return HttpResponse.json({ members: [] }); }), + ); + const trip = buildTrip({ id: 5 }); + const { rerender } = render(); + + await new Promise(resolve => setTimeout(resolve, 0)); + expect(seen).toEqual([]); + + rerender(); + await waitFor(() => expect(seen).toContain('users')); + expect(seen).toContain('members'); + }); + + it('FE-COMP-TRIPFORM-084: staging another cover revokes the preview it replaces', async () => { + const revokeObjectURL = vi.fn(); + const original = URL.revokeObjectURL; + Object.defineProperty(URL, 'revokeObjectURL', { writable: true, configurable: true, value: revokeObjectURL }); + createObjectURL.mockReturnValueOnce('blob:first').mockReturnValueOnce('blob:second'); + render(); + + fireEvent.change(fileInput(), { target: { files: [pngFile()] } }); + await waitFor(() => expect(createObjectURL).toHaveBeenCalledTimes(1)); + fireEvent.change(fileInput(), { target: { files: [pngFile()] } }); + + await waitFor(() => expect(revokeObjectURL).toHaveBeenCalledWith('blob:first')); + Object.defineProperty(URL, 'revokeObjectURL', { writable: true, configurable: true, value: original }); + }); }); diff --git a/client/src/components/Trips/TripFormModal.tsx b/client/src/components/Trips/TripFormModal.tsx index d3068bdd9..e253fda38 100644 --- a/client/src/components/Trips/TripFormModal.tsx +++ b/client/src/components/Trips/TripFormModal.tsx @@ -40,6 +40,8 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp const isEditing = !!trip const fileRef = useRef(null) const coverSearchSeq = useRef(0) + // The staged cover lives on as an object URL until it is replaced or the modal goes. + const previewUrlRef = useRef(null) const toast = useToast() const { t } = useTranslation() const currentUser = useAuthStore(s => s.user) @@ -104,6 +106,10 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp setCoverPreview(null) setCoverSearchQuery('') } + if (previewUrlRef.current) { + URL.revokeObjectURL(previewUrlRef.current) + previewUrlRef.current = null + } setPendingCoverFile(null) setPendingUnsplashUrl(null) setCoverSearchResults([]) @@ -112,16 +118,17 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp setPendingDateShift(null) setDateShiftMode('keep_bookings') setError('') + setExistingMembers([]) + // The planner keeps this modal mounted while it is closed, so nothing may be + // fetched until it is actually open. if (isOpen) { authApi.getAppConfig().then((c: { trip_reminders_enabled?: boolean }) => { if (c?.trip_reminders_enabled !== undefined) setTripRemindersEnabled(c.trip_reminders_enabled) }).catch(() => {}) - } - authApi.listUsers().then(d => setAllUsers(d.users || [])).catch(() => {}) - if (trip) { - tripsApi.getMembers(trip.id).then(d => setExistingMembers(d.members || [])).catch(() => {}) - } else { - setExistingMembers([]) + authApi.listUsers().then(d => setAllUsers(d.users || [])).catch(() => {}) + if (trip) { + tripsApi.getMembers(trip.id).then(d => setExistingMembers(d.members || [])).catch(() => {}) + } } }, [trip, isOpen]) @@ -131,6 +138,12 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp } }, [tripRemindersEnabled]) + // A staged cover that never got uploaded would otherwise pin the full image for + // as long as the tab lives. + useEffect(() => () => { + if (previewUrlRef.current) URL.revokeObjectURL(previewUrlRef.current) + }, []) + const handleSubmit = async (e) => { e.preventDefault() setError('') @@ -219,7 +232,9 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp } else { // New trip: stage for upload after creation setPendingCoverFile(normalized) - setCoverPreview(URL.createObjectURL(normalized)) + if (previewUrlRef.current) URL.revokeObjectURL(previewUrlRef.current) + previewUrlRef.current = URL.createObjectURL(normalized) + setCoverPreview(previewUrlRef.current) } } diff --git a/client/src/components/Trips/TripMembersModal.test.tsx b/client/src/components/Trips/TripMembersModal.test.tsx index 47db5a5c9..cff2f05a6 100644 --- a/client/src/components/Trips/TripMembersModal.test.tsx +++ b/client/src/components/Trips/TripMembersModal.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-MEMBERS-001 to FE-COMP-MEMBERS-054 +// FE-COMP-MEMBERS-001 to FE-COMP-MEMBERS-056 import type { Mock } from 'vitest'; import { act, render, screen, fireEvent, waitFor } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; @@ -47,9 +47,11 @@ function asShareOwner(): void { server.use(http.get('/api/trips/1/invite-link', () => HttpResponse.json({ token: null }))); } +/** A browser on https, where the async clipboard API is the path taken. */ function mockClipboard(): Mock<(text: string) => Promise> { const writeText = vi.fn(async () => {}); - Object.defineProperty(navigator, 'clipboard', { value: { writeText }, configurable: true }); + Object.defineProperty(navigator, 'clipboard', { value: { writeText }, configurable: true, writable: true }); + Object.defineProperty(window, 'isSecureContext', { value: true, configurable: true, writable: true }); return writeText; } @@ -80,6 +82,7 @@ afterEach(() => { delete window.__addToast; vi.useRealTimers(); vi.restoreAllMocks(); + Object.defineProperty(window, 'isSecureContext', { value: false, configurable: true, writable: true }); }); describe('TripMembersModal', () => { @@ -272,11 +275,7 @@ describe('TripMembersModal', () => { seedStore(usePermissionsStore, { permissions: { share_manage: 'trip_owner' } }); seedStore(useTripStore, { trip: buildTrip({ id: 1, user_id: ownerUser.id }) }); - const writeText = vi.fn().mockResolvedValue(undefined); - Object.defineProperty(navigator, 'clipboard', { - value: { writeText }, - configurable: true, - }); + const writeText = mockClipboard(); server.use( http.get('/api/trips/1/share-link', () => @@ -616,9 +615,12 @@ describe('TripMembersModal', () => { vi.useFakeTimers(); fireEvent.click(copyBtn); + // The copy is awaited now, so the badge lands a microtask after the click. + await act(async () => {}); expect(screen.getByText('Copied')).toBeInTheDocument(); // A second copy replaces the pending reset rather than stacking timers. fireEvent.click(screen.getByText('Copied').closest('button')!); + await act(async () => {}); act(() => { vi.advanceTimersByTime(2000); }); expect(screen.getByText('Copy')).toBeInTheDocument(); @@ -980,6 +982,42 @@ describe('TripMembersModal', () => { expect(screen.queryByTitle('Remove access')).not.toBeInTheDocument(); }); + // A self-hosted install served over plain HTTP has no navigator.clipboard, and + // the unguarded call threw before the button ever showed feedback. + it('FE-COMP-MEMBERS-055: the share link copies through execCommand without a clipboard API', async () => { + const user = userEvent.setup(); + asShareOwner(); + Object.defineProperty(navigator, 'clipboard', { value: undefined, configurable: true, writable: true }); + Object.defineProperty(window, 'isSecureContext', { value: false, configurable: true, writable: true }); + const execCommand = vi.fn(() => true); + Object.defineProperty(document, 'execCommand', { value: execCommand, configurable: true, writable: true }); + server.use(http.get('/api/trips/1/share-link', () => HttpResponse.json({ token: 'tok55' }))); + render(); + + await user.click((await screen.findByText('Copy')).closest('button')!); + + expect(execCommand).toHaveBeenCalledWith('copy'); + await screen.findByText('Copied'); + expect(document.querySelector('textarea')).toBeNull(); + }); + + it('FE-COMP-MEMBERS-056: the invite link copies through execCommand without a clipboard API', async () => { + const user = userEvent.setup(); + asShareOwner(); + Object.defineProperty(navigator, 'clipboard', { value: undefined, configurable: true, writable: true }); + Object.defineProperty(window, 'isSecureContext', { value: false, configurable: true, writable: true }); + const execCommand = vi.fn(() => true); + Object.defineProperty(document, 'execCommand', { value: execCommand, configurable: true, writable: true }); + server.use(http.get('/api/trips/1/invite-link', () => HttpResponse.json({ token: 'inv56' }))); + render(); + + await user.click((await screen.findByText('Copy')).closest('button')!); + + expect(execCommand).toHaveBeenCalledWith('copy'); + await screen.findByText('Copied'); + expect(document.querySelector('textarea')).toBeNull(); + }); + it('FE-COMP-MEMBERS-054: the invite-link copied badge resets after two seconds', async () => { asShareOwner(); const writeText = mockClipboard(); @@ -990,6 +1028,7 @@ describe('TripMembersModal', () => { vi.useFakeTimers(); fireEvent.click(copyBtn); + await act(async () => {}); expect(screen.getByText('Copied')).toBeInTheDocument(); act(() => { vi.advanceTimersByTime(2000); }); diff --git a/client/src/components/Trips/TripMembersModal.tsx b/client/src/components/Trips/TripMembersModal.tsx index b834e2127..787e9ad36 100644 --- a/client/src/components/Trips/TripMembersModal.tsx +++ b/client/src/components/Trips/TripMembersModal.tsx @@ -9,6 +9,7 @@ import { Crown, UserMinus, UserPlus, Users, LogOut, Link2, Trash2, Copy, Check, import { useTranslation } from '../../i18n' import { getApiErrorMessage } from '../../types' import CustomSelect from '../shared/CustomSelect' +import { copyText } from '../../utils/clipboard' interface AvatarProps { username: string @@ -78,13 +79,12 @@ function ShareLinkSection({ tripId, t }: { tripId: number; t: (key: string, para } catch { toast.error(t('common.error')) } } - const handleCopy = () => { - if (shareUrl) { - navigator.clipboard.writeText(shareUrl) - setCopied(true) - if (copyTimerRef.current) clearTimeout(copyTimerRef.current) - copyTimerRef.current = setTimeout(() => setCopied(false), 2000) - } + const handleCopy = async () => { + if (!shareUrl) return + if (!(await copyText(shareUrl))) return + setCopied(true) + if (copyTimerRef.current) clearTimeout(copyTimerRef.current) + copyTimerRef.current = setTimeout(() => setCopied(false), 2000) } if (loading) return null @@ -201,8 +201,8 @@ function TripInviteLinkSection({ tripId, t }: { tripId: number; t: (key: string, finally { setBusy(false) } } - const copy = () => { - navigator.clipboard.writeText(inviteUrl) + const copy = async () => { + if (!(await copyText(inviteUrl))) return setCopied(true) if (copyTimerRef.current) clearTimeout(copyTimerRef.current) copyTimerRef.current = setTimeout(() => setCopied(false), 2000) diff --git a/client/src/components/Vacay/VacayStats.tsx b/client/src/components/Vacay/VacayStats.tsx index 604918683..0e32961eb 100644 --- a/client/src/components/Vacay/VacayStats.tsx +++ b/client/src/components/Vacay/VacayStats.tsx @@ -98,7 +98,7 @@ interface StatCardProps { canEdit: boolean selectedYear: number isShiftedYear: boolean - onSave: (userId: number, year: number, days: number) => Promise + onSave: (year: number, days: number, targetUserId: number) => Promise t: TranslationFn } diff --git a/client/src/components/Vacay/holidayRegions.test.ts b/client/src/components/Vacay/holidayRegions.test.ts index a24678e65..4a9ad6b19 100644 --- a/client/src/components/Vacay/holidayRegions.test.ts +++ b/client/src/components/Vacay/holidayRegions.test.ts @@ -1,7 +1,9 @@ -// FE-COMP-VCYREG-001 to FE-COMP-VCYREG-007 +// FE-COMP-VCYREG-001 to FE-COMP-VCYREG-009 import { describe, it, expect, beforeEach } from 'vitest' import { http, HttpResponse } from 'msw' import { server } from '../../../tests/helpers/msw/server' +import { resetAllStores, seedStore } from '../../../tests/helpers/store' +import { useSettingsStore } from '../../store/settingsStore' import { fetchRegionOptions, fetchSchoolHolidayRegionOptions } from './holidayRegions' function holidaysRespond(rows: unknown) { @@ -14,6 +16,7 @@ function regionsRespond(body: unknown) { beforeEach(() => { server.resetHandlers() + resetAllStores() }) describe('fetchRegionOptions', () => { @@ -80,7 +83,8 @@ describe('fetchSchoolHolidayRegionOptions', () => { ]) }) - it('FE-COMP-VCYREG-007: a subdivision country prefers the German name and falls back to the code', async () => { + it('FE-COMP-VCYREG-007: a subdivision country takes the name in the active language and falls back to the code', async () => { + seedStore(useSettingsStore, { settings: { ...useSettingsStore.getState().settings, language: 'de' } }) regionsRespond({ groups: [], subdivisions: [ @@ -95,6 +99,19 @@ describe('fetchSchoolHolidayRegionOptions', () => { ]) }) + it('FE-COMP-VCYREG-009: a language the data has no name for falls back to English', async () => { + regionsRespond({ + groups: [], + subdivisions: [ + { code: 'DE-BY', name: [{ language: 'DE', text: 'Bayern' }, { language: 'EN', text: 'Bavaria' }] }, + ], + }) + + expect(await fetchSchoolHolidayRegionOptions('DE', 'fr')).toEqual([ + { value: 'DE-BY', label: 'Bavaria' }, + ]) + }) + it('FE-COMP-VCYREG-008: a failing region request degrades to no regions', async () => { server.use(http.get('/api/addons/vacay/school-holidays/regions/:country', () => HttpResponse.json({ error: 'down' }, { status: 500 }))) diff --git a/client/src/components/Vacay/holidayRegions.ts b/client/src/components/Vacay/holidayRegions.ts index 7ca5e9c5a..67d9122c1 100644 --- a/client/src/components/Vacay/holidayRegions.ts +++ b/client/src/components/Vacay/holidayRegions.ts @@ -1,4 +1,5 @@ import apiClient from '../../api/client' +import { useSettingsStore } from '../../store/settingsStore' import { SCHOOL_HOLIDAY_COUNTRY_CONFIG } from '../../vacay/schoolHolidayCountries' // Loads the subdivision (state/region) options for a holiday-calendar country. @@ -48,31 +49,40 @@ type OpenHolidayOption = { children?: OpenHolidayOption[] | null } -function localizedName(names: LocalizedName[] | undefined, fallback: string): string { - return names?.find(n => n.language?.toUpperCase() === 'DE')?.text +// OpenHolidays ships every region name in several languages, so pick the one the +// user actually reads before falling back to English. +function localizedName(names: LocalizedName[] | undefined, fallback: string, lang: string): string { + const want = lang.slice(0, 2).toUpperCase() + return names?.find(n => n.language?.toUpperCase() === want)?.text || names?.find(n => n.language?.toUpperCase() === 'EN')?.text || names?.[0]?.text || fallback } -function flattenOptions(items: OpenHolidayOption[] | undefined, prefix = ''): { value: string; label: string }[] { +function flattenOptions(items: OpenHolidayOption[] | undefined, lang: string, prefix = ''): { value: string; label: string }[] { const result: { value: string; label: string }[] = [] for (const item of items ?? []) { - const label = localizedName(item.name, item.shortName || item.code) + const label = localizedName(item.name, item.shortName || item.code, lang) result.push({ value: item.code, label: prefix ? `${prefix} / ${label}` : label }) - result.push(...flattenOptions(item.children ?? undefined, prefix ? `${prefix} / ${label}` : label)) + result.push(...flattenOptions(item.children ?? undefined, lang, prefix ? `${prefix} / ${label}` : label)) } return result } -export async function fetchSchoolHolidayRegionOptions(country: string): Promise<{ value: string; label: string }[]> { +// Read outside React: the two settings screens hand this function nothing but the +// country, and threading the language through both of them buys nothing. +function activeLanguage(): string { + return useSettingsStore.getState().settings.language || 'en' +} + +export async function fetchSchoolHolidayRegionOptions(country: string, lang = activeLanguage()): Promise<{ value: string; label: string }[]> { const config = SCHOOL_HOLIDAY_COUNTRY_CONFIG[country] if (!config || config.strategy === 'country') return [] try { const r = await apiClient.get(`/addons/vacay/school-holidays/regions/${country}`) if (config.strategy === 'groups') { - return flattenOptions(r.data.groups) + return flattenOptions(r.data.groups, lang) .map(opt => ({ value: `${country}|group:${opt.value}`, label: opt.label, @@ -80,7 +90,7 @@ export async function fetchSchoolHolidayRegionOptions(country: string): Promise< .sort((a, b) => a.label.localeCompare(b.label)) } - return flattenOptions(r.data.subdivisions) + return flattenOptions(r.data.subdivisions, lang) .map(opt => ({ value: opt.value, label: opt.label })) .sort((a, b) => a.label.localeCompare(b.label)) } catch { diff --git a/client/src/components/shared/CustomDateTimePicker.test.tsx b/client/src/components/shared/CustomDateTimePicker.test.tsx index d771b0f3f..fa72bc7ed 100644 --- a/client/src/components/shared/CustomDateTimePicker.test.tsx +++ b/client/src/components/shared/CustomDateTimePicker.test.tsx @@ -126,6 +126,26 @@ describe('CustomDatePicker', () => { expect(onChange).toHaveBeenCalledWith('2026-07-17'); }); + it('FE-COMP-DATEPICKER-031: text input rejects a day the month does not have', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', { name: /enter date manually/i })); + const input = screen.getByPlaceholderText('DD.MM.YYYY'); + fireEvent.change(input, { target: { value: '31.02.2026' } }); + fireEvent.keyDown(input, { key: 'Enter' }); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('FE-COMP-DATEPICKER-032: text input still accepts 29 February in a leap year', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', { name: /enter date manually/i })); + const input = screen.getByPlaceholderText('DD.MM.YYYY'); + fireEvent.change(input, { target: { value: '29.02.2024' } }); + fireEvent.keyDown(input, { key: 'Enter' }); + expect(onChange).toHaveBeenCalledWith('2024-02-29'); + }); + it('FE-COMP-DATEPICKER-014: Escape in text input cancels text mode', async () => { const user = userEvent.setup(); render(); diff --git a/client/src/components/shared/CustomDateTimePicker.tsx b/client/src/components/shared/CustomDateTimePicker.tsx index a04e38d52..d30227acf 100644 --- a/client/src/components/shared/CustomDateTimePicker.tsx +++ b/client/src/components/shared/CustomDateTimePicker.tsx @@ -240,6 +240,10 @@ export function CustomDatePicker({ } const year = y < 100 ? 2000 + y : y; if (m < 1 || m > 12 || d < 1 || d > 31) return; + // 31.02. parses fine as numbers but is no day at all; without the round-trip it would + // travel on as '2026-02-31' and come back out of new Date() as 3 March. + const probe = new Date(Date.UTC(year, m - 1, d)); + if (probe.getUTCFullYear() !== year || probe.getUTCMonth() !== m - 1 || probe.getUTCDate() !== d) return; const iso = `${year}-${String(m).padStart(2, '0')}-${String(d).padStart(2, '0')}`; if (isOutOfRange(iso)) return; onChange(iso); diff --git a/client/src/db/offlineDb.ts b/client/src/db/offlineDb.ts index 02328f250..a072bf761 100644 --- a/client/src/db/offlineDb.ts +++ b/client/src/db/offlineDb.ts @@ -51,6 +51,13 @@ export interface QueuedMutation { conflictServer?: unknown; /** When the conflict was detected (for ordering / display). */ conflictAt?: number; + /** + * When the row was marked 'syncing'. Only meaningful while it is — the flush + * that set it clears the row on success or moves it off 'syncing' on failure. + * A stamp that outlives its flush is how a killed tab is recognised on the + * next one (see mutationQueue's STUCK_SYNCING_MS). + */ + syncingSince?: number; } export interface SyncMeta { diff --git a/client/src/hooks/useGeolocation.test.ts b/client/src/hooks/useGeolocation.test.ts index 0f3bba137..ba536744c 100644 --- a/client/src/hooks/useGeolocation.test.ts +++ b/client/src/hooks/useGeolocation.test.ts @@ -1,4 +1,5 @@ -// FE-HOOK-GEO-001 to FE-HOOK-GEO-023 +// FE-HOOK-GEO-001 to FE-HOOK-GEO-025 +import { StrictMode } from 'react'; import { act, renderHook } from '@testing-library/react'; import { GeoOnceError, getCurrentPositionOnce, useGeolocation } from './useGeolocation'; @@ -221,6 +222,26 @@ describe('useGeolocation', () => { expect(clearWatch).toHaveBeenCalledWith(7); }); + it('FE-HOOK-GEO-015b: the updater form sees the freshest mode and starts one watch', async () => { + // The side effects run outside the state updater (React may invoke an + // updater more than once per commit, and startWatch awaits the iOS + // orientation prompt before it records the watch id), so the previous mode + // comes from a ref that setMode keeps in step within a batch. + const requestPermission = vi.fn().mockResolvedValue('granted'); + vi.stubGlobal('DeviceOrientationEvent', Object.assign(function () {}, { requestPermission })); + + const { result } = renderHook(() => useGeolocation(), { wrapper: StrictMode }); + + await act(async () => { + result.current.setMode('show'); + result.current.setMode(prev => (prev === 'show' ? 'follow' : 'off')); + await Promise.resolve(); + }); + + expect(result.current.mode).toBe('follow'); + expect(watchPosition).toHaveBeenCalledTimes(1); + }); + it('FE-HOOK-GEO-016: asks iOS for orientation permission and proceeds either way', async () => { const requestPermission = vi.fn().mockResolvedValue('denied'); vi.stubGlobal('DeviceOrientationEvent', Object.assign(function () {}, { requestPermission })); @@ -251,6 +272,39 @@ describe('useGeolocation', () => { expect(() => act(() => result.current.setMode('off'))).not.toThrow(); }); + it('FE-HOOK-GEO-024: stopping while the iOS prompt is open leaves no watch behind', async () => { + let grant!: () => void; + const requestPermission = vi.fn(() => new Promise(resolve => { grant = () => resolve('granted'); })); + vi.stubGlobal('DeviceOrientationEvent', Object.assign(function () {}, { requestPermission })); + + const { result } = renderHook(() => useGeolocation()); + act(() => result.current.setMode('show')); + act(() => result.current.setMode('off')); + + await act(async () => { grant(); await new Promise(r => setTimeout(r, 0)); }); + + expect(watchPosition).not.toHaveBeenCalled(); + }); + + it('FE-HOOK-GEO-025: stop and start during the iOS prompt still ends with one watch', async () => { + const grants: ((v: string) => void)[] = []; + const requestPermission = vi.fn(() => new Promise(resolve => { grants.push(resolve); })); + vi.stubGlobal('DeviceOrientationEvent', Object.assign(function () {}, { requestPermission })); + + const { result } = renderHook(() => useGeolocation()); + act(() => result.current.setMode('show')); + act(() => result.current.setMode('off')); + act(() => result.current.setMode('show')); + + await act(async () => { grants.forEach(g => g('granted')); await new Promise(r => setTimeout(r, 0)); }); + + // Both runs got past the prompt; only the newer one may subscribe, or the older + // watch keeps running with its id overwritten. + expect(requestPermission).toHaveBeenCalledTimes(2); + expect(watchPosition).toHaveBeenCalledTimes(1); + expect(result.current.mode).toBe('show'); + }); + it('FE-HOOK-GEO-019: stops the watch when the component unmounts', async () => { const { result, unmount } = renderHook(() => useGeolocation()); await act(() => result.current.cycleMode()); diff --git a/client/src/hooks/useGeolocation.ts b/client/src/hooks/useGeolocation.ts index e6f991082..671a1f404 100644 --- a/client/src/hooks/useGeolocation.ts +++ b/client/src/hooks/useGeolocation.ts @@ -93,12 +93,25 @@ export function getCurrentPositionOnce( export function useGeolocation(): UseGeolocationReturn { const [position, setPosition] = useState(null) const [mode, setModeState] = useState('off') + // What setMode's updater form reads instead of a state updater's argument: + // React may run an updater more than once per commit, and subscribing from + // inside one would leak a watch (see setMode below). + const modeRef = useRef(mode) + modeRef.current = mode const [error, setError] = useState(null) const watchIdRef = useRef(null) + // True between the start of startWatch and the watchPosition call it awaits. + const startingRef = useRef(false) + // Bumped by every start and by every stop, so a run parked on the iOS prompt can + // tell it has been superseded — clearing startingRef alone can't, a start that came + // after the stop has set it again by then. + const startRunRef = useRef(0) const orientationHandlerRef = useRef<((e: DeviceOrientationEvent) => void) | null>(null) const headingRef = useRef(null) const stopWatch = useCallback(() => { + startingRef.current = false + startRunRef.current++ if (watchIdRef.current !== null) { try { navigator.geolocation.clearWatch(watchIdRef.current) } catch { /* noop */ } watchIdRef.current = null @@ -116,6 +129,12 @@ export function useGeolocation(): UseGeolocationReturn { setError('Geolocation is not supported in this browser') return false } + // Already watching, or still waiting on the iOS prompt below: a second + // subscription would overwrite watchIdRef and leave the first one running + // with nobody left holding its id. + if (startingRef.current || watchIdRef.current !== null) return true + startingRef.current = true + const run = ++startRunRef.current setError(null) // iOS: ask for orientation permission up front; on Android and desktop @@ -129,6 +148,9 @@ export function useGeolocation(): UseGeolocationReturn { } } catch { /* older webkit throws — ignore and proceed */ } } + // Stopped (or restarted) while the prompt was open: subscribing now would leave a + // watch running that nobody holds the id of any more. + if (startRunRef.current !== run) return false // Device orientation → compass heading. `alpha` is rotation around the // Z-axis (0 = facing magnetic north on most devices). The webkit-only @@ -157,6 +179,7 @@ export function useGeolocation(): UseGeolocationReturn { window.addEventListener('deviceorientationabsolute', onOrientation as EventListener) window.addEventListener('deviceorientation', onOrientation as EventListener) + startingRef.current = false watchIdRef.current = navigator.geolocation.watchPosition( (pos) => { setPosition({ @@ -189,17 +212,18 @@ export function useGeolocation(): UseGeolocationReturn { }, [stopWatch]) const setMode = useCallback((m: TrackingMode | ((prev: TrackingMode) => TrackingMode)) => { - setModeState(prev => { - const next = typeof m === 'function' ? m(prev) : m - if (next === 'off') { - stopWatch() - setPosition(null) - } else if (watchIdRef.current === null) { - // started externally but no watch yet — start it - startWatch() - } - return next - }) + const next = typeof m === 'function' ? m(modeRef.current) : m + // Kept in step here as well, so two setMode calls batched into one commit + // still see each other's result. + modeRef.current = next + setModeState(next) + if (next === 'off') { + stopWatch() + setPosition(null) + } else if (watchIdRef.current === null) { + // started externally but no watch yet — start it + void startWatch() + } }, [startWatch, stopWatch]) const cycleMode = useCallback(async () => { diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index b3653355f..55c32438b 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -205,19 +205,28 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu polyline.push(c) } } - for (let i = 0; i < run.length - 1; i++) { + // Neighbouring legs that resolve to the SAME mode travel as one + // multi-waypoint request — the router answers with one leg per pair, so + // the per-connector segments stay exactly as they were, and a day without + // per-leg overrides is back to a single call per run. + let i = 0 + while (i < run.length - 1) { const mode = resolveLegMode(run[i], run[i + 1], dayDefaultMode) - const straight = (): [number, number][] => [[run[i].lat, run[i].lng], [run[i + 1].lat, run[i + 1].lng]] + let end = i + 1 + while (end < run.length - 1 && resolveLegMode(run[end], run[end + 1], dayDefaultMode) === mode) end++ + const chunk = run.slice(i, end + 1) + const straight = (): [number, number][] => chunk.map(p => [p.lat, p.lng] as [number, number]) try { - const r = await calculateRouteWithLegs([{ lat: run[i].lat, lng: run[i].lng }, { lat: run[i + 1].lat, lng: run[i + 1].lng }], { signal: controller.signal, profile: mode, tripId, dayId }) + const r = await calculateRouteWithLegs(chunk.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: mode, tripId, dayId }) pushCoords(r.coordinates.length >= 2 ? r.coordinates : straight()) - if (r.legs[0]) allLegs.push({ ...r.legs[0], mode }) + for (const leg of r.legs) allLegs.push({ ...leg, mode }) if (r.vias) allVias.push(...r.vias) } catch (err) { if (err instanceof Error && err.name === 'AbortError') throw err - // Routing failed for this leg — fall back to a straight line, no times. + // Routing failed for these legs — fall back to straight lines, no times. pushCoords(straight()) } + i = end } if (polyline.length >= 2) polylines.push(polyline) } diff --git a/client/src/i18n/TranslationContext.tsx b/client/src/i18n/TranslationContext.tsx index d320f3e1d..efacfb112 100644 --- a/client/src/i18n/TranslationContext.tsx +++ b/client/src/i18n/TranslationContext.tsx @@ -133,7 +133,9 @@ export function TranslationProvider({ children }: TranslationProviderProps) { let val: string = (strings[key] ?? en[key] ?? key) as string if (params) { Object.entries(params).forEach(([k, v]) => { - val = val.replace(new RegExp(`\\{${k}\\}`, 'g'), String(v)) + // Function replacement: a value carrying `$&` or `$1` (a trip named + // "A$&B") would otherwise be expanded as a replacement pattern. + val = val.replace(new RegExp(`\\{${k}\\}`, 'g'), () => String(v)) }) } return val @@ -145,7 +147,7 @@ export function TranslationProvider({ children }: TranslationProviderProps) { Object.entries(params).forEach(([k, v]) => { // Escape BEFORE substitution so a user-controlled value with `<` or // `&` cannot break out of the surrounding template's markup. - val = val.replace(new RegExp(`\\{${k}\\}`, 'g'), escapeHtml(String(v))) + val = val.replace(new RegExp(`\\{${k}\\}`, 'g'), () => escapeHtml(String(v))) }) } // Then re-sanitise the fully-built string: even if a translator ships a diff --git a/client/src/mobile/screens/admin/MAdmin.tsx b/client/src/mobile/screens/admin/MAdmin.tsx index 5f4923281..5130ba5f2 100644 --- a/client/src/mobile/screens/admin/MAdmin.tsx +++ b/client/src/mobile/screens/admin/MAdmin.tsx @@ -230,12 +230,14 @@ export default function MAdmin() { }} collabFeatures={collabFeatures} onToggleCollabFeature={async (key: string) => { - const next = { ...collabFeatures, [key]: !collabFeatures[key] } - setCollabFeatures(next) + const previous = collabFeatures[key] + setCollabFeatures({ ...collabFeatures, [key]: !previous }) try { - await adminApi.updateCollabFeatures({ [key]: next[key] }) + await adminApi.updateCollabFeatures({ [key]: !previous }) } catch { - setCollabFeatures(collabFeatures) + // Only this key rolls back — a slower request must not undo a toggle + // the admin made in the meantime. + setCollabFeatures(prev => ({ ...prev, [key]: previous })) } }} /> diff --git a/client/src/mobile/screens/dashboard/MNewTripSheet.tsx b/client/src/mobile/screens/dashboard/MNewTripSheet.tsx index 654e054e0..e046b0a54 100644 --- a/client/src/mobile/screens/dashboard/MNewTripSheet.tsx +++ b/client/src/mobile/screens/dashboard/MNewTripSheet.tsx @@ -89,6 +89,13 @@ export default function MNewTripSheet({ open, trip, onClose, onSave, onCoverUpda setError('') }, [trip, open]) + // The local file preview is a blob url; release it once a new cover replaces it + // or the sheet goes away. Server and Unsplash urls are left alone. + useEffect(() => { + if (!coverPreview?.startsWith('blob:')) return + return () => { URL.revokeObjectURL(coverPreview) } + }, [coverPreview]) + // Moving the start keeps the trip length (same rule as TripFormModal). const changeStart = (value: string) => { if (value && endDate && startDate && endDate >= startDate) { diff --git a/client/src/mobile/screens/journey/MJourneyDetail.tsx b/client/src/mobile/screens/journey/MJourneyDetail.tsx index 012c17dd3..058b5ebdd 100644 --- a/client/src/mobile/screens/journey/MJourneyDetail.tsx +++ b/client/src/mobile/screens/journey/MJourneyDetail.tsx @@ -11,7 +11,7 @@ import { useJourneyDetail } from '../../../pages/journeyDetail/useJourneyDetail' import { useJourneyStore } from '../../../store/journeyStore' import type { JourneyEntry, GalleryPhoto } from '../../../store/journeyStore' import { useAuthStore } from '../../../store/authStore' -import { journeyApi, addonsApi } from '../../../api/client' +import { journeyApi, addonsApi, memoriesApi } from '../../../api/client' import { normalizeImageFiles } from '../../../utils/convertHeic' import { isVideoFile } from '../../../utils/videoPoster' import { getApiErrorMessage } from '../../../types' @@ -135,7 +135,8 @@ export default function MJourneyDetail() { const [uploading, setUploading] = useState(false) useEffect(() => { - (async () => { + let active = true + ;(async () => { try { const addonsData = await addonsApi.enabled() const enabled = (addonsData.addons || []).filter( @@ -143,14 +144,19 @@ export default function MJourneyDetail() { ) const connected: { id: string; name: string }[] = [] for (const p of enabled) { + // The probes run one after another, so leaving the screen has to stop + // the queue rather than let every remaining provider be asked anyway. + if (!active) return try { - const res = await fetch(`/api/integrations/memories/${p.id}/status`, { credentials: 'include' }) - if (res.ok && (await res.json()).connected) connected.push({ id: p.id, name: p.name }) + // Same probe the desktop gallery uses, so a NAS cannot read as + // connected on one surface and not the other. + if ((await memoriesApi.status(p.id)).connected) connected.push({ id: p.id, name: p.name }) } catch { /* provider stays hidden */ } } - setAvailableProviders(connected) + if (active) setAvailableProviders(connected) } catch { /* no providers */ } })() + return () => { active = false } }, []) const handleGalleryUpload = async (e: React.ChangeEvent) => { diff --git a/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx b/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx index 33f4db055..9a049ef48 100644 --- a/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx +++ b/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx @@ -101,6 +101,11 @@ export default function MJourneyEntrySheet({ [addons], ) + // Minting the preview inside the tile markup would hand out a fresh blob URL on + // every keystroke in the story field, and nothing would ever release them. + const pendingPreviews = useMemo(() => pendingFiles.map(f => URL.createObjectURL(f)), [pendingFiles]) + useEffect(() => () => { pendingPreviews.forEach(url => URL.revokeObjectURL(url)) }, [pendingPreviews]) + useEffect(() => { if (readOnly) return // App.tsx loads the addon list on boot, so this only reacts to it. Kicking off @@ -536,13 +541,23 @@ export default function MJourneyEntrySheet({ + ), })); @@ -1569,8 +1574,38 @@ describe('AdminPage', () => { fireEvent.click(screen.getByRole('button', { name: /toggle chat/i })); - // The rollback re-renders the stub; the addon panel stays mounted either way - await waitFor(() => expect(screen.getByTestId('addon-manager')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + }); + + // A snapshot-wide rollback would undo the second toggle as well, leaving the + // panel disagreeing with the server until a reload. + it('a failing collab toggle keeps a second toggle made while it was in flight', async () => { + server.use( + http.get('/api/admin/collab-features', () => + HttpResponse.json({ chat: true, notes: true, polls: true, whatsnext: true }) + ), + http.put('/api/admin/collab-features', async ({ request }) => { + const body = (await request.json()) as Record; + if ('chat' in body) { + await delay(30); + return HttpResponse.json({}, { status: 500 }); + } + return HttpResponse.json({ success: true }); + }) + ); + + seedStore(useAuthStore, { isAuthenticated: true, user: buildAdmin() }); + render(); + + await waitFor(() => expect(screen.getByRole('button', { name: /^users$/i })).toBeInTheDocument()); + fireEvent.click(screen.getByRole('button', { name: /^addons$/i })); + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + + fireEvent.click(screen.getByRole('button', { name: /toggle chat/i })); + fireEvent.click(screen.getByRole('button', { name: /toggle notes/i })); + + await waitFor(() => expect(screen.getByTestId('collab-chat-state')).toHaveTextContent('true')); + expect(screen.getByTestId('collab-notes-state')).toHaveTextContent('false'); }); }); diff --git a/client/src/pages/AdminPage.tsx b/client/src/pages/AdminPage.tsx index 16e31f8a6..60f511300 100644 --- a/client/src/pages/AdminPage.tsx +++ b/client/src/pages/AdminPage.tsx @@ -161,9 +161,15 @@ function AdminPageDesktop(): React.ReactElement { setBagTrackingEnabled(next) try { await adminApi.updateBagTracking(next) } catch { setBagTrackingEnabled(!next) } }} collabFeatures={collabFeatures} onToggleCollabFeature={async (key: string) => { - const next = { ...collabFeatures, [key]: !collabFeatures[key] } - setCollabFeatures(next) - try { await adminApi.updateCollabFeatures({ [key]: next[key] }) } catch { setCollabFeatures(collabFeatures) } + const previous = collabFeatures[key] + setCollabFeatures({ ...collabFeatures, [key]: !previous }) + try { + await adminApi.updateCollabFeatures({ [key]: !previous }) + } catch { + // Only this key rolls back — a slower request must not undo a toggle + // the admin made in the meantime. + setCollabFeatures(prev => ({ ...prev, [key]: previous })) + } }} /> )} diff --git a/client/src/pages/JourneyDetailPage.test.tsx b/client/src/pages/JourneyDetailPage.test.tsx index 4e54ca9b5..3c116a745 100644 --- a/client/src/pages/JourneyDetailPage.test.tsx +++ b/client/src/pages/JourneyDetailPage.test.tsx @@ -3720,13 +3720,15 @@ describe('JourneyDetailPage', () => { it('clicking Copy on share link copies to clipboard', async () => { const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); - // Mock clipboard + // Mock clipboard. jsdom leaves isSecureContext undefined, which the copy + // handler reads as plain HTTP and answers with its execCommand fallback. const mockWriteText = vi.fn().mockResolvedValue(undefined); Object.defineProperty(navigator, 'clipboard', { value: { writeText: mockWriteText }, writable: true, configurable: true, }); + Object.defineProperty(window, 'isSecureContext', { value: true, writable: true, configurable: true }); server.use( http.get('/api/journeys/1/share-link', () => { diff --git a/client/src/pages/JourneyDetailPage.tsx b/client/src/pages/JourneyDetailPage.tsx index 0b95dce89..2feea2258 100644 --- a/client/src/pages/JourneyDetailPage.tsx +++ b/client/src/pages/JourneyDetailPage.tsx @@ -1,4 +1,3 @@ -import { useRef } from 'react' import { useAuthStore } from '../store/authStore' import { journeyApi } from '../api/client' import Navbar from '../components/Layout/Navbar' @@ -45,14 +44,12 @@ function JourneyDetailPageDesktop() { showInvite, setShowInvite, showAddTrip, setShowAddTrip, unlinkTrip, setUnlinkTrip, showSettings, setShowSettings, hideSkeletons, setHideSkeletons, - mapRef, fullMapRef, activeLocationId, handleMarkerClick, handleLocationClick, + mapRef, fullMapRef, galleryUploadRef, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks, feedEdge, scrollFeedTo, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto, } = useJourneyDetail() - const galleryUploadRef = useRef<(() => void) | null>(null) - if (loading || !current) { return (
diff --git a/client/src/pages/JourneyDetailPage.wiring.test.tsx b/client/src/pages/JourneyDetailPage.wiring.test.tsx index 6d867427e..8947886df 100644 --- a/client/src/pages/JourneyDetailPage.wiring.test.tsx +++ b/client/src/pages/JourneyDetailPage.wiring.test.tsx @@ -98,7 +98,7 @@ function buildHook(over: Record = {}): Record unlinkTrip: null, setUnlinkTrip: vi.fn(), showSettings: false, setShowSettings: vi.fn(), hideSkeletons: false, setHideSkeletons: vi.fn(), - mapRef: { current: null }, fullMapRef: { current: null }, + mapRef: { current: null }, fullMapRef: { current: null }, galleryUploadRef: { current: null }, activeLocationId: null, handleMarkerClick: vi.fn(), handleLocationClick: vi.fn(), mapEntries: [], sidebarMapItems: [], tripDates: new Set(), isMobile: false, feedEdge: { atTop: true, atBottom: true }, scrollFeedTo: vi.fn(), diff --git a/client/src/pages/admin/useAdmin.test.tsx b/client/src/pages/admin/useAdmin.test.tsx index dc0f31c4b..00fec9a89 100644 --- a/client/src/pages/admin/useAdmin.test.tsx +++ b/client/src/pages/admin/useAdmin.test.tsx @@ -1,4 +1,4 @@ -// FE-ADMHOOK-001 to FE-ADMHOOK-045 +// FE-ADMHOOK-001 to FE-ADMHOOK-049 import { http, HttpResponse } from 'msw'; import React from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -32,8 +32,13 @@ function wrapper({ children }: { children: React.ReactNode }) { } /** Mounts the hook and waits until the initial parallel loads have settled. */ -async function mountAdmin() { - const view = renderHook(() => useAdmin(), { wrapper }); +async function mountAdmin(url = '/admin') { + const at = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const view = renderHook(() => useAdmin(), { wrapper: url === '/admin' ? wrapper : at }); await waitFor(() => expect(view.result.current.isLoading).toBe(false)); return view; } @@ -732,6 +737,36 @@ describe('useAdmin', () => { expect(result.current.users).toHaveLength(2); }); + it('FE-ADMHOOK-046: opens on the tab the URL asks for', async () => { + const { result } = await mountAdmin('/admin?tab=audit'); + expect(result.current.activeTab).toBe('audit'); + }); + + /* + * A tab id with no panel behind it — a typo, or a link from an install that + * has tabs this one does not — used to open the page on an empty content + * area with nothing selected in the sidebar. + */ + it('FE-ADMHOOK-047: falls back to users when the URL names a tab that does not exist', async () => { + const { result } = await mountAdmin('/admin?tab=nonsense'); + expect(result.current.activeTab).toBe('users'); + }); + + /* + * The sidebar entries are built conditionally but the panels are not, so a + * hosted install had its hidden panels one URL away. + */ + it('FE-ADMHOOK-048: refuses a tab a managed install hides', async () => { + seedStore(useAuthStore, { isAuthenticated: true, user: me, managed: true }); + const { result } = await mountAdmin('/admin?tab=backup'); + await waitFor(() => expect(result.current.activeTab).toBe('users')); + }); + + it('FE-ADMHOOK-049: keeps that same tab on an install that runs itself', async () => { + const { result } = await mountAdmin('/admin?tab=backup'); + expect(result.current.activeTab).toBe('backup'); + }); + it('FE-ADMHOOK-044: mcpEnabled follows the addon store', async () => { const { result } = await mountAdmin(); diff --git a/client/src/pages/admin/useAdmin.ts b/client/src/pages/admin/useAdmin.ts index 4b2d6d8dc..bc03d9ee4 100644 --- a/client/src/pages/admin/useAdmin.ts +++ b/client/src/pages/admin/useAdmin.ts @@ -7,8 +7,32 @@ import { useAddonStore } from '../../store/addonStore' import { useTranslation } from '../../i18n' import { getApiErrorMessage } from '../../types' import { useToast } from '../../components/shared/Toast' +import { managedAdminTabs } from '../../managed' import type { AdminUser, AdminStats, OidcConfig, UpdateInfo } from './adminModel' +/** + * Every tab id AdminPage can render a panel for, whatever this install offers. + * + * ?tab= is user input and was taken as read: an id with no panel behind it — + * a typo, or a link from an install that has tabs this one does not — opened + * the admin page on an empty content area with nothing selected in the + * sidebar. Labels and icons stay in AdminPage; this is only the vocabulary. + */ +const ADMIN_TAB_IDS = [ + 'users', 'defaults', 'config', 'settings', 'addons', 'plugins', 'storage', + 'notifications', 'mcp-tokens', 'github', 'backup', 'audit', 'dev-notifications', +] + +/** + * The ones a managed install deliberately does not offer. + * + * The sidebar entries are built conditionally, but the panels are not, so + * ?tab=backup opened a backup schedule that competes with the real one on a + * hosted instance. Checked in an effect rather than in the initial state + * because `managed` arrives with /app-config, after the first render. + */ +const MANAGED_HIDDEN = ['storage', 'github', 'backup'] + /** * Admin page logic — owns every admin data slice (users, stats, invites, auth * toggles, OIDC, feature flags, API keys, SMTP, version/update) plus the CRUD @@ -29,7 +53,11 @@ export function useAdmin() { // page with eleven of them. Read once for the initial value and written back // on every change, so the address bar keeps saying where the reader is. const [searchParams, setSearchParams] = useSearchParams() - const [activeTab, setActiveTabState] = useState(() => searchParams.get('tab') || 'users') + const [activeTab, setActiveTabState] = useState(() => { + const asked = searchParams.get('tab') + const known = [...ADMIN_TAB_IDS, ...managedAdminTabs.map(tab => tab.id)] + return asked && known.includes(asked) ? asked : 'users' + }) const setActiveTab = useCallback((tab: string) => { setActiveTabState(tab) // replace, not push: eleven tabs would otherwise fill the back button with @@ -41,6 +69,10 @@ export function useAdmin() { return next }, { replace: true }) }, [setSearchParams]) + useEffect(() => { + if (managed && MANAGED_HIDDEN.includes(activeTab)) setActiveTab('users') + }, [managed, activeTab, setActiveTab]) + const [users, setUsers] = useState([]) const [stats, setStats] = useState(null) const [isLoading, setIsLoading] = useState(true) diff --git a/client/src/pages/join/useJoinTrip.test.ts b/client/src/pages/join/useJoinTrip.test.ts index 788a325f3..59cb72a51 100644 --- a/client/src/pages/join/useJoinTrip.test.ts +++ b/client/src/pages/join/useJoinTrip.test.ts @@ -1,4 +1,4 @@ -// FE-PAGE-JOIN-001 to FE-PAGE-JOIN-008 +// FE-PAGE-JOIN-001 to FE-PAGE-JOIN-011 import { act, renderHook, waitFor } from '@testing-library/react'; import { tripInviteApi } from '../../api/client'; import { useJoinTrip } from './useJoinTrip'; @@ -11,8 +11,12 @@ vi.mock('react-router', () => ({ useParams: () => params, })); +const addToast = vi.fn(); + beforeEach(() => { navigate.mockClear(); + addToast.mockClear(); + window.__addToast = addToast; params = { token: 'inv-1' }; vi.spyOn(tripInviteApi, 'preview').mockResolvedValue({ title: 'Japan 2026' } as never); vi.spyOn(tripInviteApi, 'accept').mockResolvedValue({ trip_id: 42 } as never); @@ -20,6 +24,7 @@ beforeEach(() => { afterEach(() => { vi.restoreAllMocks(); + delete window.__addToast; }); describe('useJoinTrip', () => { @@ -74,8 +79,8 @@ describe('useJoinTrip', () => { await waitFor(() => expect(navigate).toHaveBeenCalledWith('/trips/7', { replace: true })); }); - it('FE-PAGE-JOIN-006: falls back to invalid when redeeming fails', async () => { - vi.mocked(tripInviteApi.accept).mockRejectedValue(new Error('already a member')); + it('FE-PAGE-JOIN-006: falls back to invalid when the invite itself is gone', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue({ response: { status: 404, data: { error: 'Invalid or expired invite link' } } }); const { result } = renderHook(() => useJoinTrip()); await waitFor(() => expect(result.current.state).toBe('ready')); @@ -84,6 +89,34 @@ describe('useJoinTrip', () => { expect(navigate).not.toHaveBeenCalled(); }); + // A rate limit or a 500 is not an expired invite: saying so burns a link that + // still works, and the invalid screen offers no way back. + it('FE-PAGE-JOIN-010: a server failure reports itself and leaves the invite retryable', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue({ response: { status: 429, data: { error: 'Too many attempts. Please try again later.' } } }); + const { result } = renderHook(() => useJoinTrip()); + await waitFor(() => expect(result.current.state).toBe('ready')); + + act(() => result.current.accept()); + await waitFor(() => expect(result.current.state).toBe('ready')); + expect(addToast).toHaveBeenCalledWith('Too many attempts. Please try again later.', 'error', undefined); + expect(navigate).not.toHaveBeenCalled(); + + // And the second press really does go out again. + vi.mocked(tripInviteApi.accept).mockResolvedValue({ trip_id: 42 } as never); + await act(() => { result.current.accept(); return Promise.resolve(); }); + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/trips/42', { replace: true })); + }); + + it('FE-PAGE-JOIN-011: a network failure with no status is reported, not called invalid', async () => { + vi.mocked(tripInviteApi.accept).mockRejectedValue(new Error('Network Error')); + const { result } = renderHook(() => useJoinTrip()); + await waitFor(() => expect(result.current.state).toBe('ready')); + + act(() => result.current.accept()); + await waitFor(() => expect(addToast).toHaveBeenCalled()); + expect(result.current.state).toBe('ready'); + }); + it('FE-PAGE-JOIN-007: accept is a no-op without a token', () => { params = {}; const { result } = renderHook(() => useJoinTrip()); diff --git a/client/src/pages/join/useJoinTrip.ts b/client/src/pages/join/useJoinTrip.ts index 0a7b8f14d..22c575592 100644 --- a/client/src/pages/join/useJoinTrip.ts +++ b/client/src/pages/join/useJoinTrip.ts @@ -1,6 +1,9 @@ import { useEffect, useState } from 'react' import { useNavigate, useParams } from 'react-router' import { tripInviteApi } from '../../api/client' +import { useTranslation } from '../../i18n' +import { getApiErrorMessage } from '../../types' +import { useToast } from '../../components/shared/Toast' export type JoinTripState = 'loading' | 'ready' | 'joining' | 'invalid' @@ -12,6 +15,8 @@ export type JoinTripState = 'loading' | 'ready' | 'joining' | 'invalid' export function useJoinTrip() { const { token } = useParams<{ token: string }>() const navigate = useNavigate() + const { t } = useTranslation() + const toast = useToast() const [state, setState] = useState('loading') const [title, setTitle] = useState('') @@ -30,7 +35,17 @@ export function useJoinTrip() { setState('joining') tripInviteApi.accept(token) .then((data: { trip_id: number }) => navigate(`/trips/${data.trip_id}`, { replace: true })) - .catch(() => setState('invalid')) + .catch((err: unknown) => { + // Only a 404 means the invite itself is gone. A rate limit or a server + // having a bad minute used to be reported as an expired invite too, + // which sent the user back to the dashboard with no way to try again. + if ((err as { response?: { status?: number } }).response?.status === 404) { + setState('invalid') + return + } + setState('ready') + toast.error(getApiErrorMessage(err, t('common.error'))) + }) } const goToDashboard = () => navigate('/dashboard', { replace: true }) diff --git a/client/src/pages/journeyDetail/useJourneyDetail.ts b/client/src/pages/journeyDetail/useJourneyDetail.ts index e84ac47ad..d8c9edef9 100644 --- a/client/src/pages/journeyDetail/useJourneyDetail.ts +++ b/client/src/pages/journeyDetail/useJourneyDetail.ts @@ -29,6 +29,8 @@ export function useJourneyDetail() { const { current, loading, notFound, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto } = useJourneyStore() const mapRef = useRef(null) const fullMapRef = useRef(null) + /** The gallery hands its file picker up here, so the hero button can open it. */ + const galleryUploadRef = useRef<(() => void) | null>(null) const [activeLocationId, setActiveLocationId] = useState(null) const isMobile = useIsMobile() @@ -382,7 +384,7 @@ export function useJourneyDetail() { showInvite, setShowInvite, showAddTrip, setShowAddTrip, unlinkTrip, setUnlinkTrip, showSettings, setShowSettings, hideSkeletons, setHideSkeletons, - mapRef, fullMapRef, activeLocationId, handleMarkerClick, handleLocationClick, + mapRef, fullMapRef, galleryUploadRef, activeLocationId, handleMarkerClick, handleLocationClick, mapEntries, sidebarMapItems, tripDates, isMobile, tracks, feedEdge, scrollFeedTo, loadJourney, updateEntry, deleteEntry, reorderEntries, uploadPhotos, deletePhoto, diff --git a/client/src/pages/journeyStudio/useJourneyStudio.test.tsx b/client/src/pages/journeyStudio/useJourneyStudio.test.tsx new file mode 100644 index 000000000..f8e049964 --- /dev/null +++ b/client/src/pages/journeyStudio/useJourneyStudio.test.tsx @@ -0,0 +1,149 @@ +import { act, renderHook, waitFor } from '@testing-library/react' +import type { JourneyStats } from '@trek/shared' +import { useStudioStore } from '../../store/studioStore' +import { useJourneyStore } from '../../store/journeyStore' +import { journeyApi } from '../../api/client' + +/** + * Studio's shell state (#1973), for the two things that cost work rather than + * pixels: the keyboard reaching past an open text field, and the layout input + * being frozen before the track has arrived. + */ + +vi.mock('react-router', () => ({ + useParams: () => ({ id: '9' }), + useNavigate: () => vi.fn(), + useLocation: () => ({ state: null }), +})) + +vi.mock('../../api/websocket', async importOriginal => ({ + ...await importOriginal(), + addListener: vi.fn(), + removeListener: vi.fn(), + joinBook: vi.fn(), + leaveBook: vi.fn(), + sendBookCursor: vi.fn(), +})) + +const autoLayout = await vi.importActual( + '../../components/Studio/autoLayout', +) +const buildBook = vi.fn(autoLayout.buildBook) +vi.mock('../../components/Studio/autoLayout', async importOriginal => { + const actual = await importOriginal() + return { ...actual, buildBook: (...args: Parameters) => buildBook(...args) } +}) + +import { useJourneyStudio } from './useJourneyStudio' + +const TRACK = [[52.5, 13.4], [52.6, 13.5], [52.7, 13.6]] + +const stats = (): JourneyStats => ({ + journeyId: 9, distance: 1000, days: 2, steps: 2, photos: 0, places: 2, furthest: 0, + countries: [], trips: [], start: '2026-06-02', end: '2026-06-03', + points: [], +} as unknown as JourneyStats) + +/** A journey with one entry, which is all the auto layout needs to build. */ +function seedJourney() { + useJourneyStore.setState({ + current: { + id: 9, title: 'Iceland', entries: [ + { id: 1, type: 'entry', title: 'A day', story: 'Something happened.', entry_date: '2026-06-02', photos: [] }, + ], gallery: [], trips: [], + } as never, + loading: false, + }) +} + +beforeEach(() => { + seedJourney() + buildBook.mockClear() + useStudioStore.setState({ doc: null, selection: [], activeSpread: 0 }) + vi.spyOn(journeyApi, 'getBook').mockResolvedValue({ book: null } as never) + vi.spyOn(journeyApi, 'saveBook').mockResolvedValue({} as never) + vi.spyOn(journeyApi, 'stats').mockResolvedValue(stats() as never) + vi.spyOn(journeyApi, 'listTracks').mockResolvedValue({ tracks: [{ points: TRACK }] } as never) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +/** Mount and wait until the one-shot build has run. */ +async function mountStudio() { + const view = renderHook(() => useJourneyStudio()) + await waitFor(() => expect(useStudioStore.getState().doc).not.toBeNull()) + return view +} + +describe('the keyboard', () => { + /* + * Studio text commits on blur, so the sentence being typed is not in the undo + * history yet: a Ctrl+Z inside a text box took back the change before it and + * left the mistyped word exactly where it was. + */ + it('leaves undo to the field somebody is typing in', async () => { + const undo = vi.fn() + useStudioStore.setState({ undo }) + await mountStudio() + + const field = document.createElement('textarea') + document.body.appendChild(field) + act(() => { + field.dispatchEvent(new KeyboardEvent('keydown', { key: 'z', ctrlKey: true, bubbles: true })) + }) + + expect(undo).not.toHaveBeenCalled() + field.remove() + }) + + it('undoes the document when nothing is being typed into', async () => { + const undo = vi.fn() + useStudioStore.setState({ undo }) + await mountStudio() + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'z', ctrlKey: true, bubbles: true })) + }) + + expect(undo).toHaveBeenCalledTimes(1) + }) +}) + +describe('the auto layout input', () => { + /* + * The build waits on the book and the figures, never on the track, so the + * input was frozen with an empty path — and a relayout afterwards drew the + * straight ruler line over a route somebody had walked. + */ + it('picks up the track that lands after the book was built', async () => { + // Held back on purpose: the tracks request is the slow one in practice, and + // the build does not wait for it. + let release: (value: unknown) => void = () => {} + vi.mocked(journeyApi.listTracks).mockReturnValue(new Promise(resolve => { release = resolve }) as never) + + const { result } = await mountStudio() + await act(async () => { + release({ tracks: [{ points: TRACK }] }) + await Promise.resolve() + }) + + act(() => { result.current.relayoutBook() }) + + expect(buildBook).toHaveBeenCalled() + const input = buildBook.mock.calls[buildBook.mock.calls.length - 1][0] + expect(input.path).toEqual([TRACK]) + }) + + it('leaves the path empty when the journey has no track', async () => { + vi.mocked(journeyApi.listTracks).mockResolvedValue({ tracks: [] } as never) + const { result } = await mountStudio() + await waitFor(() => expect(journeyApi.listTracks).toHaveBeenCalled()) + + act(() => { result.current.relayoutBook() }) + + const input = buildBook.mock.calls[buildBook.mock.calls.length - 1][0] + expect(input.path).toEqual([]) + }) +}) diff --git a/client/src/pages/journeyStudio/useJourneyStudio.ts b/client/src/pages/journeyStudio/useJourneyStudio.ts index 655fe6f55..38ddd841a 100644 --- a/client/src/pages/journeyStudio/useJourneyStudio.ts +++ b/client/src/pages/journeyStudio/useJourneyStudio.ts @@ -389,9 +389,21 @@ export function useJourneyStudio() { useEffect(() => { if (!Number.isFinite(journeyId)) return let cancelled = false + /* + * The frozen layout input is patched too, not just the state. + * + * The build effect runs as soon as the book and the figures are in, which is + * usually before this answers, and what it froze is what the Auto layout + * buttons hand to buildBook afterwards. Without the patch a relayout drew + * the straight line between stops over a route somebody had walked. + */ + const applyPath = (p: [number, number][][]) => { + setPath(p) + if (autoInput.current) autoInput.current = { ...autoInput.current, path: p } + } journeyApi.listTracks(journeyId) - .then(data => { if (!cancelled) setPath(bookPath(data.tracks)) }) - .catch(() => { if (!cancelled) setPath([]) }) + .then(data => { if (!cancelled) applyPath(bookPath(data.tracks)) }) + .catch(() => { if (!cancelled) applyPath([]) }) return () => { cancelled = true } }, [journeyId]) @@ -589,8 +601,13 @@ export function useJourneyStudio() { useEffect(() => { const onKey = (e: KeyboardEvent) => { - const tag = (e.target as HTMLElement)?.tagName - const typing = tag === 'INPUT' || tag === 'TEXTAREA' + const el = e.target as HTMLElement | null + const typing = el?.tagName === 'INPUT' || el?.tagName === 'TEXTAREA' || el?.isContentEditable === true + + // Undo belongs to the field while somebody is in one: Studio text commits + // on blur, so the sentence being typed is not in the history yet and the + // document-level undo would take back the change before it instead. + if (typing) return if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'z') { e.preventDefault() diff --git a/client/src/repo/tripRepo.test.ts b/client/src/repo/tripRepo.test.ts index 56dc0e027..e4e422c04 100644 --- a/client/src/repo/tripRepo.test.ts +++ b/client/src/repo/tripRepo.test.ts @@ -1,4 +1,4 @@ -// FE-REPO-TRIP-001 to FE-REPO-TRIP-007 +// FE-REPO-TRIP-001 to FE-REPO-TRIP-011 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import 'fake-indexeddb/auto' import { http, HttpResponse } from 'msw' @@ -97,3 +97,50 @@ describe('tripRepo.get', () => { expect(result.trip.title).toBe('Fallback') }) }) + +describe('tripRepo.active', () => { + // Local calendar dates, like the repo itself. + function dateOffset(days: number): string { + const d = new Date() + d.setDate(d.getDate() + days) + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}` + } + + it('FE-REPO-TRIP-008: online — passes the server answer straight through', async () => { + server.use(http.get('/api/trips/active', () => + HttpResponse.json({ trip: { id: 90, title: 'Server pick', start_date: null, end_date: null } }), + )) + + const result = await tripRepo.active() + expect(result.trip!.id).toBe(90) + }) + + it('FE-REPO-TRIP-009: offline — ranks ongoing over upcoming over past', async () => { + await offlineDb.trips.put(buildTrip({ id: 91, start_date: dateOffset(-40), end_date: dateOffset(-30) })) + await offlineDb.trips.put(buildTrip({ id: 92, start_date: dateOffset(10), end_date: dateOffset(14) })) + await offlineDb.trips.put(buildTrip({ id: 93, start_date: dateOffset(-1), end_date: dateOffset(2) })) + setOnline(false) + + expect((await tripRepo.active()).trip!.id).toBe(93) + + await offlineDb.trips.delete(93) + expect((await tripRepo.active()).trip!.id).toBe(92) + + await offlineDb.trips.delete(92) + expect((await tripRepo.active()).trip!.id).toBe(91) + }) + + it('FE-REPO-TRIP-010: offline — skips archived trips and answers null when nothing is left', async () => { + await offlineDb.trips.put(buildTrip({ id: 94, is_archived: 1, start_date: dateOffset(-1), end_date: dateOffset(2) })) + setOnline(false) + + expect((await tripRepo.active()).trip).toBeNull() + }) + + it('FE-REPO-TRIP-011: an HTTP error still rejects instead of falling back', async () => { + await offlineDb.trips.put(buildTrip({ id: 95 })) + server.use(http.get('/api/trips/active', () => HttpResponse.json({ error: 'boom' }, { status: 500 }))) + + await expect(tripRepo.active()).rejects.toThrow() + }) +}) diff --git a/client/src/repo/tripRepo.ts b/client/src/repo/tripRepo.ts index e3337dd04..76d2798de 100644 --- a/client/src/repo/tripRepo.ts +++ b/client/src/repo/tripRepo.ts @@ -2,6 +2,32 @@ import { tripsApi } from '../api/client' import { offlineDb, upsertTrip } from '../db/offlineDb' import { onlineThenCache } from './withOfflineFallback' import type { Trip } from '../types' +import type { ActiveTripResponse } from '@trek/shared' + +/** + * Offline stand-in for GET /trips/active. Mirrors the server's ranking + * (trips.service.ts activeTrip): the trip running today, else the next one to + * start, else the most recently started. Dates are plain local calendar dates. + */ +function pickActive(trips: Trip[]): Trip | null { + const now = new Date() + const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}` + const relevance = (t: Trip): number => { + if (t.start_date && t.end_date && t.start_date <= today && t.end_date >= today) return 0 + if (t.start_date && t.start_date >= today) return 1 + return 2 + } + const ranked = [...trips].sort((a, b) => { + const ra = relevance(a) + const rb = relevance(b) + if (ra !== rb) return ra - rb + // Upcoming: soonest first. Everything else: most recently started first. + return ra === 2 + ? (b.start_date ?? '').localeCompare(a.start_date ?? '') + : (a.start_date ?? '').localeCompare(b.start_date ?? '') + }) + return ranked[0] ?? null +} export const tripRepo = { async list(): Promise<{ trips: Trip[]; archivedTrips: Trip[] }> { @@ -25,6 +51,26 @@ export const tripRepo = { ) }, + /** + * The startup redirect asks for this on the very first paint, so it has to + * answer offline too — otherwise "open my active trip on startup" drops the + * user on the dashboard whenever the launch has no network. + */ + async active(): Promise { + return onlineThenCache( + () => tripsApi.active(), + async () => { + const all = await offlineDb.trips.toArray() + const trip = pickActive(all.filter(t => !t.is_archived)) + return { + trip: trip + ? { id: trip.id, title: trip.title, start_date: trip.start_date, end_date: trip.end_date } + : null, + } + }, + ) + }, + async get(tripId: number | string): Promise<{ trip: Trip }> { return onlineThenCache( async () => { diff --git a/client/src/store/authStore.ts b/client/src/store/authStore.ts index 4355d424c..224f67fa1 100644 --- a/client/src/store/authStore.ts +++ b/client/src/store/authStore.ts @@ -7,7 +7,7 @@ import { getApiErrorMessage } from '../types' import { tripSyncManager } from '../sync/tripSyncManager' import { reopenForUser, deleteCurrentUserDb } from '../db/offlineDb' import { setAuthed } from '../sync/authGate' -import { unregisterSyncTriggers } from '../sync/syncTriggers' +import { registerSyncTriggers, unregisterSyncTriggers } from '../sync/syncTriggers' import { useSystemNoticeStore } from './systemNoticeStore.js' import { clearAppearanceSnapshot } from '../theme/applyAppearance' import { clearAllPluginSessions } from './pluginStore' @@ -95,6 +95,10 @@ async function onAuthSuccess(userId: number): Promise { } catch (err) { console.error('[auth] failed to open user-scoped offline DB', err) } + // logout() tears the triggers down, and App's mount effect never runs again in + // an SPA session, so a second login in the same tab would leave the mutation + // queue without a flush trigger. Re-registering is a no-op while they are up. + registerSyncTriggers() } export const useAuthStore = create()( diff --git a/client/src/store/slices/dayNotesSlice.ts b/client/src/store/slices/dayNotesSlice.ts index 1ca9578cd..7d795acc3 100644 --- a/client/src/store/slices/dayNotesSlice.ts +++ b/client/src/store/slices/dayNotesSlice.ts @@ -112,7 +112,9 @@ export const createDayNotesSlice = (set: SetState, get: GetState): DayNotesSlice })) try { - await dayNotesApi.delete(tripId, fromDayId, noteId) + // There is no atomic move on the server, so the destructive half goes last: + // if the create were second and failed, the note would already be gone for + // good and the rollback below would only fake it back into the store. // Every field the note carries, not just the ones it had when this was // written: a move is a delete plus a create, so anything omitted here is // silently dropped — which is how a coloured note lost its colour on the @@ -120,6 +122,13 @@ export const createDayNotesSlice = (set: SetState, get: GetState): DayNotesSlice const result = await dayNotesApi.create(tripId, toDayId, { text: note.text, time: note.time, icon: note.icon, color: note.color ?? null, sort_order, }) + try { + await dayNotesApi.delete(tripId, fromDayId, noteId) + } catch (delErr: unknown) { + // The source survived, so drop the copy rather than leave a duplicate behind. + await dayNotesApi.delete(tripId, toDayId, result.note.id).catch(() => {}) + throw delErr + } set(s => ({ dayNotes: { ...s.dayNotes, diff --git a/client/src/store/slices/placesSlice.test.ts b/client/src/store/slices/placesSlice.test.ts index a3c1b0687..6ab305dea 100644 --- a/client/src/store/slices/placesSlice.test.ts +++ b/client/src/store/slices/placesSlice.test.ts @@ -28,6 +28,11 @@ describe('placesSlice', () => { const assignment = buildAssignment({ id: 100, day_id: 3, + // The override is what makes the embedded copy differ from the pool row: + // the server projects COALESCE(assignment_time, place_time), so without + // it these two could not disagree in the first place. + assignment_time: '09:00', + assignment_end_time: '10:30', place: { ...place, place_time: '09:00', end_time: '10:30' }, }); seedStore(useTripStore, { places: [place], assignments: { '3': [assignment] } }); @@ -43,7 +48,7 @@ describe('placesSlice', () => { expect(useTripStore.getState().places[0].image_url).toBe('/uploads/places/pic.jpg'); const embedded = useTripStore.getState().assignments['3'][0].place; expect(embedded.image_url).toBe('/uploads/places/pic.jpg'); - // The assignment owns its own times — the fresh place must not overwrite them. + // The assignment owns these times, so the fresh place must not overwrite them. expect(embedded.place_time).toBe('09:00'); expect(embedded.end_time).toBe('10:30'); }); diff --git a/client/src/store/slices/placesSlice.ts b/client/src/store/slices/placesSlice.ts index fe7069883..64fdf9e44 100644 --- a/client/src/store/slices/placesSlice.ts +++ b/client/src/store/slices/placesSlice.ts @@ -19,6 +19,32 @@ export interface PlacesSlice { updatePlacesMany: (tripId: number | string, placeIds: number[], patch: Partial) => Promise } +/** + * Swap the pool place embedded in an assignment, resolving its times the way + * the server does. + * + * The embedded copy is a projection, not the pool row: assignments.service + * selects COALESCE(da.assignment_time, p.place_time). So a per-stop override + * wins and has to survive a pool edit, and where there is no override the pool + * time is what the day card shows - which means a pool edit must reach it. + * Pinning the old embedded times instead would keep a renamed place's schedule + * frozen until the next reload. + * + * Exported because the WS applier in remoteEventHandler has to merge + * identically; two spellings of this rule is how the two paths drifted apart in + * the first place. + */ +export function mergeAssignmentPlace(a: Assignment, place: Place): Assignment { + return { + ...a, + place: { + ...place, + place_time: a.assignment_time ?? place.place_time, + end_time: a.assignment_end_time ?? place.end_time, + }, + } +} + /** Replace a place in the pool and in every day-assignment that embeds it, * preserving the assignment's own times (mirrors updatePlace's reconciliation). */ function applyUpdatedPlace(set: SetState, placeId: number, place: Place): void { @@ -28,7 +54,7 @@ function applyUpdatedPlace(set: SetState, placeId: number, place: Place): void { for (const [dayId, items] of Object.entries(state.assignments)) { if (items.some((a: Assignment) => a.place?.id === placeId)) { updatedAssignments[dayId] = items.map((a: Assignment) => - a.place?.id === placeId ? { ...a, place: { ...place, place_time: a.place.place_time, end_time: a.place.end_time } } : a + a.place?.id === placeId ? mergeAssignmentPlace(a, place) : a ) changed = true } diff --git a/client/src/store/slices/remoteEventHandler.ts b/client/src/store/slices/remoteEventHandler.ts index 1615ce179..d7877ee61 100644 --- a/client/src/store/slices/remoteEventHandler.ts +++ b/client/src/store/slices/remoteEventHandler.ts @@ -4,6 +4,7 @@ import type { TripStoreState } from '../tripStore' import type { Assignment, Place, Day, DayNote, PackingItem, TodoItem, BudgetItem, BudgetItemMember, Reservation, Trip, TripFile, WebSocketEvent } from '../../types' import { offlineDb } from '../../db/offlineDb' import { useAuthStore } from '../authStore' +import { mergeAssignmentPlace } from './placesSlice' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -212,7 +213,7 @@ export const STATE_APPLIERS: Partial> assignments: Object.fromEntries( Object.entries(state.assignments).map(([dayId, items]) => [ dayId, - items.map(a => a.place?.id === (payload.place as Place).id ? { ...a, place: payload.place as Place } : a) + items.map(a => a.place?.id === (payload.place as Place).id ? mergeAssignmentPlace(a, payload.place as Place) : a) ]) ), }), diff --git a/client/src/sync/mutationQueue.ts b/client/src/sync/mutationQueue.ts index 43112821b..1c21ab487 100644 --- a/client/src/sync/mutationQueue.ts +++ b/client/src/sync/mutationQueue.ts @@ -39,6 +39,13 @@ export function generateUUID(): string { } let _flushing = false +// A row sits on 'syncing' only while its request is in flight, and the shared +// axios instance times out at 8s. Anything still 'syncing' a minute later +// belongs to a flush that never reached its catch — the tab was killed, the PWA +// was evicted, the device slept mid-request. flush() only ever selects +// 'pending', so without recovery that row is stranded: counted in the badge, +// never replayed, and blocking every dependent behind it. +const STUCK_SYNCING_MS = 60_000 // Monotonically increasing timestamp so same-millisecond enqueues // still get a deterministic FIFO order when sorted by createdAt. let _lastTs = 0 @@ -121,14 +128,31 @@ export const mutationQueue = { // without its base token, so one more pass overwrites the server cleanly. let needsRetry = false try { + // Reclaim what an interrupted flush left behind before picking up work. + // Replaying is safe: the mutation id doubles as the X-Idempotency-Key, so + // a write the server already applied is answered from its replay cache. + const stuckBefore = Date.now() - STUCK_SYNCING_MS + await offlineDb.mutationQueue + .where('status') + .equals('syncing') + .filter(m => (m.syncingSince ?? 0) < stuckBefore) + .modify(m => { m.status = 'pending'; m.syncingSince = undefined }) + const pending = await offlineDb.mutationQueue .where('status') .equals('pending') .sortBy('createdAt') for (const mutation of pending) { - // Mark as syncing so UI can show progress - await offlineDb.mutationQueue.update(mutation.id, { status: 'syncing' }) + // Re-checked every pass, not just on entry: this loop writes server + // responses straight into Dexie, and after a logout the proxy points at + // the shared anonymous database. A flush that started before logout + // would otherwise seed it with the previous account's rows. + if (!isAuthed()) break + + // Mark as syncing so UI can show progress. The stamp is what lets the + // next flush tell an in-flight row from one a killed tab abandoned. + await offlineDb.mutationQueue.update(mutation.id, { status: 'syncing', syncingSince: Date.now() }) // Resolve a temp-id reference now that earlier CREATEs in this flush // may have completed (FIFO order guarantees the CREATE ran first). diff --git a/client/src/sync/tripSyncManager.ts b/client/src/sync/tripSyncManager.ts index 0d5b3a15a..64de8af8b 100644 --- a/client/src/sync/tripSyncManager.ts +++ b/client/src/sync/tripSyncManager.ts @@ -52,8 +52,15 @@ interface TripBundle { // ── Helpers ─────────────────────────────────────────────────────────────────── +// Trip dates are plain local calendar dates, so format ours the same way. +// toISOString() would hand back the UTC day and drop an ongoing trip a day +// early west of UTC (and keep a finished one a day too long east of it). +function ymd(d: Date): string { + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}` +} + function todayStr(): string { - return new Date().toISOString().slice(0, 10) + return ymd(new Date()) } /** @@ -76,7 +83,7 @@ function isStale(trip: Trip): boolean { if (!trip.end_date) return false const cutoff = new Date() cutoff.setDate(cutoff.getDate() - 7) - return trip.end_date < cutoff.toISOString().slice(0, 10) + return trip.end_date < ymd(cutoff) } function isPhoto(file: TripFile): boolean { @@ -123,6 +130,9 @@ async function cacheFilesForTrip(tripId: number, files: TripFile[]): Promise upsertTags(d.tags)).catch(() => {}) - categoriesApi.list().then(d => upsertCategories(d.categories)).catch(() => {}) + // Cache global user data (tags + categories) — fire-and-forget, so the + // gate has to be re-read when the response lands, not when it was issued: + // a logout in between would put these rows in the anonymous database. + tagsApi.list().then(d => { if (isAuthed()) upsertTags(d.tags) }).catch(() => {}) + categoriesApi.list().then(d => { if (isAuthed()) upsertCategories(d.categories) }).catch(() => {}) // Cache file blobs + map tiles in background (don't block syncAll) const cacheTiles = getOfflinePrefs().cacheTiles const tileUrl = useSettingsStore.getState().settings.map_tile_url || undefined for (const trip of toSync) { + if (!isAuthed()) return const files = await offlineDb.tripFiles.where('trip_id').equals(trip.id).toArray() cacheFilesForTrip(trip.id, files).catch(console.error) } @@ -246,6 +262,7 @@ export const tripSyncManager = { // 1) Trip bundles (structured data). let i = 0 for (const trip of toSync) { + if (!isAuthed()) return 0 onProgress?.({ phase: 'trips', current: ++i, total, label: trip.title }) try { await syncTrip(trip.id) @@ -263,6 +280,7 @@ export const tripSyncManager = { // 2) File blobs — awaited so "prepared" really means downloaded. i = 0 for (const trip of toSync) { + if (!isAuthed()) return 0 onProgress?.({ phase: 'files', current: ++i, total, label: trip.title }) const files = await offlineDb.tripFiles.where('trip_id').equals(trip.id).toArray() await cacheFilesForTrip(trip.id, files).catch(console.error) @@ -273,6 +291,7 @@ export const tripSyncManager = { const tileUrl = useSettingsStore.getState().settings.map_tile_url || undefined i = 0 for (const trip of toSync) { + if (!isAuthed()) return 0 onProgress?.({ phase: 'tiles', current: ++i, total, label: trip.title }) const places = await offlineDb.places.where('trip_id').equals(trip.id).toArray() await prefetchTilesForTrip(trip.id, places, tileUrl, true).catch(console.error) diff --git a/client/src/utils/clipboard.test.ts b/client/src/utils/clipboard.test.ts new file mode 100644 index 000000000..33af293f5 --- /dev/null +++ b/client/src/utils/clipboard.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect, vi, afterEach } from 'vitest' +import { copyText } from './clipboard' + +function setContext(clipboard: unknown, secure: boolean) { + Object.defineProperty(navigator, 'clipboard', { value: clipboard, configurable: true, writable: true }) + Object.defineProperty(window, 'isSecureContext', { value: secure, configurable: true, writable: true }) +} + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('copyText', () => { + it('FE-UTIL-CLIP-001: uses the async clipboard API in a secure context', async () => { + const writeText = vi.fn(async () => {}) + setContext({ writeText }, true) + + expect(await copyText('https://trip.example/s/abc')).toBe(true) + expect(writeText).toHaveBeenCalledWith('https://trip.example/s/abc') + }) + + // The case every plain-HTTP self-host is in: navigator.clipboard is not there + // at all, so reading .writeText off it throws before anything is copied. + it('FE-UTIL-CLIP-002: falls back to execCommand when there is no clipboard API', async () => { + setContext(undefined, false) + const execCommand = vi.fn(() => true) + Object.defineProperty(document, 'execCommand', { value: execCommand, configurable: true, writable: true }) + + expect(await copyText('https://trip.example/s/abc')).toBe(true) + expect(execCommand).toHaveBeenCalledWith('copy') + expect(document.querySelector('textarea')).toBeNull() + }) + + it('FE-UTIL-CLIP-003: falls back when the page is served over plain HTTP but the API object exists', async () => { + const writeText = vi.fn(async () => {}) + setContext({ writeText }, false) + const execCommand = vi.fn(() => true) + Object.defineProperty(document, 'execCommand', { value: execCommand, configurable: true, writable: true }) + + expect(await copyText('x')).toBe(true) + expect(writeText).not.toHaveBeenCalled() + expect(execCommand).toHaveBeenCalledWith('copy') + }) + + it('FE-UTIL-CLIP-004: reports a refused clipboard write instead of throwing', async () => { + setContext({ writeText: vi.fn().mockRejectedValue(new Error('denied')) }, true) + + expect(await copyText('x')).toBe(false) + }) + + // A leftover textarea would steal focus and show up in the next query. + it('FE-UTIL-CLIP-005: removes the temporary textarea even when execCommand throws', async () => { + setContext(undefined, false) + Object.defineProperty(document, 'execCommand', { + value: () => { throw new Error('not allowed') }, + configurable: true, + writable: true, + }) + + expect(await copyText('x')).toBe(false) + expect(document.querySelector('textarea')).toBeNull() + }) +}) diff --git a/client/src/utils/clipboard.ts b/client/src/utils/clipboard.ts new file mode 100644 index 000000000..bb4eab55d --- /dev/null +++ b/client/src/utils/clipboard.ts @@ -0,0 +1,35 @@ +/** + * Copying a link to the clipboard. + * + * `navigator.clipboard` is gated on a secure context, so on a plain-HTTP + * self-host — which is how a good part of TREK is deployed — it is simply + * undefined and reading `.writeText` off it throws. Every copy button has to + * carry the deprecated textarea + execCommand path as a fallback, so it lives + * here once instead of in each of them. + */ +export async function copyText(text: string): Promise { + try { + if (navigator.clipboard && window.isSecureContext) { + await navigator.clipboard.writeText(text) + return true + } + const ta = document.createElement('textarea') + ta.value = text + ta.style.position = 'fixed' + ta.style.left = '-9999px' + document.body.appendChild(ta) + try { + ta.focus() + ta.select() + document.execCommand('copy') + } finally { + document.body.removeChild(ta) + } + // execCommand reports false for a refused copy in some browsers and true in + // others that did nothing, so it is not worth reading: a true here means + // nothing objected, not that the clipboard definitely changed. + return true + } catch { + return false + } +} diff --git a/client/src/utils/safeUrl.ts b/client/src/utils/safeUrl.ts index a89390fa1..b6a21bef8 100644 --- a/client/src/utils/safeUrl.ts +++ b/client/src/utils/safeUrl.ts @@ -16,3 +16,35 @@ import { placeWebsiteSchema } from '@trek/shared' export function safeHttpUrl(value: string | null | undefined): string | null { return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null } + +/** + * A user-pasted link, made safe for an href without narrowing what people are + * allowed to paste. + * + * Different job from safeHttpUrl above. That one guards fields whose contract + * really is http(s)-only and capped. A booking url is deliberately free-form - + * shared's reservationUrlSchema says so in as many words, because people paste + * bare hosts and very long provider deep links - so holding it to the stricter + * contract would turn a link somebody saved last year into dead text. + * + * Only the schemes that execute in this origin are refused. A value with no + * scheme gets https://, which is what an address bar does with the same input; + * without it the browser would resolve "www.hotel.com" against the current page + * and navigate inside the app. + */ +export function safeExternalHref(value: string | null | undefined): string | null { + if (typeof value !== 'string') return null + const trimmed = value.trim() + if (!trimmed) return null + // Browsers strip control characters before they resolve a scheme, so the test + // has to run against the same string they will see. + const collapsed = Array.from(trimmed).filter(c => c > ' ').join('') + if (/^https?:\/\//i.test(collapsed)) return trimmed + // Any other scheme, executing or merely unexpected, stays unlinked. + if (/^[a-z][a-z0-9+.-]*:/i.test(collapsed)) return null + // A bare host gets https://, the way an address bar would take it. Free text + // does not: "www.hotel.com" is a site somebody meant to link, "call the hotel" + // is a note, and turning the second into a link is worse than leaving it flat. + if (/^[^\s/?#]+\.[^\s/?#]+/.test(trimmed)) return `https://${trimmed}` + return null +} diff --git a/client/src/utils/uploadQueue.test.ts b/client/src/utils/uploadQueue.test.ts index bbc81a465..6390f0dab 100644 --- a/client/src/utils/uploadQueue.test.ts +++ b/client/src/utils/uploadQueue.test.ts @@ -89,6 +89,22 @@ describe('uploadFilesResilient', () => { expect(res.failed).toEqual([f]) }) + it('FE-W4UPQ-007b: retries a 429 rather than dropping the file', async () => { + let calls = 0 + const uploadOne = vi.fn(async (f: File) => { + calls++ + // A proxy in front of a self-hosted instance rate-limits the batch. + if (calls === 1) throw { response: { status: 429 } } + return [f.name] + }) + + const res = await uploadFilesResilient([file('a.txt')], uploadOne, { retries: 1 }) + + expect(calls).toBe(2) + expect(res.succeeded).toEqual(['a.txt']) + expect(res.failed).toEqual([]) + }) + it('FE-W4UPQ-008: retries a 5xx up to the limit then gives up', async () => { const uploadOne = vi.fn(async () => { throw { response: { status: 502 } } }) const f = file('a.txt') diff --git a/client/src/utils/uploadQueue.ts b/client/src/utils/uploadQueue.ts index de2f92b8e..721cc2d77 100644 --- a/client/src/utils/uploadQueue.ts +++ b/client/src/utils/uploadQueue.ts @@ -22,6 +22,10 @@ const sleep = (ms: number) => new Promise(r => setTimeout(r, ms)) function isRetryable(err: unknown): boolean { if (err && typeof err === 'object' && 'response' in err) { const status = (err as { response?: { status?: number } }).response?.status + // 408/425/429: timeout / too-early / rate-limited. TREK's own upload routes + // do not send them, but a proxy in front of a self-hosted instance does, and + // dropping the file for one of those loses a photo we could have re-sent. + if (status === 408 || status === 425 || status === 429) return true if (status !== undefined && status >= 400 && status < 500) return false } return true diff --git a/client/tests/helpers/msw/handlers/external.ts b/client/tests/helpers/msw/handlers/external.ts index 183389c21..8cd9c922f 100644 --- a/client/tests/helpers/msw/handlers/external.ts +++ b/client/tests/helpers/msw/handlers/external.ts @@ -3,12 +3,11 @@ import { http, HttpResponse } from 'msw'; /** * Third-party endpoints the app calls directly, rather than through /api. * - * Without these the request leaves the runner: MSW is started with - * `onUnhandledRequest: 'warn'`, so an unmatched call is a warning and a live - * network round trip, not a failure. That is how the mobile FX widget reached - * api.frankfurter.dev from CI and settled its promise after the test - * environment was gone, surfacing as `window is not defined` attributed to - * whichever case happened to be running. + * Without these the call is refused: tests/setup.ts errors on an unhandled request to + * another origin. It used to warn and then perform the request, which is how the mobile + * FX widget reached api.frankfurter.dev from CI and settled its promise after the test + * environment was gone, surfacing as `window is not defined` attributed to whichever + * case happened to be running. Add the endpoint here rather than letting it out. * * An empty rate list is the shape the widget already handles: it seeds the * base's own self-rate and renders with nothing else selectable. diff --git a/client/tests/integration/api/client.test.ts b/client/tests/integration/api/client.test.ts index 0a6ecd05c..1c221b9b2 100644 --- a/client/tests/integration/api/client.test.ts +++ b/client/tests/integration/api/client.test.ts @@ -360,6 +360,11 @@ describe('API client interceptors', () => { await expect(backupApi.download('backup.zip')).resolves.toBeUndefined(); expect(createObjectURL).toHaveBeenCalled(); + expect(clickSpy).toHaveBeenCalled(); + // The shared helper appends the anchor (Firefox ignores a click on a + // detached one) and revokes the object URL a tick later, not in-line. + expect(revokeObjectURL).not.toHaveBeenCalled(); + await new Promise(r => setTimeout(r, 150)); expect(revokeObjectURL).toHaveBeenCalled(); vi.restoreAllMocks(); diff --git a/client/tests/integration/hooks/useRouteCalculation.test.ts b/client/tests/integration/hooks/useRouteCalculation.test.ts index a8288a383..cd2c6f8b3 100644 --- a/client/tests/integration/hooks/useRouteCalculation.test.ts +++ b/client/tests/integration/hooks/useRouteCalculation.test.ts @@ -117,6 +117,48 @@ describe('useRouteCalculation', () => { expect(result.current.routeSegments).toEqual(MOCK_SEGMENTS.map(s => ({ ...s, mode: 'driving' }))); }); + it('FE-HOOK-ROUTE-023: consecutive legs of one mode go out as a single multi-waypoint request', async () => { + // Three places routed in the day default and a fourth reached on foot: two + // requests, not three, and the connectors stay one per pair in order. + (calculateRouteWithLegs as ReturnType).mockImplementation( + (waypoints: { lat: number; lng: number }[]) => Promise.resolve({ + coordinates: [] as [number, number][], + distance: 0, + duration: 0, + legs: waypoints.slice(0, -1).map((w, i) => ({ + ...MOCK_SEGMENTS[0], + from: [w.lat, w.lng] as [number, number], + to: [waypoints[i + 1].lat, waypoints[i + 1].lng] as [number, number], + })), + }), + ); + const pts = [ + { lat: 48.86, lng: 2.35 }, { lat: 48.87, lng: 2.36 }, + { lat: 48.88, lng: 2.37 }, { lat: 48.89, lng: 2.38 }, + ]; + const assignments = pts.map((pt, i) => ({ + ...buildAssignment({ day_id: 5, order_index: i, place: buildPlace(pt) }), + // The third place walks to the fourth; the first two use the day default. + leg_transport_mode: i === 2 ? 'walking' : null, + })); + const store = buildMockStore({ '5': assignments as never }); + + const { result } = renderHook(() => useRouteCalculation(store as TripStoreState, 5)); + await act(async () => {}); + + const calls = (calculateRouteWithLegs as ReturnType).mock.calls; + expect(calls).toHaveLength(2); + expect(calls[0][0]).toEqual(pts.slice(0, 3)); + expect(calls[0][1].profile).toBe('driving'); + expect(calls[1][0]).toEqual(pts.slice(2)); + expect(calls[1][1].profile).toBe('walking'); + // One segment per pair, in the order they are walked. + expect(result.current.routeSegments.map(s => s.mode)).toEqual(['driving', 'driving', 'walking']); + expect(result.current.routeSegments.map(s => s.from)).toEqual([ + [pts[0].lat, pts[0].lng], [pts[1].lat, pts[1].lng], [pts[2].lat, pts[2].lng], + ]); + }); + it('FE-HOOK-ROUTE-006: assignments are sorted by order_index before extracting waypoints', async () => { const p1 = buildPlace({ lat: 10, lng: 10 }); const p2 = buildPlace({ lat: 20, lng: 20 }); diff --git a/client/tests/setup.ts b/client/tests/setup.ts index dcbe38674..d0375bea4 100644 --- a/client/tests/setup.ts +++ b/client/tests/setup.ts @@ -21,8 +21,16 @@ vi.mock('../src/api/websocket', () => ({ removeListener: vi.fn(), })); -// MSW lifecycle -beforeAll(() => server.listen({ onUnhandledRequest: 'warn' })); +// MSW lifecycle. A cross-origin request nobody mocked is the dangerous kind: 'warn' +// lets it through, so the runner really talks to frankfurter or a tile server and settles +// a promise after the test environment is gone (see handlers/external.ts). Those fail now. +// An unhandled same-origin call stays a warning: that is a missing handler, not egress. +beforeAll(() => server.listen({ + onUnhandledRequest: (request, print) => { + if (new URL(request.url).origin === location.origin) print.warning(); + else print.error(); + }, +})); afterEach(() => { server.resetHandlers(); cleanup(); diff --git a/client/tests/unit/i18n/TranslationContext.test.tsx b/client/tests/unit/i18n/TranslationContext.test.tsx index 1887ca623..a70980462 100644 --- a/client/tests/unit/i18n/TranslationContext.test.tsx +++ b/client/tests/unit/i18n/TranslationContext.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-I18NCTX-001 to FE-COMP-I18NCTX-017 +// FE-COMP-I18NCTX-001 to FE-COMP-I18NCTX-017 (plus -003b / -013b) import { act, render, screen, waitFor } from '@testing-library/react'; import { SUPPORTED_LANGUAGES, TranslationProvider, useTranslation } from '../../../src/i18n/TranslationContext'; import { TransHtml } from '../../../src/i18n/TransHtml'; @@ -51,6 +51,17 @@ describe('TranslationProvider', () => { expect(screen.getByTestId('value').textContent).toContain('5'); }); + it('FE-COMP-I18NCTX-003b: inserts a value containing $ patterns literally', () => { + render( + + + , + ); + // A trip really can be named this, and `$&` as a replacement string would + // paste the matched placeholder back in. + expect(screen.getByTestId('value').textContent).toContain('A$&B $1 $`'); + }); + it('FE-COMP-I18NCTX-004: defaults to English with the matching locale', () => { render(); expect(screen.getByTestId('language').textContent).toBe('en'); @@ -149,6 +160,16 @@ describe('tHtml', () => { expect(raw).toContain('<img'); }); + it('FE-COMP-I18NCTX-013b: keeps $ patterns in an escaped value literal', () => { + render( + + + , + ); + + expect(screen.getByTestId('raw').textContent).toContain('A$&B'); + }); + it('FE-COMP-I18NCTX-014: returns the key unchanged when it is unknown', () => { render(); expect(screen.getByTestId('raw').textContent).toBe('nope.nope'); diff --git a/client/tests/unit/mobile/admin/MAdmin.test.tsx b/client/tests/unit/mobile/admin/MAdmin.test.tsx index 5a45f7f9d..74defb4c4 100644 --- a/client/tests/unit/mobile/admin/MAdmin.test.tsx +++ b/client/tests/unit/mobile/admin/MAdmin.test.tsx @@ -77,6 +77,8 @@ vi.mock('../../../../src/mobile/screens/admin/MAdminAddonManager', () => ({ {String(collabFeatures.chat)} + {String(collabFeatures.notes)} +
), })); @@ -362,6 +364,34 @@ describe('MAdmin', () => { await waitFor(() => expect(screen.getByTestId('chat-state')).toHaveTextContent('true')); }); + it('FE-MOB-ADMIN-018: a failing toggle only rolls back its own key', async () => { + let releaseChat: () => void = () => {}; + server.use( + http.put('/api/admin/collab-features', async ({ request }) => { + const body = (await request.json()) as Record; + if ('chat' in body) { + await new Promise((resolve) => { releaseChat = resolve; }); + return HttpResponse.json({ error: 'boom' }, { status: 500 }); + } + return HttpResponse.json({ ...body }); + }), + ); + renderAdmin(); + await waitFor(() => expect(screen.getByText('alice')).toBeInTheDocument()); + await openSection('Addons'); + + await waitFor(() => expect(screen.getByTestId('chat-state')).toHaveTextContent('true')); + fireEvent.click(screen.getByRole('button', { name: 'toggle chat' })); + fireEvent.click(screen.getByRole('button', { name: 'toggle notes' })); + await waitFor(() => expect(screen.getByTestId('notes-state')).toHaveTextContent('false')); + + releaseChat(); + + // Chat comes back, but the notes toggle the admin made meanwhile survives. + await waitFor(() => expect(screen.getByTestId('chat-state')).toHaveTextContent('true')); + expect(screen.getByTestId('notes-state')).toHaveTextContent('false'); + }); + it('FE-MOB-ADMIN-017: the stats grid falls back to zero files', async () => { server.use( http.get('/api/admin/stats', () => diff --git a/client/tests/unit/mobile/dashboard/MNewTripSheet.test.tsx b/client/tests/unit/mobile/dashboard/MNewTripSheet.test.tsx index c73f6c2f3..441519042 100644 --- a/client/tests/unit/mobile/dashboard/MNewTripSheet.test.tsx +++ b/client/tests/unit/mobile/dashboard/MNewTripSheet.test.tsx @@ -44,6 +44,8 @@ vi.mock('../../../../src/components/shared/CustomSelect', () => ({ })); const origCreateObjectURL = URL.createObjectURL; +const origRevokeObjectURL = URL.revokeObjectURL; +const revokeSpy = vi.fn(); function buildDashTrip(over: Partial = {}): DashboardTrip { return { @@ -69,12 +71,15 @@ beforeEach(() => { return 1; }) as unknown as typeof window.__addToast; URL.createObjectURL = (() => 'blob:preview') as unknown as typeof URL.createObjectURL; + revokeSpy.mockClear(); + URL.revokeObjectURL = revokeSpy as unknown as typeof URL.revokeObjectURL; useAuthStore.setState({ isAuthenticated: true, user: buildUser({ id: 1, role: 'user' }) }); usePermissionsStore.setState({ permissions: {} }); }); afterEach(() => { URL.createObjectURL = origCreateObjectURL; + URL.revokeObjectURL = origRevokeObjectURL; delete window.__addToast; useSettingsStore.setState(s => ({ settings: { ...s.settings, default_currency: '' } })); vi.restoreAllMocks(); @@ -374,6 +379,30 @@ describe('MNewTripSheet', () => { expect(update).not.toHaveBeenCalled(); }); + // The preview holds the whole picked file in memory until the url is released. + it('FE-MOB-NTSH-034: a replaced or dropped file preview releases its blob url', async () => { + render( {}} onSave={() => {}} />); + + fireEvent.change(fileInput(), { target: { files: [png()] } }); + await waitFor(() => expect(screen.getByRole('button', { name: 'Delete' })).toBeInTheDocument()); + expect(revokeSpy).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })); + await waitFor(() => expect(revokeSpy).toHaveBeenCalledWith('blob:preview')); + }); + + it('FE-MOB-NTSH-035: a stored cover url is never handed to revokeObjectURL', async () => { + const { unmount } = render( + {}} onSave={() => {}} />, + ); + + await waitFor(() => + expect(document.querySelector('img[src="/uploads/covers/a.jpg"]')).toBeInTheDocument(), + ); + unmount(); + expect(revokeSpy).not.toHaveBeenCalled(); + }); + it('FE-MOB-NTSH-023: removing a stored cover clears it on the server', async () => { const update = vi.spyOn(tripsApi, 'update').mockResolvedValue({ trip: { id: 42 } as Trip }); const onCoverUpdate = vi.fn(); diff --git a/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx b/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx index 45a13a9db..50daf400c 100644 --- a/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx @@ -1,8 +1,8 @@ // FE-MOB-JDET-001 to FE-MOB-JDET-037 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { render, screen, fireEvent, waitFor } from '../../../helpers/render'; +import { render, screen, fireEvent, waitFor, act } from '../../../helpers/render'; import MJourneyDetail from '../../../../src/mobile/screens/journey/MJourneyDetail'; -import { addonsApi, journeyApi } from '../../../../src/api/client'; +import { addonsApi, journeyApi, memoriesApi } from '../../../../src/api/client'; import { useJourneyStore } from '../../../../src/store/journeyStore'; import { useAuthStore } from '../../../../src/store/authStore'; import type { GalleryPhoto, JourneyDetail, JourneyEntry } from '../../../../src/store/journeyStore'; @@ -198,6 +198,7 @@ beforeEach(() => { useJourneyStore.setState({ ...journeyStoreInitial, uploadGalleryPhotos, createEntry } as never, true); useAuthStore.setState({ user: { id: 5, username: 'maurice' } } as never); vi.spyOn(addonsApi, 'enabled').mockResolvedValue({ addons: [] }); + vi.spyOn(memoriesApi, 'status').mockResolvedValue({ connected: false }); vi.stubGlobal('fetch', vi.fn(async () => new Response('{}', { status: 200 }))); }); @@ -353,7 +354,7 @@ describe('MJourneyDetail', () => { { id: 'legacy', name: 'Legacy', type: 'other', enabled: true }, ], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: true }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: true }); setup({ view: 'gallery' }); fireEvent.click(await screen.findByRole('button', { name: 'common.upload' })); @@ -366,7 +367,7 @@ describe('MJourneyDetail', () => { vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: true }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: true }); const toGallery = vi.spyOn(journeyApi, 'addProviderPhotosToGallery').mockResolvedValue({ added: 3 }); const toEntry = vi.spyOn(journeyApi, 'addProviderPhotos').mockResolvedValue({ added: 1 }); const { hook } = setup({ view: 'gallery' }); @@ -389,7 +390,7 @@ describe('MJourneyDetail', () => { vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: true }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: true }); vi.spyOn(journeyApi, 'addProviderPhotosToGallery').mockRejectedValue(new Error('boom')); setup({ view: 'gallery' }); @@ -507,7 +508,7 @@ describe('MJourneyDetail', () => { vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: true }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: true }); const { container } = setup({ view: 'gallery' }); const input = container.ownerDocument.querySelector('input[type="file"]') as HTMLInputElement; const click = vi.spyOn(input, 'click').mockImplementation(() => {}); @@ -522,7 +523,7 @@ describe('MJourneyDetail', () => { vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: false }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: false }); const { container } = setup({ view: 'gallery' }); const input = container.ownerDocument.querySelector('input[type="file"]') as HTMLInputElement; const click = vi.spyOn(input, 'click').mockImplementation(() => {}); @@ -536,7 +537,7 @@ describe('MJourneyDetail', () => { vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }], }); - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ connected: true }), { status: 200 }))); + vi.mocked(memoriesApi.status).mockResolvedValue({ connected: true }); const gallery = [buildGalleryPhoto({ id: 601, asset_id: 'asset-1' }), buildGalleryPhoto({ id: 602, asset_id: null })]; const { hook } = setup({ view: 'gallery', @@ -629,6 +630,27 @@ describe('MJourneyDetail', () => { expect(hook.updateEntry).toHaveBeenCalledWith(77, { title: 'Fresh' }); }); + // Leaving the screen mid-probe used to leave the requests running and land a + // setState on a gone component. The probes are sequential, so the queue has to + // stop as well instead of walking the rest of the providers. + it('FE-MOB-JDET-038: leaving the screen stops the provider probes', async () => { + vi.mocked(addonsApi.enabled).mockResolvedValue({ + addons: [ + { id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }, + { id: 'synology', name: 'Synology', type: 'photo_provider', enabled: true }, + ], + }); + let release: (value: { connected: boolean }) => void = () => {}; + vi.mocked(memoriesApi.status).mockReturnValue(new Promise(resolve => { release = resolve; })); + const { unmount } = setup({ view: 'gallery' }); + + await waitFor(() => expect(memoriesApi.status).toHaveBeenCalledTimes(1)); + + unmount(); + await act(async () => { release({ connected: true }); }); + expect(memoriesApi.status).toHaveBeenCalledTimes(1); + }); + it('FE-MOB-JDET-037: the screen measures itself, not the shell', () => { // #1809: the shell scrolls with the document now and hands down no definite // height. A map on a percentage of an auto-height parent collapses to zero. diff --git a/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx b/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx index 002f4384b..361fde069 100644 --- a/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx @@ -1,3 +1,4 @@ +import { StrictMode } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { http, HttpResponse } from 'msw'; import userEvent from '@testing-library/user-event'; @@ -244,6 +245,7 @@ interface MountOptions { withDelete?: boolean; withProviderHook?: boolean; upload?: UploadFn; + strict?: boolean; } const defaultUpload: UploadFn = async (_entryId, files, cbs) => { @@ -258,7 +260,7 @@ function mountSheet(sheetEntry: JourneyEntry, opts: MountOptions = {}) { const onSave = vi.fn(async (_data: Record, _existingEntryId?: number) => 42); const onUploadPhotos = vi.fn(opts.upload ?? defaultUpload); const onAddProviderPhotos = vi.fn(async (_entryId: number, _group: Record) => {}); - const view = render( + const sheet = ( ); + // main.tsx runs the app inside StrictMode, so a handler that is not pure shows + // its double-invoke here and nowhere else. + const view = render(opts.strict ? {sheet} : sheet); return { ...view, onClose, onDone, onDelete, onSave, onUploadPhotos, onAddProviderPhotos }; } @@ -283,14 +288,20 @@ const timeField = () => document.querySelector('input[type="time"]') as HTMLInpu const jpeg = (name = 'a.jpg') => new File(['x'], name, { type: 'image/jpeg' }); const originalCreateObjectURL = URL.createObjectURL; +const originalRevokeObjectURL = URL.revokeObjectURL; +const revokeSpy = vi.fn(); describe('MJourneyEntrySheet full editor', () => { beforeEach(() => { toastSpy.mockClear(); + revokeSpy.mockClear(); window.__addToast = toastSpy; Object.defineProperty(URL, 'createObjectURL', { configurable: true, writable: true, value: vi.fn(() => 'blob:preview'), }); + Object.defineProperty(URL, 'revokeObjectURL', { + configurable: true, writable: true, value: revokeSpy, + }); }); afterEach(() => { @@ -298,6 +309,9 @@ describe('MJourneyEntrySheet full editor', () => { Object.defineProperty(URL, 'createObjectURL', { configurable: true, writable: true, value: originalCreateObjectURL, }); + Object.defineProperty(URL, 'revokeObjectURL', { + configurable: true, writable: true, value: originalRevokeObjectURL, + }); }); it('FE-MOB-JENTRY-003: shows every editor section for a brand new entry', () => { @@ -530,6 +544,22 @@ describe('MJourneyEntrySheet full editor', () => { expect(document.querySelector('img[src="blob:preview"]')).not.toBeInTheDocument(); }); + it('FE-MOB-JENTRY-048: mints one preview url per queued file and releases it on unmount', async () => { + const { unmount } = mountSheet(buildEntry()); + + fireEvent.change(multiFileInput(), { target: { files: [jpeg()] } }); + await waitFor(() => expect(document.querySelector('img[src="blob:preview"]')).toBeInTheDocument()); + expect(URL.createObjectURL).toHaveBeenCalledTimes(1); + + // Typing re-renders the tile; the url must not be minted again. + fireEvent.change(screen.getByPlaceholderText('Give this moment a name...'), { target: { value: 'Ferry' } }); + fireEvent.change(screen.getByPlaceholderText('Give this moment a name...'), { target: { value: 'Ferry day' } }); + expect(URL.createObjectURL).toHaveBeenCalledTimes(1); + + unmount(); + expect(revokeSpy).toHaveBeenCalledWith('blob:preview'); + }); + it('FE-MOB-JENTRY-015: queues gallery photos on a new entry and links them after the save', async () => { const linked: unknown[] = []; server.use(http.post('/api/journeys/entries/42/link-photo', async ({ request }) => { @@ -639,6 +669,41 @@ describe('MJourneyEntrySheet full editor', () => { expect(order[0]).toBe('/api/photos/101/thumbnail'); }); + it('FE-MOB-JENTRY-049: promoting a photo under StrictMode still patches each photo once', async () => { + const patched: Array<{ id: string; body: unknown }> = []; + server.use(http.patch('/api/journeys/photos/:id', async ({ params, request }) => { + patched.push({ id: String(params.id), body: await request.json() }); + return HttpResponse.json({ ok: true }); + })); + const user = userEvent.setup(); + mountSheet(buildEntry({ id: 5, photos: [buildPhoto(100), buildPhoto(101)] }), { strict: true }); + + await user.click(screen.getByRole('button', { name: '1st' })); + + await waitFor(() => expect(patched.length).toBeGreaterThanOrEqual(2)); + // Let a doubled batch land before counting, otherwise the extra PATCHes slip in + // after the assertion. + await act(async () => { await new Promise(r => setTimeout(r, 20)); }); + expect(patched.map(p => p.id)).toEqual(['101', '100']); + }); + + it('FE-MOB-JENTRY-050: a refused reorder snaps the strip back and says so', async () => { + let attempts = 0; + server.use(http.patch('/api/journeys/photos/:id', () => { + attempts += 1; + return HttpResponse.json({ error: 'sort rejected' }, { status: 500 }); + })); + const user = userEvent.setup(); + mountSheet(buildEntry({ id: 5, photos: [buildPhoto(100), buildPhoto(101)] })); + + await user.click(screen.getByRole('button', { name: '1st' })); + + await waitFor(() => expect(attempts).toBe(2)); + await waitFor(() => expect(toastSpy).toHaveBeenCalledWith('sort rejected', 'error', undefined)); + const order = Array.from(document.querySelectorAll('.h-16 img')).map(i => i.getAttribute('src')); + expect(order).toEqual(['/api/photos/100/thumbnail', '/api/photos/101/thumbnail']); + }); + it('FE-MOB-JENTRY-021: toggles mood and weather chips back off', async () => { const user = userEvent.setup(); const { onSave } = mountSheet(buildEntry({ id: 5, mood: 'rough', weather: 'cold' })); @@ -735,18 +800,21 @@ describe('MJourneyEntrySheet full editor', () => { expect(screen.queryByText('All photos already added')).not.toBeInTheDocument(); }); - it('FE-MOB-JENTRY-037: reorders photos locally even when persisting the order fails', async () => { - let attempts = 0; - server.use(http.patch('/api/journeys/photos/:id', () => { - attempts += 1; - return new HttpResponse(null, { status: 500 }); + it('FE-MOB-JENTRY-037: a partly accepted order keeps what the server took', async () => { + const patched: number[] = []; + server.use(http.patch('/api/journeys/photos/:id', ({ params }) => { + const id = Number(params.id); + patched.push(id); + if (id === 100) return new HttpResponse(null, { status: 500 }); + return HttpResponse.json({ ok: true }); })); const user = userEvent.setup(); mountSheet(buildEntry({ id: 5, photos: [buildPhoto(100), buildPhoto(101)] })); await user.click(screen.getByRole('button', { name: '1st' })); - await waitFor(() => expect(attempts).toBe(2)); + await waitFor(() => expect(patched).toEqual([101, 100])); + // 101 is first on the server now; snapping the strip back would hide that. const order = Array.from(document.querySelectorAll('.h-16 img')).map(i => i.getAttribute('src')); expect(order).toEqual(['/api/photos/101/thumbnail', '/api/photos/100/thumbnail']); }); diff --git a/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx b/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx index cd45158f6..e89815eb4 100644 --- a/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneySettingsSheet.test.tsx @@ -267,7 +267,8 @@ describe('MJourneySettingsSheet', () => { }); const create = vi.spyOn(journeyApi, 'createShareLink').mockResolvedValue({ token: 'tok-9' }); const writeText = vi.fn(async () => {}); - Object.defineProperty(navigator, 'clipboard', { configurable: true, value: { writeText } }); + Object.defineProperty(navigator, 'clipboard', { configurable: true, writable: true, value: { writeText } }); + Object.defineProperty(window, 'isSecureContext', { configurable: true, writable: true, value: true }); setup(); expect(await screen.findByText(`${window.location.origin}/public/journey/tok-9`)).toBeInTheDocument(); @@ -370,6 +371,26 @@ describe('MJourneySettingsSheet', () => { expect(props.onClose).toHaveBeenCalled(); }); + // A self-hosted install served over plain HTTP has no navigator.clipboard, and the + // unguarded call used to throw before the button ever showed feedback. + it('FE-MOB-JSET-031: copies through execCommand when the clipboard API is unavailable', async () => { + vi.mocked(journeyApi.getShareLink).mockResolvedValue({ + link: { token: 'tok-9', share_timeline: true, share_gallery: true, share_map: false }, + }); + Object.defineProperty(navigator, 'clipboard', { configurable: true, writable: true, value: undefined }); + Object.defineProperty(window, 'isSecureContext', { configurable: true, writable: true, value: false }); + const execCommand = vi.fn(() => true); + Object.defineProperty(document, 'execCommand', { configurable: true, writable: true, value: execCommand }); + setup(); + + fireEvent.click(await screen.findByRole('button', { name: 'Copy' })); + + await waitFor(() => expect(execCommand).toHaveBeenCalledWith('copy')); + expect(await screen.findByRole('button', { name: 'Copied!' })).toBeInTheDocument(); + // The temporary textarea is removed again. + expect(document.querySelector('textarea')).toBeNull(); + }); + it('FE-MOB-JSET-030: an existing cover is shown instead of the gradient', async () => { setup({ cover_image: 'covers/j.jpg' }); await waitFor(() => diff --git a/client/tests/unit/mobile/settings/MSettingsAccount.test.tsx b/client/tests/unit/mobile/settings/MSettingsAccount.test.tsx index 62eb83561..050bb281c 100644 --- a/client/tests/unit/mobile/settings/MSettingsAccount.test.tsx +++ b/client/tests/unit/mobile/settings/MSettingsAccount.test.tsx @@ -663,6 +663,28 @@ describe('MSettingsAccount – backup codes', () => { openSpy.mockRestore(); }); + // Parity with the desktop AccountTab: nothing reaches the popup unescaped, even + // though real codes are plain hex. + it('FE-MOB-SETACC-049: Print escapes the codes before writing them', async () => { + const user = userEvent.setup(); + const written: string[] = []; + const popup = { + document: { open: vi.fn(), write: (html: string) => written.push(html), close: vi.fn() }, + focus: vi.fn(), + print: vi.fn(), + }; + const openSpy = vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + sessionStorage.setItem(BACKUP_KEY, JSON.stringify([''])); + renderAccount(); + + await user.click(await screen.findByRole('button', { name: 'Print / PDF' })); + + const html = written.join(''); + expect(html).not.toContain(' { const user = userEvent.setup(); const openSpy = vi.spyOn(window, 'open').mockReturnValue(null); diff --git a/client/tests/unit/mobile/settings/MSettingsAppearance.test.tsx b/client/tests/unit/mobile/settings/MSettingsAppearance.test.tsx index e4570051b..bb39734d0 100644 --- a/client/tests/unit/mobile/settings/MSettingsAppearance.test.tsx +++ b/client/tests/unit/mobile/settings/MSettingsAppearance.test.tsx @@ -386,7 +386,9 @@ describe('MSettingsAppearance', () => { ); }); - it('FE-MOB-SETAPP-027: unmounting before the debounce fires cancels the pending save', async () => { + // The scheme is already on the DOM by then, so dropping the write would revert the + // choice on the next load without ever telling the user. + it('FE-MOB-SETAPP-027: unmounting before the debounce fires flushes the pending save once', async () => { const user = userEvent.setup(); const updateSetting = seedAppearance(undefined); const { unmount } = renderAppearance(); @@ -394,7 +396,23 @@ describe('MSettingsAppearance', () => { await user.click(screen.getByRole('button', { name: 'Indigo' })); unmount(); + expect(appearanceCalls(updateSetting)).toHaveLength(1); + expect(appearanceCalls(updateSetting)[0].schemeId).toBe('indigo'); + + // The timer is cancelled, so nothing lands a second time. await new Promise((resolve) => setTimeout(resolve, 500)); - expect(appearanceCalls(updateSetting)).toHaveLength(0); + expect(appearanceCalls(updateSetting)).toHaveLength(1); + }); + + it('FE-MOB-SETAPP-028: a debounce that already fired is not written again on unmount', async () => { + const user = userEvent.setup(); + const updateSetting = seedAppearance(undefined); + const { unmount } = renderAppearance(); + + await user.click(screen.getByRole('button', { name: 'Teal' })); + await waitFor(() => expect(appearanceCalls(updateSetting)).toHaveLength(1)); + + unmount(); + expect(appearanceCalls(updateSetting)).toHaveLength(1); }); }); diff --git a/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx b/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx index 3f424f390..2aa52faca 100644 --- a/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx +++ b/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx @@ -504,6 +504,43 @@ describe('MSettingsNotifications', () => { await waitFor(() => expect(screen.getByRole('button', { name: 'In-App' }).className).toBe(activeClass)); }); + it('FE-MOB-SETNOTIF-032: a failing update toasts and leaves a later toggle alone', async () => { + const user = userEvent.setup(); + let rejectInapp!: () => void; + server.use( + http.put('/api/notifications/preferences', async ({ request }) => { + const body = (await request.json()) as Record>; + // The first write is the in-app flip; it hangs until the test rejects it. + if (body.trip_invite.webhook === false) { + return new Promise(resolve => { + rejectInapp = () => resolve(HttpResponse.json({ error: 'nope' }, { status: 500 }) as unknown as Response); + }); + } + return HttpResponse.json({ success: true }); + }), + ); + render(<>); + + const inapp = await screen.findByRole('button', { name: 'In-App' }); + // The webhook chip is off in this matrix — its look is the "inactive" reference. + const inactiveClass = screen.getByRole('button', { name: 'Webhook' }).className; + const activeClass = inapp.className; + + await user.click(inapp); + await waitFor(() => expect(screen.getByRole('button', { name: 'In-App' }).className).toBe(inactiveClass)); + + // Flipped while the in-app write is still hanging. + await user.click(screen.getByRole('button', { name: 'Webhook' })); + await waitFor(() => expect(screen.getByRole('button', { name: 'Webhook' }).className).toBe(activeClass)); + + rejectInapp(); + + expect(await screen.findByText('Error')).toBeInTheDocument(); + await waitFor(() => expect(screen.getByRole('button', { name: 'In-App' }).className).toBe(activeClass)); + // The webhook toggle the user made meanwhile is not undone. + expect(screen.getByRole('button', { name: 'Webhook' }).className).toBe(activeClass); + }); + it('FE-MOB-SETNOTIF-029: the saving hint shows while the update is in flight', async () => { const user = userEvent.setup(); let release!: () => void; diff --git a/client/tests/unit/mobile/trip/MCostSheet.test.tsx b/client/tests/unit/mobile/trip/MCostSheet.test.tsx index 3689e1b13..f06e873aa 100644 --- a/client/tests/unit/mobile/trip/MCostSheet.test.tsx +++ b/client/tests/unit/mobile/trip/MCostSheet.test.tsx @@ -5,6 +5,7 @@ import { clearExchangeRateCache } from '../../../../src/hooks/useExchangeRates' import { useTripStore, type TripStoreState } from '../../../../src/store/tripStore' import type { BudgetItem, BudgetItemMember } from '../../../../src/types' import { buildBudgetItem } from '../../../helpers/factories' +import { localToday } from '../../../../src/components/Planner/today' import { resetAllStores } from '../../../helpers/store' import { act, fireEvent, render, screen, waitFor } from '../../../helpers/render' @@ -17,7 +18,8 @@ const PEOPLE = [ { id: 2, username: 'bob', avatar_url: null }, ] as TripMember[] -const TODAY = new Date().toISOString().slice(0, 10) +// The traveller's own calendar day, not the UTC one — see components/Planner/today.ts. +const TODAY = localToday() function member(user_id: number, amount: number | null): BudgetItemMember { return { user_id, paid: 0, username: user_id === 1 ? 'alice' : 'bob', amount } @@ -572,6 +574,32 @@ describe('MCostSheet', () => { expect(screen.getByText('Guest')).toBeInTheDocument() }) + // #1567 all over again: an expense entered at 23:00 in Tokyo belongs on that + // evening's day, not on the UTC date that has not started there yet. + it('FE-MOB-COSTSH-031: the default expense date comes off the local clock, not UTC', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + try { + // East of Greenwich the two dates part just after midnight, west of it just + // before, so pick the edge that this runner's zone actually has. + const hour = new Date(2026, 7, 12).getTimezoneOffset() > 0 ? 23 : 0 + const now = new Date(2026, 7, 12, hour, 30, 0) + vi.setSystemTime(now) + const { onSaved } = renderSheet() + fillBasics('Ramen', '12') + fireEvent.click(submit()) + + await waitFor(() => expect(addBudgetItem).toHaveBeenCalledTimes(1)) + expect(addBudgetItem.mock.calls[0][1]).toMatchObject({ expense_date: '2026-08-12' }) + // Guard the guard: in a UTC runner there is no drift to catch. + if (now.getTimezoneOffset() !== 0) { + expect(now.toISOString().slice(0, 10)).not.toBe('2026-08-12') + } + await waitFor(() => expect(onSaved).toHaveBeenCalledTimes(1)) + } finally { + vi.useRealTimers() + } + }) + it('FE-MOB-COSTSH-030: a currency without a known symbol falls back to its code', () => { localStorage.setItem('trek_fx_XTS', JSON.stringify({ rates: { XTS: 1 }, ts: Date.now() })) renderSheet({ base: 'XTS' }) diff --git a/client/tests/unit/mobile/trip/MFileLightbox.test.tsx b/client/tests/unit/mobile/trip/MFileLightbox.test.tsx index c59778d45..35a000fb3 100644 --- a/client/tests/unit/mobile/trip/MFileLightbox.test.tsx +++ b/client/tests/unit/mobile/trip/MFileLightbox.test.tsx @@ -205,4 +205,27 @@ describe('MFileLightbox', () => { fireEvent.click(screen.getByTestId('video-player').parentElement as HTMLElement) expect(onClose).not.toHaveBeenCalled() }) + + // Swiping is quicker than the token round trip, so the first file's url must not + // land on the second one. + it('FE-MOB-FLBOX-018: a token that resolves after a swipe does not overwrite the new image', async () => { + const release: Array<(url: string) => void> = [] + vi.mocked(getAuthUrl).mockImplementation( + (url: string) => new Promise(resolve => { release.push(() => resolve(`${url}?token=t1`)) }), + ) + const { rerender, onIndexChange, onClose } = renderBox(0) + + rerender( + , + ) + await waitFor(() => expect(release).toHaveLength(2)) + + // The first file resolves last. + release[1]('') + release[0]('') + + await waitFor(() => + expect(screen.getByAltText('castle.png')).toHaveAttribute('src', '/api/trips/1/files/2/download?token=t1'), + ) + }) }) diff --git a/client/tests/unit/mobile/trip/MReservationSheet.test.tsx b/client/tests/unit/mobile/trip/MReservationSheet.test.tsx index f168a4654..f40077a2a 100644 --- a/client/tests/unit/mobile/trip/MReservationSheet.test.tsx +++ b/client/tests/unit/mobile/trip/MReservationSheet.test.tsx @@ -718,6 +718,27 @@ describe('MReservationSheet', () => { expect(selects()[2]).toHaveAttribute('data-value', '13') }) + // A legacy or corrupted metadata column used to throw straight out of the open + // effect and leave the sheet blank. + it('FE-MOB-RESSH-052: unparseable metadata opens the sheet with empty check-in times', () => { + setup(makePlanner({ + editingReservation: { ...DINNER, type: 'hotel', metadata: '{not json' }, + })) + expect(titleField()).toHaveValue('Dinner') + expect(screen.getByPlaceholderText('15:00')).toHaveValue('') + }) + + it('FE-MOB-RESSH-053: double-encoded metadata still fills the check-in times', () => { + setup(makePlanner({ + editingReservation: { + ...DINNER, type: 'hotel', + metadata: JSON.stringify(JSON.stringify({ check_in_time: '16:00', check_out_time: '10:30' })), + }, + })) + expect(screen.getByPlaceholderText('15:00')).toHaveValue('16:00') + expect(screen.getByPlaceholderText('11:00')).toHaveValue('10:30') + }) + it('FE-MOB-RESSH-051: a hotel place without an address keeps the typed one', () => { setup() fireEvent.click(screen.getByRole('button', { name: 'reservations.type.hotel' })) diff --git a/client/tests/unit/mobile/trip/costsModel.test.ts b/client/tests/unit/mobile/trip/costsModel.test.ts index 6c38dee69..dd399a306 100644 --- a/client/tests/unit/mobile/trip/costsModel.test.ts +++ b/client/tests/unit/mobile/trip/costsModel.test.ts @@ -371,6 +371,30 @@ describe('costsModel — CSV export', () => { expect(filename).toBe('costs-trip.csv'); }); + it('FE-MOB-CMOD-028: prefixes formula-leading names and notes so spreadsheets keep them as text', () => { + const items = [ + expense({ + id: 46, + name: '=HYPERLINK("http://evil","click")', + category: 'food', + expense_date: null, + total_price: 5, + note: '@SUM(A1:A9)', + }), + expense({ id: 47, name: '-12 refund', category: 'other', expense_date: null, total_price: 5 }), + ]; + + const { content } = buildCostsCsv(items, { base: 'EUR', ctx, locale: 'en-US', tripTitle: null, t }); + + const rows = content.split('\r\n'); + // The quote inside the name still forces the field to be quoted around the apostrophe. + expect(rows[1]).toContain(`"'=HYPERLINK(""http://evil"",""click"")"`); + expect(rows[1]).toContain("'@SUM(A1:A9)"); + expect(rows[2]).toContain("'-12 refund"); + // Numbers never go through the escaper, so the amount columns stay parseable. + expect(rows[2]).toContain(';5.00;EUR;5.00;'); + }); + it('FE-MOB-CMOD-024: falls back to the raw ISO date when the locale is unusable', () => { const { content } = buildCostsCsv( [expense({ id: 44, name: 'Bus', category: 'transport', expense_date: '2026-07-16', total_price: 3 })], diff --git a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts index 486990ffc..df9650b93 100644 --- a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts +++ b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts @@ -607,6 +607,9 @@ describe('useMPlanTimeline', () => { expect(planner.handleUpdateDayTitle).toHaveBeenCalledWith(2, 'Old Town') }) + // The two built-in modes were the only labels on this menu written in English + // instead of looked up, so a German reader got "Driving"/"Walking" between + // translated rows — and next to plugin profiles carrying their own label. it('FE-MOB-PLTL-040: offers the built-in travel modes plus every plugin profile', async () => { usePluginStore.setState({ plugins: [ @@ -616,8 +619,8 @@ describe('useMPlanTimeline', () => { }) const { result } = await renderTimeline(makePlanner()) expect(result.current.routeModeOptions).toEqual([ - { key: 'driving', label: 'Driving' }, - { key: 'walking', label: 'Walking' }, + { key: 'driving', label: 'mobileTrip.profileDriving' }, + { key: 'walking', label: 'mobileTrip.profileWalking' }, { key: 'plugin:ev/eco', label: 'Eco' }, { key: 'plugin:ev/fast', label: 'Fast' }, ]) diff --git a/client/tests/unit/mswUnhandledRequests.test.ts b/client/tests/unit/mswUnhandledRequests.test.ts new file mode 100644 index 000000000..76eabe011 --- /dev/null +++ b/client/tests/unit/mswUnhandledRequests.test.ts @@ -0,0 +1,10 @@ +import { describe, it, expect } from 'vitest'; + +// Guards the lifecycle option in tests/setup.ts: a cross-origin call nobody mocked must +// fail here rather than actually leaving the runner (see msw/handlers/external.ts for the +// FX widget that once did, and answered after its test environment was gone). +describe('msw unhandled requests', () => { + it('FE-MSW-EGRESS-001: an unmocked cross-origin request is refused', async () => { + await expect(fetch('https://unmocked.example/rates')).rejects.toThrow(); + }); +}); diff --git a/client/tests/unit/remoteEventHandler/places.test.ts b/client/tests/unit/remoteEventHandler/places.test.ts index 58d5c9ea9..82b2ca238 100644 --- a/client/tests/unit/remoteEventHandler/places.test.ts +++ b/client/tests/unit/remoteEventHandler/places.test.ts @@ -51,6 +51,56 @@ describe('remoteEventHandler > places', () => { expect(assignments['10'][0].place?.name).toBe('Cascaded Update'); }); + it('FE-WSEVT-PLACE-004b: place:updated keeps a per-stop override', () => { + // The day view renders COALESCE(assignment_time, place_time), so an + // assignment that carries its own time keeps it when the pool row changes + // (the local reducer merges the same way). + const place = buildPlace({ id: 1, name: 'Original', place_time: '08:00', end_time: '09:00' }); + const assignment = buildAssignment({ + id: 100, + day_id: 10, + assignment_time: '14:00', + assignment_end_time: '15:30', + place: { ...place, place_time: '14:00', end_time: '15:30' }, + }); + useTripStore.setState({ places: [place], assignments: { '10': [assignment] } }); + + useTripStore.getState().handleRemoteEvent({ + type: 'place:updated', + place: buildPlace({ id: 1, name: 'Renamed', place_time: '08:00', end_time: '09:00' }), + }); + + const embedded = useTripStore.getState().assignments['10'][0].place!; + expect(embedded.name).toBe('Renamed'); + expect(embedded.place_time).toBe('14:00'); + expect(embedded.end_time).toBe('15:30'); + }); + + it('FE-WSEVT-PLACE-004c: without an override the pool time reaches the day card', () => { + // The other half of the COALESCE. An assignment with no assignment_time + // shows whatever the pool row says, so a collaborator moving the place from + // 08:00 to 10:00 has to land here - freezing the embedded copy would leave + // the day plan wrong until the next reload. + const place = buildPlace({ id: 1, name: 'Original', place_time: '08:00', end_time: '09:00' }); + const assignment = buildAssignment({ + id: 100, + day_id: 10, + assignment_time: null, + assignment_end_time: null, + place: { ...place }, + }); + useTripStore.setState({ places: [place], assignments: { '10': [assignment] } }); + + useTripStore.getState().handleRemoteEvent({ + type: 'place:updated', + place: buildPlace({ id: 1, name: 'Original', place_time: '10:00', end_time: '11:00' }), + }); + + const embedded = useTripStore.getState().assignments['10'][0].place!; + expect(embedded.place_time).toBe('10:00'); + expect(embedded.end_time).toBe('11:00'); + }); + it('FE-WSEVT-PLACE-005: place:deleted removes place from places array', () => { seedData(); useTripStore.getState().handleRemoteEvent({ type: 'place:deleted', placeId: 1 }); diff --git a/client/tests/unit/slices/dayNotesSlice.test.ts b/client/tests/unit/slices/dayNotesSlice.test.ts index 3d770ec0f..65d488a59 100644 --- a/client/tests/unit/slices/dayNotesSlice.test.ts +++ b/client/tests/unit/slices/dayNotesSlice.test.ts @@ -138,17 +138,49 @@ describe('dayNotesSlice', () => { const note = buildDayNote({ id: 10, day_id: 1, text: 'Move me' }); seedStore(useTripStore, { dayNotes: { '1': [note], '2': [] } }); + let deleted = false; server.use( - http.delete('/api/trips/1/days/1/notes/10', () => + http.post('/api/trips/1/days/2/notes', () => HttpResponse.json({ message: 'Error' }, { status: 500 }) ), + http.delete('/api/trips/1/days/1/notes/10', () => { + deleted = true; + return HttpResponse.json({ success: true }); + }), ); await expect(useTripStore.getState().moveDayNote(1, 1, 2, 10)).rejects.toThrow(); + // The create is the first half of the move, so a failure there must leave + // the note where it was instead of deleting it out from under the user. + expect(deleted).toBe(false); expect(useTripStore.getState().dayNotes['1']).toHaveLength(1); expect(useTripStore.getState().dayNotes['1'][0].id).toBe(10); }); + + it('FE-DAYNOTES-006b: moveDayNote removes the copy again when the source delete fails', async () => { + const note = buildDayNote({ id: 10, day_id: 1, text: 'Move me' }); + const newNote = buildDayNote({ id: 99, day_id: 2, text: 'Move me' }); + seedStore(useTripStore, { dayNotes: { '1': [note], '2': [] } }); + + const deletedFromTarget: number[] = []; + server.use( + http.post('/api/trips/1/days/2/notes', () => HttpResponse.json({ note: newNote })), + http.delete('/api/trips/1/days/1/notes/10', () => + HttpResponse.json({ message: 'Error' }, { status: 500 }) + ), + http.delete('/api/trips/1/days/2/notes/:noteId', ({ params }) => { + deletedFromTarget.push(Number(params.noteId)); + return HttpResponse.json({ success: true }); + }), + ); + + await expect(useTripStore.getState().moveDayNote(1, 1, 2, 10)).rejects.toThrow(); + + expect(deletedFromTarget).toEqual([99]); + expect(useTripStore.getState().dayNotes['1']).toHaveLength(1); + expect(useTripStore.getState().dayNotes['2']).toHaveLength(0); + }); }); describe('updateDayNotes', () => { diff --git a/client/tests/unit/stores/authStore.test.ts b/client/tests/unit/stores/authStore.test.ts index 9ade8057a..9b81d1083 100644 --- a/client/tests/unit/stores/authStore.test.ts +++ b/client/tests/unit/stores/authStore.test.ts @@ -10,6 +10,15 @@ import { buildUser } from '../../helpers/factories'; // The websocket module is already mocked globally in tests/setup.ts import { connect, disconnect } from '../../../src/api/websocket'; +// authStore is the only consumer of these two, so stubbing the module keeps the +// real 30s interval and window listeners out of the suite while still letting us +// assert the register/unregister ordering around a logout. +const syncTriggers = vi.hoisted(() => ({ + registerSyncTriggers: vi.fn(), + unregisterSyncTriggers: vi.fn(), +})); +vi.mock('../../../src/sync/syncTriggers', () => syncTriggers); + // The user-scoped offline DB is real (fake-indexeddb); only the reopen step is // made failable so the "auth still succeeds when the DB won't open" path runs. const dbControl = vi.hoisted(() => ({ reopenFails: false })); @@ -732,4 +741,22 @@ describe('authStore', () => { expect(state.placesDetailsEnabled).toBe(false); }); }); + + describe('FE-STORE-AUTH-033: logging in again after a logout', () => { + it('re-registers the sync triggers logout tore down', async () => { + const user = buildUser(); + server.use( + http.post('/api/auth/login', () => HttpResponse.json({ user, token: 'tok' })) + ); + + useAuthStore.setState({ user, isAuthenticated: true }); + await useAuthStore.getState().logout(); + expect(syncTriggers.unregisterSyncTriggers).toHaveBeenCalled(); + + syncTriggers.registerSyncTriggers.mockClear(); + await useAuthStore.getState().login(user.email, 'password'); + + expect(syncTriggers.registerSyncTriggers).toHaveBeenCalled(); + }); + }); }); diff --git a/client/tests/unit/studio/StudioCanvas.test.tsx b/client/tests/unit/studio/StudioCanvas.test.tsx new file mode 100644 index 000000000..eda09fb10 --- /dev/null +++ b/client/tests/unit/studio/StudioCanvas.test.tsx @@ -0,0 +1,57 @@ +import { describe, it, expect, beforeEach } from 'vitest' +import { render, screen, act } from '../../helpers/render' +import { bookPageSetupSchema, bookSpreadSchema } from '@trek/shared' +import { seedStore } from '../../helpers/store' +import { buildSettings } from '../../helpers/factories' +import { useSettingsStore } from '../../../src/store/settingsStore' +import { useStudioStore } from '../../../src/store/studioStore' +import { StudioCanvas } from '../../../src/components/Studio/StudioCanvas' + +const page = bookPageSetupSchema.parse({}) +const spreadWith = (locked: boolean) => bookSpreadSchema.parse({ + id: 'sp-1', + elements: [{ id: 'el-1', kind: 'shape', frame: { x: 10, y: 10, w: 40, h: 30 }, locked }], +}) + +/** Mount, then select the one element — the chrome only exists for a selection. */ +const mountSelected = (locked = false) => { + render( + + ) + act(() => { useStudioStore.setState({ selection: ['el-1'] }) }) +} + +describe('StudioCanvas', () => { + beforeEach(() => { + useStudioStore.setState({ selection: [] }) + }) + + // The quickbar and the rotation handles were the last chrome in Studio with + // English written into the JSX, so they stayed English while the panels + // around them followed the reader's language. + it('FE-COMP-STUDIOCANVAS-001: the quickbar and the rotation handles are translated', async () => { + seedStore(useSettingsStore, { settings: buildSettings({ language: 'de' }) }) + mountSelected() + + // The locale bundle is fetched, so the first paint is still English. + expect(await screen.findByTitle('Duplizieren')).toBeInTheDocument() + expect(screen.getAllByTitle('Drehen')).toHaveLength(4) + expect(screen.getByTitle('Nach links drehen')).toBeInTheDocument() + expect(screen.getByTitle('Nach rechts drehen')).toBeInTheDocument() + expect(screen.getByTitle('Ganz nach vorn')).toBeInTheDocument() + expect(screen.getByTitle('Ganz nach hinten')).toBeInTheDocument() + expect(screen.getByTitle('Sperren')).toBeInTheDocument() + expect(screen.getByTitle('Löschen')).toBeInTheDocument() + expect(screen.queryByTitle('Rotate')).not.toBeInTheDocument() + }) + + it('FE-COMP-STUDIOCANVAS-002: the lock button names the press that follows, in the same language', async () => { + seedStore(useSettingsStore, { settings: buildSettings({ language: 'de' }) }) + mountSelected(true) + + expect(await screen.findByTitle('Entsperren')).toBeInTheDocument() + // A locked element keeps its handles hidden, so nothing rotates it by accident. + expect(screen.queryByTitle('Drehen')).not.toBeInTheDocument() + }) +}) diff --git a/client/tests/unit/studio/applyTemplate.test.ts b/client/tests/unit/studio/applyTemplate.test.ts index 1c3d957d3..3a8b7b28e 100644 --- a/client/tests/unit/studio/applyTemplate.test.ts +++ b/client/tests/unit/studio/applyTemplate.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import type { BookElement } from '@trek/shared' +import type { BookElement, JourneyStats } from '@trek/shared' import { bookPageSetupSchema } from '@trek/shared' import { applyTemplate, templateFit, type TemplateEntry } from '../../../src/components/Studio/applyTemplate' import type { SpreadTemplate } from '../../../src/components/Studio/bookTemplates.data' @@ -110,6 +110,14 @@ describe('what gets filled', () => { expect((spread.elements[0] as { text: string }).text).toBe('DAY 1') }) + /* Every other figure on the page follows the book language; so does this one. */ + it('sets the distance in the book language', () => { + const mark = el({ kind: 'badge', variant: 'distance', text: '', sub: '', code: null, style: 'plain' }) + const travelled = { distance: 1_189_000, countries: [] } as unknown as JourneyStats + const spread = applyTemplate(template([mark]), entry(), { ...ctx, locale: 'de', stats: travelled }) + expect((spread.elements[0] as { text: string }).text).toBe('1.189 km') + }) + it('belongs to the entry it was filled from', () => { expect(applyTemplate(template([titleEl]), entry(), ctx).entryId).toBe(7) }) diff --git a/client/tests/unit/studio/autoLayout.travel.test.ts b/client/tests/unit/studio/autoLayout.travel.test.ts index b3d8b8302..f369fa26b 100644 --- a/client/tests/unit/studio/autoLayout.travel.test.ts +++ b/client/tests/unit/studio/autoLayout.travel.test.ts @@ -197,6 +197,16 @@ describe('the back cover', () => { expect(el.opacity).toBeLessThan(1) }) + /* + * places and steps are separate figures — a journal with entries but no + * linked trip has steps and no places at all, and the tile printed "0". + */ + it('leaves out the places tally when the journey has none', () => { + const back = last(build({ stats: stats({ places: 0, steps: 4 }) }).spreads) + const el = back.elements.find(e => e.kind === 'stats')! + expect(el.kind === 'stats' && el.metrics).not.toContain('places') + }) + it('still closes the book when there are no figures', () => { const back = last(build({ stats: null }).spreads) expect(back.role).toBe('back') diff --git a/client/tests/unit/studio/lockedElement.test.tsx b/client/tests/unit/studio/lockedElement.test.tsx index 8e80e9a41..fbf502ca4 100644 --- a/client/tests/unit/studio/lockedElement.test.tsx +++ b/client/tests/unit/studio/lockedElement.test.tsx @@ -110,6 +110,41 @@ describe('a locked element', () => { expect(container.querySelectorAll('.st-select')).toHaveLength(1) }) + /* + * Delete was the one gesture that ignored the flag: the element was still + * selectable, so a stray Delete took away exactly what had been pinned down. + */ + it('is not deleted by the Delete key', () => { + load(shape({ locked: true })) + const { container } = draw() + fireEvent.pointerDown(hitTarget(container)) + + fireEvent.keyDown(document, { key: 'Delete' }) + + expect(useStudioStore.getState().doc!.spreads[0].elements).toHaveLength(1) + }) + + it('is not deleted by the quick bar either', () => { + load(shape({ locked: true })) + const { container, getByTitle } = draw() + fireEvent.pointerDown(hitTarget(container)) + fireEvent.pointerUp(container.querySelector('.st-stage')!) + + fireEvent.click(getByTitle('Delete')) + + expect(useStudioStore.getState().doc!.spreads[0].elements).toHaveLength(1) + }) + + it('leaves an unlocked element deletable', () => { + load(shape({ locked: false })) + const { container } = draw() + fireEvent.pointerDown(hitTarget(container)) + + fireEvent.keyDown(document, { key: 'Delete' }) + + expect(useStudioStore.getState().doc!.spreads[0].elements).toHaveLength(0) + }) + it('leaves an unlocked element movable, with its handles', () => { load(shape({ locked: false })) const { container } = draw() diff --git a/client/tests/unit/studio/mapRoads.test.tsx b/client/tests/unit/studio/mapRoads.test.tsx index 660a7351b..86016242e 100644 --- a/client/tests/unit/studio/mapRoads.test.tsx +++ b/client/tests/unit/studio/mapRoads.test.tsx @@ -1,8 +1,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import type { BookElement, BookPageSetup, BookSpread } from '@trek/shared' +import type { BookDocument, BookElement, BookPageSetup, BookSpread, JourneyStats } from '@trek/shared' import { bookPageSetupSchema } from '@trek/shared' -import { render } from '../../helpers/render' +import { fireEvent, render } from '../../helpers/render' import { SpreadView } from '../../../src/components/Studio/SpreadView' +import { StudioTravelPanel } from '../../../src/components/Studio/StudioTravelPanel' +import { useStudioStore } from '../../../src/store/studioStore' import { fetchRoads } from '../../../src/components/Studio/roadRoute' vi.mock('../../../src/components/Map/RouteCalculator', () => ({ @@ -216,3 +218,41 @@ describe('what gets asked for', () => { expect(roads[0]!.length).toBe(40) }) }) + +/** + * The lookups outlive the click by design — they are paced so the router is not + * hammered — so the panel has to take them with it when it goes. + */ +describe('placing a map from the panel', () => { + const journeyStats = { + journeyId: 1, distance: 0, days: 1, steps: 3, photos: 0, places: 3, furthest: 0, + countries: [], trips: [], start: null, end: null, + points: STOPS.map(p => ({ ...p, date: null, country: null, tripId: null })), + } as unknown as JourneyStats + + beforeEach(() => { + vi.mocked(calculateRouteWithLegs).mockReset() + useStudioStore.getState().load({ + version: 1, title: 'T', page, + spreads: [{ id: 's1', role: 'inner', background: null, elements: [], parked: [], entryId: null }], + } as unknown as BookDocument) + }) + + it('drops the road lookup when the panel goes away', async () => { + let asked: AbortSignal | undefined + vi.mocked(calculateRouteWithLegs).mockImplementation((_stops, opts) => { + asked = (opts as { signal?: AbortSignal } | undefined)?.signal + return new Promise(() => {}) as never + }) + + const { getByTitle, unmount } = render( + k} locale="en" />, + ) + fireEvent.click(getByTitle('journey.studio.mapStyle.minimal')) + await vi.waitFor(() => expect(calculateRouteWithLegs).toHaveBeenCalled()) + + unmount() + + expect(asked?.aborted).toBe(true) + }) +}) diff --git a/client/tests/unit/studio/useBookStore.test.tsx b/client/tests/unit/studio/useBookStore.test.tsx index 0f92f8fa6..1a14b582e 100644 --- a/client/tests/unit/studio/useBookStore.test.tsx +++ b/client/tests/unit/studio/useBookStore.test.tsx @@ -368,11 +368,55 @@ describe('conflicts', () => { expect(result.current.state.status).toBe('conflict') api.saveBook.mockResolvedValue(record({ version: 7 })) + // Choosing re-sends what was refused, so this is the third write, not the second. act(() => { result.current.keepMine(theirs) }) + await tick() act(() => { result.current.queueSave(doc('mine again'), 'T') }) await tick() + expect(api.saveBook).toHaveBeenCalledTimes(3) + }) + + /* + * The one that lost an afternoon: write() empties the queue before it makes + * the request, so after a 409 nothing is queued. "Keep mine" cleared the + * conflict and left the document unsent until the user happened to touch the + * book again — and closing the editor writes the queue, which was empty. + */ + it('sends the refused document as soon as the local one is kept', async () => { + const theirs = record({ version: 6 }) + api.saveBook.mockRejectedValueOnce(conflictError(theirs)) + const { result } = await mount() + + act(() => { result.current.queueSave(doc('mine'), 'T') }) + await tick() + expect(result.current.state.status).toBe('conflict') + + api.saveBook.mockResolvedValue(record({ version: 7 })) + act(() => { result.current.keepMine(theirs) }) + await tick() + expect(api.saveBook).toHaveBeenCalledTimes(2) + expect(api.saveBook.mock.calls[1][1]).toMatchObject({ title: 'T', baseVersion: 6 }) + expect((api.saveBook.mock.calls[1][1].document as BookDocument).title).toBe('mine') + expect(result.current.state.status).toBe('saved') + }) + + /* Nothing of their own to send: keeping theirs-as-mine is just the version. */ + it('only clears the conflict when the local document is the agreed one', async () => { + const theirs = record({ version: 6 }) + api.saveBook.mockRejectedValueOnce(conflictError(theirs)) + const { result } = await mount() + + act(() => { result.current.queueSave(doc('mine'), 'T') }) + await tick() + + act(() => { result.current.acceptTheirs(theirs) }) + act(() => { result.current.keepMine(theirs) }) + await tick() + + expect(api.saveBook).toHaveBeenCalledTimes(1) + expect(result.current.state.status).toBe('idle') }) it('returns their document when their version is taken, and drops the pending write', async () => { @@ -408,11 +452,8 @@ describe('conflicts', () => { await tick() expect(result.current.state.status).toBe('conflict') - act(() => { result.current.keepMine(theirs) }) - expect(result.current.state.status).toBe('idle') - api.saveBook.mockResolvedValue(record({ version: 7 })) - act(() => { result.current.queueSave(doc('mine again'), 'T') }) + act(() => { result.current.keepMine(theirs) }) await tick() expect(api.saveBook.mock.calls[1][1].baseVersion).toBe(6) diff --git a/client/tests/unit/sync/mutationQueue.test.ts b/client/tests/unit/sync/mutationQueue.test.ts index 8e7868acf..19cfc6a5d 100644 --- a/client/tests/unit/sync/mutationQueue.test.ts +++ b/client/tests/unit/sync/mutationQueue.test.ts @@ -241,6 +241,47 @@ describe('mutationQueue.flush — offline guard', () => { // ── pending / pendingCount ──────────────────────────────────────────────────── +describe('mutationQueue.flush — interrupted flush recovery', () => { + it('replays a row an earlier flush abandoned on syncing', async () => { + let seen = 0; + server.use( + http.post('/api/trips/1/places', () => { + seen += 1; + return HttpResponse.json({ place: buildPlace({ id: 42 }) }); + }), + ); + + // What a killed tab leaves behind: marked syncing, stamped long enough ago + // that no live request could still be in flight. + const id = generateUUID(); + await offlineDb.mutationQueue.add({ + id, tripId: 1, method: 'POST', url: '/trips/1/places', + body: { name: 'Eiffel Tower' }, resource: 'places', + createdAt: Date.now() - 600_000, status: 'syncing', attempts: 0, lastError: null, + syncingSince: Date.now() - 600_000, + }); + + await mutationQueue.flush(); + + expect(seen).toBe(1); + expect(await offlineDb.mutationQueue.get(id)).toBeUndefined(); + }); + + it('leaves a freshly marked syncing row alone (a concurrent flush owns it)', async () => { + const id = generateUUID(); + await offlineDb.mutationQueue.add({ + id, tripId: 1, method: 'POST', url: '/trips/1/places', + body: { name: 'Eiffel Tower' }, resource: 'places', + createdAt: Date.now(), status: 'syncing', attempts: 0, lastError: null, + syncingSince: Date.now(), + }); + + await mutationQueue.flush(); + + expect((await offlineDb.mutationQueue.get(id))!.status).toBe('syncing'); + }); +}); + describe('mutationQueue.pending', () => { it('returns pending mutations for a trip', async () => { const id1 = generateUUID(); diff --git a/client/tests/unit/sync/syncTriggers.test.ts b/client/tests/unit/sync/syncTriggers.test.ts index fb1aa6eb3..b2826fca2 100644 --- a/client/tests/unit/sync/syncTriggers.test.ts +++ b/client/tests/unit/sync/syncTriggers.test.ts @@ -82,6 +82,10 @@ describe('syncTriggers', () => { expect(flush).toHaveBeenCalled(); expect(syncAll).toHaveBeenCalled(); expect(hydrate).toHaveBeenCalledWith('7'); + // The order is the point: re-seeding Dexie before the queue drains would + // overwrite edits that have not been sent yet. + expect(flush.mock.invocationCallOrder[0]).toBeLessThan(syncAll.mock.invocationCallOrder[0]); + expect(flush.mock.invocationCallOrder[0]).toBeLessThan(hydrate.mock.invocationCallOrder[0]); }); it('online event retries the settings load when it has not yet succeeded (#1618)', async () => { diff --git a/client/tests/unit/sync/tripSyncManager.test.ts b/client/tests/unit/sync/tripSyncManager.test.ts index 46ff38291..8dea8a2e1 100644 --- a/client/tests/unit/sync/tripSyncManager.test.ts +++ b/client/tests/unit/sync/tripSyncManager.test.ts @@ -22,11 +22,12 @@ import { buildTripFile, } from '../../helpers/factories'; -// Helper to get today ± N days as YYYY-MM-DD +// Helper to get today ± N days as YYYY-MM-DD. Local calendar date, like the +// manager itself, so the ±1-day boundary cases hold outside UTC too. function dateOffset(days: number): string { const d = new Date(); d.setDate(d.getDate() + days); - return d.toISOString().slice(0, 10); + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}`; } function makeBundle(tripId: number) { @@ -283,3 +284,94 @@ describe('tripSyncManager — file blob caching', () => { expect(cached.length).toBe(0); }); }); + +// ── local calendar dates ─────────────────────────────────────────────────────── + +describe('tripSyncManager.syncAll — local calendar dates', () => { + // Only Date is faked; Dexie and MSW keep their real timers. + const withClock = (tz: string, iso: string, run: () => Promise) => async () => { + const prevTz = process.env.TZ; + process.env.TZ = tz; + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date(iso)); + try { + await run(); + } finally { + vi.useRealTimers(); + if (prevTz === undefined) delete process.env.TZ; + else process.env.TZ = prevTz; + } + }; + + it('caches a trip on its last evening west of UTC', withClock( + 'America/Los_Angeles', + // 21:00 on the 22nd in Los Angeles is already the 23rd in UTC. + '2026-08-23T04:00:00Z', + async () => { + const tripId = 110; + const trip = buildTrip({ id: tripId, end_date: '2026-08-22' }); + const bundle = { ...makeBundle(tripId), trip }; + + server.use( + http.get('/api/trips', () => HttpResponse.json({ trips: [trip] })), + http.get(`/api/trips/${tripId}/bundle`, () => HttpResponse.json(bundle)), + ); + + await tripSyncManager.syncAll(); + expect(await offlineDb.trips.get(tripId)).toBeDefined(); + }, + )); + + it('drops a trip that ended yesterday east of UTC', withClock( + 'Asia/Tokyo', + // 08:00 on the 23rd in Tokyo is still the 22nd in UTC. + '2026-08-22T23:00:00Z', + async () => { + const tripId = 111; + const trip = buildTrip({ id: tripId, end_date: '2026-08-22' }); + + let bundleCalled = false; + server.use( + http.get('/api/trips', () => HttpResponse.json({ trips: [trip] })), + http.get(`/api/trips/${tripId}/bundle`, () => { + bundleCalled = true; + return HttpResponse.json({}); + }), + ); + + await tripSyncManager.syncAll(); + expect(bundleCalled).toBe(false); + expect(await offlineDb.trips.get(tripId)).toBeUndefined(); + }, + )); +}); + +// ── logout mid-sync ──────────────────────────────────────────────────────────── + +describe('tripSyncManager.syncAll — logout while syncing', () => { + it('stops writing trips once the gate closes mid-loop', async () => { + const firstId = 500; + const secondId = 501; + const first = buildTrip({ id: firstId, end_date: dateOffset(5) }); + const second = buildTrip({ id: secondId, end_date: dateOffset(5) }); + + let secondBundleCalled = false; + server.use( + http.get('/api/trips', () => HttpResponse.json({ trips: [first, second] })), + http.get(`/api/trips/${firstId}/bundle`, () => { + // The user logs out while the first bundle is on the wire. + setAuthed(false); + return HttpResponse.json({ ...makeBundle(firstId), trip: first }); + }), + http.get(`/api/trips/${secondId}/bundle`, () => { + secondBundleCalled = true; + return HttpResponse.json({ ...makeBundle(secondId), trip: second }); + }), + ); + + await tripSyncManager.syncAll(); + + expect(secondBundleCalled).toBe(false); + expect(await offlineDb.trips.get(secondId)).toBeUndefined(); + }); +}); diff --git a/plugin-sdk/src/cli/dev.ts b/plugin-sdk/src/cli/dev.ts index 5189696b6..51716929b 100644 --- a/plugin-sdk/src/cli/dev.ts +++ b/plugin-sdk/src/cli/dev.ts @@ -456,7 +456,7 @@ export async function runDev(dir: string, opts: { port?: number } = {}): Promise // renders into docs/screenshot.png. ?theme=dark picks the dark palette. const chrome = url.searchParams.get('chrome') !== '0'; const theme = url.searchParams.get('theme') === 'dark' ? 'dark' : 'light'; - return send(200, preview(id, String(manifest.type), previewTripId, { chrome, theme }), 'text/html; charset=utf-8'); + return send(200, preview(id, String(manifest.type), previewTripId, { chrome, theme, geoAllowed: grants.has('geolocation:read') }), 'text/html; charset=utf-8'); } // Static plugin UI at /ui (page/widget client bundle). The iframe loads @@ -598,7 +598,7 @@ ${LIVE_RELOAD}`; * toggle), proxies trek:invoke to the dev server's /api routes with the dev user, * and surfaces resize/notify/navigate. This is where the design kit renders themed. */ -function preview(id: string, type: string, tripId: number, opts: { chrome?: boolean; theme?: 'light' | 'dark' } = {}): string { +export function preview(id: string, type: string, tripId: number, opts: { chrome?: boolean; theme?: 'light' | 'dark'; geoAllowed?: boolean } = {}): string { const maxW = type === 'widget' ? '440px' : '1000px'; // `trek-plugin shot` renders this page into docs/screenshot.png — the image the plugin store // shows. The theme picker and the "runs sandboxed at an opaque origin" note are DEV furniture: @@ -666,6 +666,7 @@ function postCtx(){ if(f.contentWindow) f.contentWindow.postMessage(ctx(),"*"); var tt; function toast(msg){var el=document.getElementById("toast");el.textContent=msg;el.classList.add("on");clearTimeout(tt);tt=setTimeout(function(){el.classList.remove("on");},2200);} var SESSION_PREFIX="trek:plugin-session:1:"+encodeURIComponent(${JSON.stringify(id)})+":"; var SESSION_MAX_KEYS=32,SESSION_MAX_KEY_LENGTH=64,SESSION_MAX_VALUE_BYTES=1024; +var GEO_ALLOWED=${opts.geoAllowed ? 'true' : 'false'}; function sessionFail(m,code,message){f.contentWindow.postMessage({type:"trek:error",requestId:m.requestId,code:code,message:message},"*");} function sessionMessage(m){ try { @@ -715,7 +716,11 @@ window.addEventListener("message", function(ev){ } else if(m.type==="trek:geolocation"){ // Mirror the host's geolocation bridge with a fixed dev position, so - // trek.geolocation.* works in the preview without a permission prompt. + // trek.geolocation.* works in the preview without a browser prompt. The grant + // still gates it: without geolocation:read the host answers "forbidden" + // (PluginFrame.tsx), and a preview that answered anyway would green-light a + // plugin that production then refuses. + if(!GEO_ALLOWED){ f.contentWindow.postMessage({type:"trek:geolocation:result",requestId:m.requestId,error:"forbidden"},"*"); return; } var pos={lat:52.5163,lng:13.3777,accuracy:12,heading:null,speed:null,timestamp:Date.now()}; var act=m.action||"get"; if(act==="clear"){ clearInterval(geoTick); geoTick=null; f.contentWindow.postMessage({type:"trek:geolocation:result",requestId:m.requestId,cleared:true},"*"); } diff --git a/plugin-sdk/src/ui/kit.ts b/plugin-sdk/src/ui/kit.ts index ac09c6758..25b74a422 100644 --- a/plugin-sdk/src/ui/kit.ts +++ b/plugin-sdk/src/ui/kit.ts @@ -483,7 +483,12 @@ export const TREK_THEME_JS = `(function () { var seq = 0; var lastH = -1; - function send(msg) { try { window.parent.postMessage(msg, '*'); } catch (e) {} } + function send(msg) { try { window.parent.postMessage(msg, '*'); return true; } catch (e) { return false; } } + // postMessage throws synchronously on a value structured clone cannot carry (a + // function, a DOM node, a Proxy). Swallowing that left the request registered and + // the promise pending forever, so the same call rejects under the mock host and + // hangs in the real one. Drop the entry and reject with the host's own code. + function unsendable(bucket, id, code, message) { delete bucket[id]; var e = new Error(message); e.code = code; return e; } function setFlag(name, on) { if (on) { docEl.setAttribute(name, ''); } else { docEl.removeAttribute(name); } } function applyContext(m) { @@ -593,28 +598,28 @@ export const TREK_THEME_JS = `(function () { var id = 's' + (++seq); return new Promise(function (resolve, reject) { pending[id] = { resolve: resolve, reject: reject }; - send({ type: 'trek:session:get', requestId: id, key: key, scope: opts && opts.scope }); + if (!send({ type: 'trek:session:get', requestId: id, key: key, scope: opts && opts.scope })) { reject(unsendable(pending, id, 'SESSION_INVALID_VALUE', 'session value must be JSON-serialisable')); } }); }, set: function (key, value, opts) { var id = 's' + (++seq); return new Promise(function (resolve, reject) { pending[id] = { resolve: resolve, reject: reject }; - send({ type: 'trek:session:set', requestId: id, key: key, value: value, scope: opts && opts.scope }); + if (!send({ type: 'trek:session:set', requestId: id, key: key, value: value, scope: opts && opts.scope })) { reject(unsendable(pending, id, 'SESSION_INVALID_VALUE', 'session value must be JSON-serialisable')); } }); }, remove: function (key, opts) { var id = 's' + (++seq); return new Promise(function (resolve, reject) { pending[id] = { resolve: resolve, reject: reject }; - send({ type: 'trek:session:remove', requestId: id, key: key, scope: opts && opts.scope }); + if (!send({ type: 'trek:session:remove', requestId: id, key: key, scope: opts && opts.scope })) { reject(unsendable(pending, id, 'SESSION_INVALID_VALUE', 'session value must be JSON-serialisable')); } }); }, clear: function (opts) { var id = 's' + (++seq); return new Promise(function (resolve, reject) { pending[id] = { resolve: resolve, reject: reject }; - send({ type: 'trek:session:clear', requestId: id, scope: opts && opts.scope }); + if (!send({ type: 'trek:session:clear', requestId: id, scope: opts && opts.scope })) { reject(unsendable(pending, id, 'SESSION_INVALID_VALUE', 'session value must be JSON-serialisable')); } }); } }; @@ -641,7 +646,7 @@ export const TREK_THEME_JS = `(function () { var id = 'c' + (++seq); return new Promise(function (resolve) { pendingConfirms[id] = resolve; - send({ type: 'trek:confirm', requestId: id, title: opts.title, message: opts.message, confirmLabel: opts.confirmLabel, cancelLabel: opts.cancelLabel, danger: opts.danger }); + if (!send({ type: 'trek:confirm', requestId: id, title: opts.title, message: opts.message, confirmLabel: opts.confirmLabel, cancelLabel: opts.cancelLabel, danger: opts.danger })) { delete pendingConfirms[id]; resolve(false); } }); }, // Core-event names for the trip in view ({ event, tripId } only, no payloads) — @@ -655,7 +660,7 @@ export const TREK_THEME_JS = `(function () { var id = 'r' + (++seq); return new Promise(function (resolve, reject) { pending[id] = { resolve: resolve, reject: reject }; - send({ type: 'trek:invoke', requestId: id, sub: sub, method: opts.method, body: opts.body }); + if (!send({ type: 'trek:invoke', requestId: id, sub: sub, method: opts.method, body: opts.body })) { reject(unsendable(pending, id, 'error', 'invoke body must be JSON-serialisable')); } }); }, // Host-brokered browser position (needs the geolocation:read grant; the diff --git a/plugin-sdk/test/permissions-parity.test.ts b/plugin-sdk/test/permissions-parity.test.ts index fc7713d32..c01d5754d 100644 --- a/plugin-sdk/test/permissions-parity.test.ts +++ b/plugin-sdk/test/permissions-parity.test.ts @@ -15,26 +15,32 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { HOOK_PERMISSION } from '../src/permissions.js'; +import { PLUGIN_SESSION_MAX_KEYS, PLUGIN_SESSION_MAX_KEY_LENGTH, PLUGIN_SESSION_MAX_VALUE_BYTES } from '../src/index.js'; const here = path.dirname(fileURLToPath(import.meta.url)); const serverPlugins = path.resolve(here, '../../server/src/nest/plugins'); const envelopeFile = path.join(serverPlugins, 'protocol/envelope.ts'); const serverEgress = path.join(serverPlugins, 'runtime/egress-policy.ts'); const inMonorepo = fs.existsSync(envelopeFile) && fs.existsSync(serverEgress); +const hostFrame = path.resolve(here, '../../client/src/components/Plugins/PluginFrame.tsx'); describe.skipIf(!inMonorepo)('parity with the host', () => { - it('the generated host facts match the server's envelope.ts', () => { + it("the generated host facts match the server's envelope.ts", () => { // Not a regex scrape any more: server/scripts/gen-plugin-facts.ts imports envelope.ts // and writes src/generated/host-facts.ts, and its --check mode is a CI gate. This // asserts the checked-in artefact is the one the host would produce, so a standalone // publish (which has no server present) still ships the right list. - const generated = fs.readFileSync(path.resolve(here, '../src/generated/host-facts.ts'), 'utf8'); + // + // Compared as whole maps, both ways round: asserting the artefact merely CONTAINS + // each entry passed when the host grew a hook the artefact never learned about, + // which is the drift that actually ships a wrong permission list. const envelope = fs.readFileSync(envelopeFile, 'utf8'); + const block = envelope.match(/HOOK_PERMISSION[^{]*\{([\s\S]*?)\n\}/); + expect(block, 'HOOK_PERMISSION not found in envelope.ts').toBeTruthy(); + const theirs: Record = {}; + for (const m of block![1].matchAll(/^\s*(\w+):\s*'([^']+)'/gm)) theirs[m[1]] = m[2]; - for (const [hook, perm] of Object.entries(HOOK_PERMISSION)) { - expect(generated, `${hook} missing from the generated facts`).toContain(`${hook}: '${perm}'`); - expect(envelope, `${hook} missing from envelope.ts`).toContain(`${hook}: '${perm}'`); - } + expect(theirs).toEqual({ ...HOOK_PERMISSION }); expect(Object.keys(HOOK_PERMISSION).length).toBeGreaterThan(0); }); @@ -55,3 +61,28 @@ describe.skipIf(!inMonorepo)('parity with the host', () => { for (const fn of fns) expect(body(ours, fn), `${fn} has drifted from the server`).toBe(body(theirs, fn)); }); }); + +describe.skipIf(!fs.existsSync(hostFrame))('parity with the host frame', () => { + it('the session storage contract is one contract in all three copies', () => { + // Nothing generates these: the limits are written out by hand in the SDK export + // authors import, in the dev preview's inlined host, and in the real frame, because + // the SDK ships standalone and cannot import across the package boundary. Bump one + // and the preview happily stores what production then rejects. + const frame = fs.readFileSync(hostFrame, 'utf8'); + const devServer = fs.readFileSync(path.resolve(here, '../src/cli/dev.ts'), 'utf8'); + + expect(frame).toContain(`const PLUGIN_SESSION_MAX_KEYS = ${PLUGIN_SESSION_MAX_KEYS}`); + expect(frame).toContain(`const PLUGIN_SESSION_MAX_KEY_LENGTH = ${PLUGIN_SESSION_MAX_KEY_LENGTH}`); + expect(frame).toContain(`const PLUGIN_SESSION_MAX_VALUE_BYTES = ${PLUGIN_SESSION_MAX_VALUE_BYTES}`); + expect(devServer).toContain( + `SESSION_MAX_KEYS=${PLUGIN_SESSION_MAX_KEYS},SESSION_MAX_KEY_LENGTH=${PLUGIN_SESSION_MAX_KEY_LENGTH},SESSION_MAX_VALUE_BYTES=${PLUGIN_SESSION_MAX_VALUE_BYTES}`, + ); + + // Same for the codes a plugin branches on — a rename in one copy is a silent + // behaviour change for anyone who developed against the other. + for (const code of ['SESSION_INVALID_KEY', 'SESSION_INVALID_VALUE', 'SESSION_VALUE_TOO_LARGE', 'SESSION_KEY_LIMIT', 'NO_TRIP_CONTEXT', 'SESSION_STORAGE_ERROR']) { + expect(frame, `${code} missing from the host frame`).toContain(`'${code}'`); + expect(devServer, `${code} missing from the dev preview`).toContain(`"${code}"`); + } + }); +}); diff --git a/plugin-sdk/test/sdk.test.ts b/plugin-sdk/test/sdk.test.ts index 4103b745c..15b4d2832 100644 --- a/plugin-sdk/test/sdk.test.ts +++ b/plugin-sdk/test/sdk.test.ts @@ -669,6 +669,21 @@ describe('dev-server SDK injection', () => { }); }); +describe('dev-server preview', () => { + // The preview plays the host, so it has to refuse what the host refuses. A preview + // that answered trek:geolocation without the grant green-lights a plugin production + // then breaks — the one drift direction this package cannot afford. + it('gates the geolocation bridge on geolocation:read, like PluginFrame', async () => { + const { preview } = await import('../src/cli/dev.js'); + const without = preview('demo', 'widget', 1, {}); + expect(without).toContain('var GEO_ALLOWED=false;'); + expect(without).toContain('if(!GEO_ALLOWED){'); + expect(without).toContain('error:"forbidden"'); + + expect(preview('demo', 'widget', 1, { geoAllowed: true })).toContain('var GEO_ALLOWED=true;'); + }); +}); + describe('dev db bind shapes', () => { let tmp: string; beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'devdb-')); }); diff --git a/plugin-sdk/test/ui-kit.test.ts b/plugin-sdk/test/ui-kit.test.ts index e329a4183..26a80fc88 100644 --- a/plugin-sdk/test/ui-kit.test.ts +++ b/plugin-sdk/test/ui-kit.test.ts @@ -70,3 +70,55 @@ describe('design kit', () => { expect(out.match(/