From 795947080259f971e19bb41c83385d8c862a488e Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 23 Aug 2026 10:07:56 +0200 Subject: [PATCH] fix(collab): scope the realtime handlers to the trip, and stop losing writes quietly The notes and polls WebSocket handlers applied every event they saw without checking which trip it belonged to, so an event for another trip the user is a member of landed in the open one. The poll load effect had no cancellation either, so switching trips fast let the old response win. What's Next compared trip-local calendar dates against a UTC one while building the time from the local clock. West of UTC that means the rest of today disappears from the widget every evening and tomorrow's items are labelled Today; east of UTC it happens after local midnight instead. A note website was rendered as a raw href with no scheme check, the same stored-javascript: shape the planner had. Category rename walked the notes in a loop with no handling for a partial failure. And the school-holiday region labels resolved German first for every locale. --- .../Collab/CollabChatLinkPreview.tsx | 23 +++++-- .../Collab/CollabChatSmall.test.tsx | 55 ++++++++++++++-- .../components/Collab/CollabNotes.test.tsx | 62 ++++++++++++++++-- client/src/components/Collab/CollabNotes.tsx | 41 ++++++++---- .../Collab/CollabNotesWebsiteThumbnail.tsx | Bin 2651 -> 3106 bytes .../components/Collab/CollabPolls.test.tsx | 46 ++++++++++--- client/src/components/Collab/CollabPolls.tsx | 16 ++++- .../Collab/WhatsNextWidget.test.tsx | 37 +++++++++-- .../src/components/Collab/WhatsNextWidget.tsx | 12 ++-- client/src/components/Vacay/VacayStats.tsx | 2 +- .../components/Vacay/holidayRegions.test.ts | 21 +++++- client/src/components/Vacay/holidayRegions.ts | 26 +++++--- 12 files changed, 283 insertions(+), 58 deletions(-) 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..5bd075343 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,56 @@ 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, and the modal still closes. + await waitFor(() => expect(puts).toEqual(['1', '2'])); + await waitFor(() => expect(addToast).toHaveBeenCalledWith('Error', 'error', undefined)); + await waitFor(() => expect(screen.queryByText('Manage Categories', { selector: 'h3' })).not.toBeInTheDocument()); + // Re-read: the note the server refused is still shown under its old category. + 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' })] }); diff --git a/client/src/components/Collab/CollabNotes.tsx b/client/src/components/Collab/CollabNotes.tsx index bf713d00a..a5a948559 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 @@ -161,16 +164,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. handleUpdateNote already toasts each failure. + 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 +194,17 @@ 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 }) } catch { failed++ } + } + if (failed > 0) await resyncNotes() + }, [notes, handleUpdateNote, resyncNotes]) const handleEditSubmit = useCallback(async (data) => { if (!editingNote) return @@ -232,7 +257,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 +487,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 +536,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/CollabNotesWebsiteThumbnail.tsx b/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx index 92e440a29e10dfbfc94836f177d0f410f1d0307a..4a8e648323f06a6acf8321a54601aa4c97adfd51 100644 GIT binary patch delta 674 zcmZXQ!Dn4;LaBi-@r37n2j(nbLKziJO6)v?tQ!e_4B?;Gh+!$I5CwkiLz6h zpup6c6y4!4SUxUke&`*HRC#tH~SoXjP(A&JOnvLYgoSV#whOlS2#C$62s;iN(U8@=E}iXHB%bI^wgwM6GKvIH1A1Nv~aAfB-oN1 z;#i~-R*LvhCTNaXE^J&**0?MYkz}~bWW*MbJL#TQ;BuO6sU+cP9ZTy5P=vGG5!eE! zV%v$*iy@OZ9{-+OImNiXQc$0%m)peeZ+C91DS8jfhCbscw|f0z`)aE>sD}LY&iP)= zDAyj-1@9hX8TfVMai_u_p_>_{N!l(B(T(L)@{z?1SY8jhUC|y(nOQFkzp44q&`Rh17NE{8T0+*NI3lw}vjx=6w(iM~HyZE<(seBgyym5D@jO*VuPTHI1 Tb?@fx*1fW&ESCnI`A4NN%3XQpY%1k*!>^D7B=tC{ICyO92RM5*0FvQqyd!ON(+Q zFXjqis|E{CUc+X>pl9>)sOT)T;zmvOQk zhy3J7juuIfh^B%S)L;YUG ({ 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..f7e693657 100644 --- a/client/src/components/Collab/WhatsNextWidget.tsx +++ b/client/src/components/Collab/WhatsNextWidget.tsx @@ -17,11 +17,15 @@ function formatTime(timeStr, is12h) { return `${String(h).padStart(2, '0')}:${String(m).padStart(2, '0')}` } +const localIsoDate = (d) => + `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}` + 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 = localIsoDate(now) + const tomorrowDate = localIsoDate(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 +51,7 @@ export default function WhatsNextWidget({ tripMembers = [] }: WhatsNextWidgetPro const upcoming = useMemo(() => { const now = new Date() - const nowDate = now.toISOString().split('T')[0] + const nowDate = localIsoDate(now) const nowTime = `${String(now.getHours()).padStart(2, '0')}:${String(now.getMinutes()).padStart(2, '0')}` const items = [] 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 {