diff --git a/client/src/components/Collab/CollabNotes.tsx b/client/src/components/Collab/CollabNotes.tsx index 31331097b..a4a563bbe 100644 --- a/client/src/components/Collab/CollabNotes.tsx +++ b/client/src/components/Collab/CollabNotes.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback, useId, useMemo } from 'react' +import { useState, useCallback, useId, useMemo } from 'react' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' @@ -10,13 +10,13 @@ import { DialogButton, DialogFooter, DialogHeader, DialogSection, DialogShell, D import { collabApi } from '../../api/client' import { useCanDo } from '../../store/permissionsStore' import { useTripStore } from '../../store/tripStore' -import { addListener, removeListener } from '../../api/websocket' import { useTranslation } from '../../i18n' import { useToast } from '../shared/Toast' import ConfirmDialog from '../shared/ConfirmDialog' import EmptyState from '../shared/EmptyState' import type { User } from '../../types' import type { CollabNote } from './CollabNotes.types' +import { useCollabNotesData } from './useCollabNotesData' import { FONT, NOTE_COLORS } from './CollabNotes.constants' import { NoteFormModal } from './CollabNotesFormModal' import { CategorySettingsModal } from './CollabNotesCategorySettingsModal' @@ -43,8 +43,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { const can = useCanDo() const trip = useTripStore((s) => s.trip) const canEdit = can('collab_edit', trip) - const [notes, setNotes] = useState([]) - const [loading, setLoading] = useState(true) + const { notes, setNotes, loading, uploadNoteFiles } = useCollabNotesData({ tripId, t, toast, resetOnLoadError: true }) const [showNewModal, setShowNewModal] = useState(false) const [editingNote, setEditingNote] = useState(null) const [viewingNote, setViewingNote] = useState(null) @@ -77,49 +76,6 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { return NOTE_COLORS[Object.keys(categoryColors).length % NOTE_COLORS.length].value } - // ── Load notes on mount ── - useEffect(() => { - if (!tripId) return - let cancelled = false - setLoading(true) - collabApi.getNotes(tripId) - .then(data => { if (!cancelled) setNotes(data?.notes || data || []) }) - .catch(() => { if (!cancelled) setNotes([]) }) - .finally(() => { if (!cancelled) setLoading(false) }) - return () => { cancelled = true } - }, [tripId]) - - // ── WebSocket real-time sync ── - useEffect(() => { - 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 - return [msg.note, ...prev] - }) - } - if (msg.type === 'collab:note:updated' && msg.note) { - setNotes(prev => - prev.map(n => (n.id === msg.note.id ? { ...n, ...msg.note } : n)) - ) - } - if (msg.type === 'collab:note:deleted') { - const deletedId = msg.noteId || msg.id - if (deletedId) { - setNotes(prev => prev.filter(n => n.id !== deletedId)) - } - } - } - - addListener(handler) - return () => removeListener(handler) - }, [tripId]) - // ── Actions ── const handleCreateNote = useCallback(async (data) => { const pendingFiles = data._pendingFiles || [] @@ -135,11 +91,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { const note = created.note || created // Upload pending files if (pendingFiles.length > 0 && note.id) { - for (const file of pendingFiles) { - const fd = new FormData() - fd.append('file', file) - try { await collabApi.uploadNoteFile(tripId, note.id, fd) } catch (err) { console.error('Failed to upload note attachment:', err); toast.error(t('common.error')) } - } + await uploadNoteFiles(note.id, pendingFiles, err => console.error('Failed to upload note attachment:', err)) // Reload note with attachments const fresh = await collabApi.getNotes(tripId) if (fresh?.notes) setNotes(fresh.notes) @@ -151,7 +103,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { return [note, ...prev] }) } - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes, uploadNoteFiles]) const handleUpdateNote = useCallback(async (noteId, data, opts: { silent?: boolean } = {}) => { let result @@ -168,7 +120,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { prev.map(n => (n.id === noteId ? { ...n, ...updated } : n)) ) } - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes]) // 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 @@ -178,7 +130,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { const fresh = await collabApi.getNotes(tripId) setNotes(fresh?.notes || fresh || []) } catch {} - }, [tripId]) + }, [tripId, setNotes]) const saveCategoryColors = useCallback(async (newMap) => { let failed = 0 @@ -224,16 +176,12 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { delete data._pendingFiles await handleUpdateNote(editingNote.id, data) if (pendingFiles.length > 0) { - for (const file of pendingFiles) { - const fd = new FormData() - fd.append('file', file) - try { await collabApi.uploadNoteFile(tripId, editingNote.id, fd) } catch { toast.error(t('common.error')) } - } + await uploadNoteFiles(editingNote.id, pendingFiles) const fresh = await collabApi.getNotes(tripId) if (fresh?.notes) setNotes(fresh.notes) window.dispatchEvent(new Event('collab-files-changed')) } - }, [editingNote, tripId, handleUpdateNote, toast, t]) + }, [editingNote, tripId, handleUpdateNote, setNotes, uploadNoteFiles]) const handleDeleteNoteFile = useCallback(async (noteId, fileId) => { try { await collabApi.deleteNoteFile(tripId, noteId, fileId) } catch { toast.error(t('common.error')) } @@ -249,7 +197,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { } setNotes(prev => prev.filter(n => n.id !== noteId)) window.dispatchEvent(new Event('collab-files-changed')) - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes]) // ── Derived data ── const categories = [...new Set(notes.map(n => n.category).filter(Boolean))] diff --git a/client/src/components/Collab/CollabNotes.types.ts b/client/src/components/Collab/CollabNotes.types.ts index c61f3d0bb..bf9c18f7a 100644 --- a/client/src/components/Collab/CollabNotes.types.ts +++ b/client/src/components/Collab/CollabNotes.types.ts @@ -21,6 +21,7 @@ export interface CollabNote { avatar: string | null user_id: number created_at: string + updated_at?: string author?: { username: string; avatar: string | null } user?: { username: string; avatar: string | null } files?: NoteFile[] diff --git a/client/src/components/Collab/useCollabNotesData.test.ts b/client/src/components/Collab/useCollabNotesData.test.ts new file mode 100644 index 000000000..04bfb8330 --- /dev/null +++ b/client/src/components/Collab/useCollabNotesData.test.ts @@ -0,0 +1,167 @@ +// FE-COLLAB-NOTESDATA-001 to FE-COLLAB-NOTESDATA-008: the note list behind the desktop +// Collab panel (empties on a failed load) and the phone's notes tab (keeps the list), with +// the live note events and the attachment upload both run after saving a note. +import { act, renderHook, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { collabApi } from '../../api/client'; +import { addListener, removeListener } from '../../api/websocket'; +import type { CollabNoteData } from './collabModel'; +import { useCollabNotesData, type CollabNotesDataOptions } from './useCollabNotesData'; + +const t = (key: string) => key; + +function note(id: number, over: Partial = {}): CollabNoteData { + return { + id, + trip_id: 1, + user_id: 2, + title: `Note ${id}`, + content: null, + category: null, + color: null, + website: null, + pinned: false, + username: 'alice', + avatar: null, + avatar_url: null, + created_at: '2026-05-01T10:00:00Z', + attachments: [], + ...over, + }; +} + +function setup(over: Partial = {}) { + const toast = { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() }; + const hook = renderHook( + (props: Partial) => useCollabNotesData({ tripId: 1, t, toast, ...over, ...props }), + { initialProps: {} } + ); + return { ...hook, toast }; +} + +async function loaded(over: Partial = {}) { + const view = setup(over); + await waitFor(() => expect(view.result.current.loading).toBe(false)); + return view; +} + +function wsHandler(): (event: Record) => void { + const calls = vi.mocked(addListener).mock.calls; + return calls[calls.length - 1][0] as (event: Record) => void; +} + +beforeEach(() => { + vi.mocked(addListener).mockClear(); + vi.mocked(removeListener).mockClear(); + vi.spyOn(collabApi, 'getNotes').mockResolvedValue({ notes: [note(1), note(2)] }); + vi.spyOn(collabApi, 'uploadNoteFile').mockResolvedValue({}); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('useCollabNotesData', () => { + it('FE-COLLAB-NOTESDATA-001: loads the notes, from an object or a bare array', async () => { + const { result } = setup(); + expect(result.current.loading).toBe(true); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(collabApi.getNotes).toHaveBeenCalledWith(1); + expect(result.current.notes.map((n) => n.id)).toEqual([1, 2]); + + vi.mocked(collabApi.getNotes).mockResolvedValue([note(3)]); + const bare = await loaded(); + expect(bare.result.current.notes.map((n) => n.id)).toEqual([3]); + + vi.mocked(collabApi.getNotes).mockResolvedValue({}); + const empty = await loaded(); + expect(empty.result.current.notes).toEqual([]); + }); + + it('FE-COLLAB-NOTESDATA-002: without a trip nothing loads or listens', () => { + const { result } = setup({ tripId: 0 }); + expect(collabApi.getNotes).not.toHaveBeenCalled(); + expect(addListener).not.toHaveBeenCalled(); + expect(result.current.loading).toBe(true); + }); + + it('FE-COLLAB-NOTESDATA-003: a failed reload empties the desktop list and keeps the phone list', async () => { + const desktop = await loaded({ resetOnLoadError: true }); + const phone = await loaded(); + vi.mocked(collabApi.getNotes).mockRejectedValue(new Error('boom')); + + desktop.rerender({ tripId: 2 }); + phone.rerender({ tripId: 2 }); + await waitFor(() => expect(desktop.result.current.loading).toBe(false)); + await waitFor(() => expect(phone.result.current.loading).toBe(false)); + expect(desktop.result.current.notes).toEqual([]); + expect(phone.result.current.notes.map((n) => n.id)).toEqual([1, 2]); + }); + + it('FE-COLLAB-NOTESDATA-004: live events create, update and delete, only for this trip', async () => { + const { result } = await loaded(); + const handler = wsHandler(); + act(() => { + handler({ type: 'collab:note:created', tripId: 2, note: note(7) }); + handler({ type: 'collab:note:created', tripId: '1', note: note(7) }); + handler({ type: 'collab:note:created', tripId: 1, note: note(7) }); + handler({ type: 'collab:note:updated', tripId: 1, note: { id: 1, title: 'renamed' } }); + handler({ type: 'collab:note:updated', tripId: 1 }); + }); + expect(result.current.notes.map((n) => n.id)).toEqual([7, 1, 2]); + // An update folds into the note it names. + expect(result.current.notes[1]).toMatchObject({ title: 'renamed', username: 'alice' }); + + act(() => { + handler({ type: 'collab:note:deleted', tripId: 1, noteId: 7 }); + handler({ type: 'collab:note:deleted', tripId: 1, id: 1 }); + handler({ type: 'collab:note:deleted', tripId: 1 }); + }); + expect(result.current.notes.map((n) => n.id)).toEqual([2]); + }); + + it('FE-COLLAB-NOTESDATA-005: unmounting removes the live listener', async () => { + const { unmount } = await loaded(); + const handler = wsHandler(); + unmount(); + expect(removeListener).toHaveBeenCalledWith(handler); + }); + + it('FE-COLLAB-NOTESDATA-006: attachments upload one by one as a file field', async () => { + const { result, toast } = await loaded(); + const a = new File(['a'], 'a.pdf'); + const b = new File(['b'], 'b.png'); + await act(async () => { + await result.current.uploadNoteFiles(5, [a, b]); + }); + const calls = vi.mocked(collabApi.uploadNoteFile).mock.calls; + expect(calls.map((c) => [c[0], c[1]])).toEqual([ + [1, 5], + [1, 5], + ]); + expect(((calls[0][2] as FormData).get('file') as File).name).toBe(a.name); + expect(((calls[1][2] as FormData).get('file') as File).name).toBe(b.name); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('FE-COLLAB-NOTESDATA-007: a refused file is reported and toasted, the rest still go', async () => { + const err = new Error('too big'); + vi.mocked(collabApi.uploadNoteFile).mockRejectedValueOnce(err); + const onFileError = vi.fn(); + const { result, toast } = await loaded(); + await act(async () => { + await result.current.uploadNoteFiles(5, [new File(['a'], 'a.pdf'), new File(['b'], 'b.pdf')], onFileError); + }); + expect(collabApi.uploadNoteFile).toHaveBeenCalledTimes(2); + expect(onFileError).toHaveBeenCalledWith(err); + expect(toast.error).toHaveBeenCalledTimes(1); + expect(toast.error).toHaveBeenCalledWith('common.error'); + }); + + it('FE-COLLAB-NOTESDATA-008: setNotes lets a shell write the list it reloaded', async () => { + const { result } = await loaded(); + act(() => result.current.setNotes([note(9)])); + expect(result.current.notes.map((n) => n.id)).toEqual([9]); + }); +}); diff --git a/client/src/components/Collab/useCollabNotesData.ts b/client/src/components/Collab/useCollabNotesData.ts new file mode 100644 index 000000000..19eddc006 --- /dev/null +++ b/client/src/components/Collab/useCollabNotesData.ts @@ -0,0 +1,95 @@ +import { useCallback, useEffect, useState } from 'react'; + +import { collabApi } from '../../api/client'; +import { addListener, removeListener } from '../../api/websocket'; +import type { useTranslation } from '../../i18n'; +import type { useToast } from '../shared/Toast'; +import type { CollabNoteData } from './collabModel'; + +export interface CollabNotesDataOptions { + tripId: number; + t: ReturnType['t']; + toast: ReturnType; + /** The desktop panel outlives a trip change, so a failed load there empties the list. */ + resetOnLoadError?: boolean; +} + +/** + * The trip notes behind the desktop Collab panel and the phone's notes tab, each reading + * the rows through its own note type: the list as the server has it, kept current by the + * WebSocket note events, and the attachment upload both run after a note is saved. Writing + * a note, its colours and its categories stays with each shell, which differ there. Collab + * has no store slice, so the notes live here. + */ +export function useCollabNotesData({ + tripId, + t, + toast, + resetOnLoadError = false, +}: CollabNotesDataOptions) { + const [notes, setNotes] = useState([]); + const [loading, setLoading] = useState(true); + + useEffect(() => { + if (!tripId) return; + let cancelled = false; + setLoading(true); + collabApi + .getNotes(tripId) + .then((data: N[] | { notes?: N[] } | null) => { + if (!cancelled) setNotes(Array.isArray(data) ? data : data?.notes || []); + }) + .catch(() => { + if (!cancelled && resetOnLoadError) setNotes([]); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [tripId, resetOnLoadError]); + + useEffect(() => { + if (!tripId) return; + const handler = (msg: Record) => { + // An event still in flight from a trip just left must not land in this list. + if (String(msg?.tripId) !== String(tripId)) return; + const note = msg.note as N | undefined; + if (msg.type === 'collab:note:created' && note) { + setNotes((prev) => (prev.some((n) => n.id === note.id) ? prev : [note, ...prev])); + } + if (msg.type === 'collab:note:updated' && note) { + setNotes((prev) => prev.map((n) => (n.id === note.id ? { ...n, ...note } : n))); + } + if (msg.type === 'collab:note:deleted') { + const deletedId = (msg.noteId || msg.id) as number | undefined; + if (deletedId) setNotes((prev) => prev.filter((n) => n.id !== deletedId)); + } + }; + addListener(handler); + return () => removeListener(handler); + }, [tripId]); + + /** + * Uploads a saved note's attachments one by one; a file the server refuses is toasted + * and the rest still go. `onFileError` sees each refusal before the toast. + */ + const uploadNoteFiles = useCallback( + async (noteId: number, files: File[], onFileError?: (err: unknown) => void) => { + for (const file of files) { + const fd = new FormData(); + fd.append('file', file); + try { + await collabApi.uploadNoteFile(tripId, noteId, fd); + } catch (err) { + onFileError?.(err); + toast.error(t('common.error')); + } + } + }, + [tripId, toast, t] + ); + + return { notes, setNotes, loading, uploadNoteFiles }; +} diff --git a/client/src/mobile/screens/trip/tabs/MCollabNotes.tsx b/client/src/mobile/screens/trip/tabs/MCollabNotes.tsx index 333f9a23b..c1a1d2c13 100644 --- a/client/src/mobile/screens/trip/tabs/MCollabNotes.tsx +++ b/client/src/mobile/screens/trip/tabs/MCollabNotes.tsx @@ -6,7 +6,7 @@ import { sanitizedMarkdownPlugins, sanitizedMarkdownComponents } from '../../../ import { Check, ExternalLink, FileText, Paperclip, Pin, PinOff, Plus, StickyNote, Trash2, X } from 'lucide-react' import MDancingTrek from '../../../components/MDancingTrek' import { collabApi } from '../../../../api/client' -import { addListener, removeListener } from '../../../../api/websocket' +import { useCollabNotesData } from '../../../../components/Collab/useCollabNotesData' import { openFile } from '../../../../utils/fileDownload' import { safeExternalHref } from '../../../../utils/safeUrl' import MSheet from '../../../components/MSheet' @@ -47,9 +47,9 @@ function linkHost(url: string): string { } /** - * Trip-tab Collab / Notes. Same architecture as MCollabChat: own state, own - * `collabApi` calls, own WebSocket listener — no `tripStore`/`tripActions` - * (10-tab-databindings.md §8.4). The demo only has a placeholder for this + * Trip-tab Collab / Notes. The list and its WebSocket listener come from + * useCollabNotesData, the hook the desktop panel reads too; no + * `tripStore`/`tripActions` (10-tab-databindings.md §8.4). The demo only has a placeholder for this * sub-tab, so the card/filter/form design below is new (spec 03 §6.4 audit * item), built in the same visual language as MTransportsTab. */ @@ -58,44 +58,12 @@ export default function MCollabNotes({ planner }: MCollabNotesProps) { const canEdit = planner.can('collab_edit', planner.trip) const canUploadFiles = planner.can('file_upload', planner.trip) - const [notes, setNotes] = useState([]) - const [loading, setLoading] = useState(true) + const { notes, setNotes, loading, uploadNoteFiles } = useCollabNotesData({ tripId, t, toast }) const [activeCategory, setActiveCategory] = useState(null) const [formTarget, setFormTarget] = useState(null) const [viewingNote, setViewingNote] = useState(null) const [pendingDeleteId, setPendingDeleteId] = useState(null) - // ── Load ── - useEffect(() => { - let cancelled = false - setLoading(true) - collabApi.getNotes(tripId).then((data: GetNotesResponse) => { - if (!cancelled) setNotes(data.notes || []) - }).catch(() => { /* leave notes empty */ }).finally(() => { if (!cancelled) setLoading(false) }) - return () => { cancelled = true } - }, [tripId]) - - // ── WebSocket (own listener, not handleRemoteEvent) ── - useEffect(() => { - const handler = (event: Record) => { - if (String(event.tripId) !== String(tripId)) return - if (event.type === 'collab:note:created') { - const note = event.note as CollabNoteData - setNotes(prev => (prev.some(n => n.id === note.id) ? prev : [note, ...prev])) - } - if (event.type === 'collab:note:updated') { - const note = event.note as CollabNoteData - setNotes(prev => prev.map(n => (n.id === note.id ? { ...n, ...note } : n))) - } - if (event.type === 'collab:note:deleted') { - const noteId = event.noteId as number - setNotes(prev => prev.filter(n => n.id !== noteId)) - } - } - addListener(handler) - return () => removeListener(handler) - }, [tripId]) - const categories = noteCategoriesList(notes) const colorMap = buildCategoryColorMap(notes) const sorted = sortNotes(notes, activeCategory) @@ -113,17 +81,13 @@ export default function MCollabNotes({ planner }: MCollabNotesProps) { throw new Error('create failed') } if (data.pendingFiles.length > 0) { - for (const file of data.pendingFiles) { - const fd = new FormData() - fd.append('file', file) - try { await collabApi.uploadNoteFile(tripId, created.id, fd) } catch { toast.error(t('common.error')) } - } + await uploadNoteFiles(created.id, data.pendingFiles) const fresh = (await collabApi.getNotes(tripId)) as GetNotesResponse setNotes(fresh.notes || []) return } setNotes(prev => (prev.some(n => n.id === created.id) ? prev : [created, ...prev])) - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes, uploadNoteFiles]) const handleUpdate = useCallback(async ( noteId: number, @@ -139,17 +103,13 @@ export default function MCollabNotes({ planner }: MCollabNotesProps) { throw new Error('update failed') } if (pendingFiles.length > 0) { - for (const file of pendingFiles) { - const fd = new FormData() - fd.append('file', file) - try { await collabApi.uploadNoteFile(tripId, noteId, fd) } catch { toast.error(t('common.error')) } - } + await uploadNoteFiles(noteId, pendingFiles) const fresh = (await collabApi.getNotes(tripId)) as GetNotesResponse setNotes(fresh.notes || []) return } if (updated) setNotes(prev => prev.map(n => (n.id === noteId ? { ...n, ...updated } : n))) - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes, uploadNoteFiles]) const handleDelete = useCallback(async (noteId: number) => { try { @@ -158,7 +118,7 @@ export default function MCollabNotes({ planner }: MCollabNotesProps) { } catch { toast.error(t('common.error')) } - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes]) // Returns whether the file is really gone — the form sheet only drops the // chip once the server confirmed it. @@ -173,7 +133,7 @@ export default function MCollabNotes({ planner }: MCollabNotesProps) { toast.error(t('common.error')) return false } - }, [tripId, toast, t]) + }, [tripId, toast, t, setNotes]) const openNote = (note: CollabNoteData) => (canEdit ? setFormTarget(note) : setViewingNote(note))