diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index 76fcddfaf..ff9c2fe90 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -155,12 +155,16 @@ describe('RootRedirect — startup destination', () => { const onLine = Object.getOwnPropertyDescriptor(Navigator.prototype, 'onLine') Object.defineProperty(navigator, 'onLine', { value: false, configurable: true }) - renderApp('/') - await waitFor(() => expect(screen.getByText('TripPlanner')).toBeInTheDocument()) - - if (onLine) Object.defineProperty(Navigator.prototype, 'onLine', onLine) - delete (navigator as unknown as { onLine?: boolean }).onLine - await offlineDb.trips.clear() + // 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 diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx index 626a97794..c8ad05d7b 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx @@ -197,6 +197,9 @@ describe('BackgroundTasksWidget — poll backstop', () => { 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 () => { diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx index 6101c4e81..19c34b2a1 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx @@ -114,7 +114,8 @@ export default function BackgroundTasksWidget() { .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. - if (err?.response?.status === 404) setError(task.id, task.tripId, t('reservations.import.error')) + // 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) diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index 04f719807..3a5a8cff8 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -1438,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 diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index fa9b7c097..d1badfa7a 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -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' @@ -568,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 (
@@ -702,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 @@ -1038,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)) : '' diff --git a/client/src/components/Collab/CollabNotes.test.tsx b/client/src/components/Collab/CollabNotes.test.tsx index 5bd075343..dcd9f9bbd 100644 --- a/client/src/components/Collab/CollabNotes.test.tsx +++ b/client/src/components/Collab/CollabNotes.test.tsx @@ -1846,11 +1846,14 @@ describe('CollabNotes details', () => { 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. + // 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)); - await waitFor(() => expect(screen.queryByText('Manage Categories', { selector: 'h3' })).not.toBeInTheDocument()); + 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); }); @@ -1871,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 a5a948559..db3a7b871 100644 --- a/client/src/components/Collab/CollabNotes.tsx +++ b/client/src/components/Collab/CollabNotes.tsx @@ -148,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 @@ -166,7 +167,7 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { // 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. + // change the server never took. Reporting the failure is the caller's job. const resyncNotes = useCallback(async () => { try { const fresh = await collabApi.getNotes(tripId) @@ -201,10 +202,16 @@ function useCollabNotes({ tripId, currentUser }: CollabNotesProps) { 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++ } + try { await handleUpdateNote(n.id, { category: newName }, { silent: true }) } catch { failed++ } } - if (failed > 0) await resyncNotes() - }, [notes, handleUpdateNote, resyncNotes]) + // 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 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/WhatsNextWidget.tsx b/client/src/components/Collab/WhatsNextWidget.tsx index f7e693657..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 '' @@ -17,15 +18,12 @@ 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() // 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)) + 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' @@ -51,7 +49,7 @@ export default function WhatsNextWidget({ tripMembers = [] }: WhatsNextWidgetPro const upcoming = useMemo(() => { const now = new Date() - const nowDate = localIsoDate(now) + 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/Journey/JourneyDetailPageEntryEditor.test.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx index 725483433..cf51a6ba8 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.test.tsx @@ -603,6 +603,26 @@ describe('EntryEditor', () => { 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']) + }) + it('FE-JRN-EDITOR-030: dropping an unsaved gallery pick cancels its link', async () => { const linked: string[] = [] server.use(http.post('/api/journeys/entries/55/link-photo', async ({ request }) => { diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx index 8c6e15bdb..0608df282 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx @@ -450,12 +450,19 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, next.unshift(moved) setPhotos(next) // The order is shared: other members, the share view and the - // PDF all read it, so a rejected write has to go back. - Promise.all(next.map((ph, i) => journeyApi.updatePhoto(ph.id, { sort_order: i }))) - .catch(err => { - setPhotos(prevOrder) - toast.error(getApiErrorMessage(err, t('common.error'))) - }) + // PDF all read it, so a rejected write has to go back. Every + // write is awaited first — snapping back on the first + // rejection would do it while the rest are still landing. + void (async () => { + const results = await Promise.allSettled(next.map((ph, i) => journeyApi.updatePhoto(ph.id, { sort_order: i }))) + const rejected = results.filter(r => r.status === 'rejected') as PromiseRejectedResult[] + if (rejected.length === 0) return + toast.error(getApiErrorMessage(rejected[0].reason, t('common.error'))) + // Only a run where nothing landed can be put back: with a + // partial one the server already holds part of the new + // order, and hiding that would be the worse lie. + if (rejected.length === results.length) setPhotos(prevOrder) + })() }} className="absolute bottom-0.5 left-0.5 px-1.5 py-0.5 rounded bg-black/60 text-white text-[8px] font-semibold opacity-0 group-hover:opacity-100 transition-opacity" > diff --git a/client/src/components/Journey/JourneyShareSection.tsx b/client/src/components/Journey/JourneyShareSection.tsx index 2f808e1a7..d12ad9f2b 100644 --- a/client/src/components/Journey/JourneyShareSection.tsx +++ b/client/src/components/Journey/JourneyShareSection.tsx @@ -3,6 +3,7 @@ import { Link, List, Grid, MapPin, Check } from 'lucide-react' import { journeyApi } from '../../api/client' import { useTranslation } from '../../i18n' import { useToast } from '../shared/Toast' +import { copyText } from '../../utils/clipboard' export default function JourneyShareSection({ journeyId }: { journeyId: number }) { const { t } = useTranslation() @@ -53,24 +54,9 @@ export default function JourneyShareSection({ journeyId }: { journeyId: number } const shareUrl = link ? `${window.location.origin}/public/journey/${link.token}` : '' const copyLink = async () => { - try { - if (navigator.clipboard && window.isSecureContext) { - await navigator.clipboard.writeText(shareUrl) - } else { - // Fallback for non-secure contexts (plain HTTP) where navigator.clipboard is unavailable - const ta = document.createElement('textarea') - ta.value = shareUrl - ta.style.position = 'fixed' - ta.style.left = '-9999px' - document.body.appendChild(ta) - ta.focus() - ta.select() - document.execCommand('copy') - document.body.removeChild(ta) - } - setCopied(true) - setTimeout(() => setCopied(false), 2000) - } catch { /* ignore */ } + if (!(await copyText(shareUrl))) return + setCopied(true) + setTimeout(() => setCopied(false), 2000) } if (loading || !manageable) return null diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 6228afcbc..782bb6d71 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -2735,6 +2735,61 @@ describe('DayPlanSidebar', () => { expect(stored.day_positions).toBeUndefined() }) + it('FE-PLANNER-DAYPLAN-207: a rejected note write leaves the stored booking slot alone', async () => { + const updateDayNote = vi.fn().mockRejectedValue(new Error('note write failed')) + stubTripActions({ updateDayNote }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const placeA = buildPlace({ id: 1, name: 'Place A' }) + const placeB = buildPlace({ id: 2, name: 'Place B' }) + const a1 = buildAssignment({ id: 11, day_id: 10, order_index: 0, place: placeA }) + const a2 = buildAssignment({ id: 12, day_id: 10, order_index: 1, place: placeB }) + const note = buildDayNote({ id: 70, day_id: 10, text: 'Pack sunscreen', sort_order: 1.4 }) + const bus = buildReservation({ id: 430, type: 'bus', title: 'Airport bus', day_id: 10, day_plan_position: 1.5 }) + mockDayNotesState.dayNotes = { '10': [note] } + seedStore(useTripStore, { reservations: [bus] }) + render( undefined), + })} />) + fireEvent.dragStart(dragRow(screen.getByText('Place B')), { dataTransfer: emptyDataTransfer }) + fireEvent.drop(dragRow(screen.getByText('Place A')), { dataTransfer: { getData: vi.fn(() => '') } }) + + await waitFor(() => expect(mockToast.error).toHaveBeenCalledWith('note write failed')) + // The slot write went through before the note failed, so the day must keep it. + const stored = useTripStore.getState().reservations.find(r => r.id === 430)! + expect(stored.day_positions).toEqual({ 10: expect.any(Number) }) + expect(stored.day_plan_position).not.toBe(1.5) + }) + + it('FE-PLANNER-DAYPLAN-208: a booking that arrived mid-write survives the rollback', async () => { + const { reservationsApi } = await import('../../api/client') + const ferry = buildReservation({ id: 431, type: 'ferry', title: 'Island ferry', day_id: 10 }) + vi.mocked(reservationsApi.updatePositions).mockImplementationOnce(async () => { + // Stands in for a collaborator's reservation:created event landing while the + // slot write is still out. + useTripStore.setState(state => ({ reservations: [...state.reservations, ferry] })) + throw new Error('server said no') + }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const placeA = buildPlace({ id: 1, name: 'Place A' }) + const placeB = buildPlace({ id: 2, name: 'Place B' }) + const a1 = buildAssignment({ id: 11, day_id: 10, order_index: 0, place: placeA }) + const a2 = buildAssignment({ id: 12, day_id: 10, order_index: 1, place: placeB }) + const bus = buildReservation({ id: 430, type: 'bus', title: 'Airport bus', day_id: 10, day_plan_position: 1.5 }) + seedStore(useTripStore, { reservations: [bus] }) + render( undefined), + })} />) + fireEvent.dragStart(dragRow(screen.getByText('Place B')), { dataTransfer: emptyDataTransfer }) + fireEvent.drop(dragRow(screen.getByText('Place A')), { dataTransfer: { getData: vi.fn(() => '') } }) + + await waitFor(() => expect(mockToast.error).toHaveBeenCalledWith('server said no')) + const stored = useTripStore.getState().reservations + expect(stored.find(r => r.id === 430)!.day_plan_position).toBe(1.5) + expect(stored.find(r => r.id === 431)).toBeDefined() + }) + it('FE-PLANNER-DAYPLAN-126: reordering around a multi-leg flight writes a position per leg', async () => { const updateReservation = vi.fn(async () => ({} as Reservation)) stubTripActions({ updateReservation }) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 1ccfac070..768af3397 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -689,9 +689,25 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const applyMergedOrder = async (dayId: number, newOrder: { type: string; data: any }[]) => { // Capture previous place order for undo const prevAssignmentIds = getDayAssignments(dayId).map(a => a.id) - // …and the reservations as they stand, so a failed write can put the visible - // order back instead of leaving a phantom one behind the error toast. - const prevReservations = useTripStore.getState().reservations + // …and, per booking, the fields this call is about to overwrite, so a failed write + // can put the visible order back instead of leaving a phantom one behind the error + // toast. Restoring the whole array instead would also drop what a collaborator's + // socket event applied while the requests were in flight. + const pendingRollback: (Partial & { id: number })[] = [] + const dropRollback = (id: number) => { + const i = pendingRollback.findIndex(p => p.id === id) + if (i >= 0) pendingRollback.splice(i, 1) + } + const rollBackReservations = () => { + if (!pendingRollback.length) return + useTripStore.setState(state => ({ + reservations: state.reservations.map(r => { + const p = pendingRollback.find(x => x.id === r.id) + return p ? { ...r, ...p } : r + }) + })) + setTransportPosVersion(v => v + 1) + } // Places get sequential integer positions (0, 1, 2, ...) // Non-place items between place N-1 and place N get fractional positions @@ -733,6 +749,10 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // Update transport positions in store FIRST so the useEffect triggered by // onReorder's optimistic assignment update reads the correct positions. if (transportUpdates.length) { + for (const tu of transportUpdates) { + const r = useTripStore.getState().reservations.find(x => x.id === tu.id) + if (r) pendingRollback.push({ id: r.id, day_plan_position: r.day_plan_position, day_positions: r.day_positions }) + } useTripStore.setState(state => ({ reservations: state.reservations.map(r => { const tu = transportUpdates.find(u => u.id === r.id) @@ -762,8 +782,11 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // here double-encodes it on the server, which wipes metadata.legs on read // and collapses the flight back to a single span. const newMeta = { ...parsed, legs } + pendingRollback.push({ id: rid, metadata: r.metadata }) useTripStore.setState(state => ({ reservations: state.reservations.map(x => (x.id === rid ? { ...x, metadata: newMeta } : x)) })) await tripActions.updateReservation(tripId, rid, { metadata: newMeta }) + // Stored, so a later step failing must not put the old legs back. + dropRollback(rid) } setTransportPosVersion(v => v + 1) } @@ -771,6 +794,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { if (transportUpdates.length) { onRouteRefresh?.() await reservationsApi.updatePositions(tripId, transportUpdates, dayId) + for (const tu of transportUpdates) dropRollback(tu.id) } for (const n of noteUpdates) { await tripActions.updateDayNote(tripId, dayId, n.id, { sort_order: n.sort_order }) @@ -783,8 +807,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { }) } } catch (err: unknown) { - useTripStore.setState({ reservations: prevReservations }) - setTransportPosVersion(v => v + 1) + rollBackReservations() toast.error(err instanceof Error ? err.message : t('common.unknownError')) } } diff --git a/client/src/components/Settings/NotificationsTab.test.tsx b/client/src/components/Settings/NotificationsTab.test.tsx index c2efc0116..b234b318a 100644 --- a/client/src/components/Settings/NotificationsTab.test.tsx +++ b/client/src/components/Settings/NotificationsTab.test.tsx @@ -772,12 +772,13 @@ describe('NotificationsTab', () => { 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>; - // The first write is the email flip; in-app is still untouched there. - if (body.trip_invite.inapp === true) { + 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); }); @@ -795,7 +796,11 @@ describe('NotificationsTab', () => { // 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(); diff --git a/client/src/components/Settings/NotificationsTab.tsx b/client/src/components/Settings/NotificationsTab.tsx index 094adfa4b..f59e60895 100644 --- a/client/src/components/Settings/NotificationsTab.tsx +++ b/client/src/components/Settings/NotificationsTab.tsx @@ -116,14 +116,16 @@ 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 { // Only this cell rolls back — restoring the whole snapshot would also undo a // toggle the user made while this request was in flight. 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/hooks/useGeolocation.test.ts b/client/src/hooks/useGeolocation.test.ts index 1f9bc118a..ba536744c 100644 --- a/client/src/hooks/useGeolocation.test.ts +++ b/client/src/hooks/useGeolocation.test.ts @@ -1,4 +1,4 @@ -// 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'; @@ -272,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 565699208..671a1f404 100644 --- a/client/src/hooks/useGeolocation.ts +++ b/client/src/hooks/useGeolocation.ts @@ -102,11 +102,16 @@ export function useGeolocation(): UseGeolocationReturn { 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 @@ -129,6 +134,7 @@ export function useGeolocation(): UseGeolocationReturn { // 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 @@ -142,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 diff --git a/client/src/mobile/screens/journey/MJourneyDetail.tsx b/client/src/mobile/screens/journey/MJourneyDetail.tsx index a98175d3c..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' @@ -136,7 +136,6 @@ export default function MJourneyDetail() { useEffect(() => { let active = true - const ac = new AbortController() ;(async () => { try { const addonsData = await addonsApi.enabled() @@ -145,15 +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', signal: ac.signal }) - 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 */ } } if (active) setAvailableProviders(connected) } catch { /* no providers */ } })() - return () => { active = false; ac.abort() } + 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 9692cb6ab..9a049ef48 100644 --- a/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx +++ b/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx @@ -543,11 +543,21 @@ export default function MJourneyEntrySheet({ onClick={() => { // The PATCHes stay outside the updater — StrictMode invokes it // twice in dev and would send the whole batch a second time. + const prevOrder = photos const next = [...photos] const [moved] = next.splice(idx, 1) next.unshift(moved) setPhotos(next) - next.forEach((ph, i) => { journeyApi.updatePhoto(ph.id, { sort_order: i }).catch(() => {}) }) + // Same as the desktop editor: the order is shared with the other + // members, the share view and the PDF, so a run the server refused + // outright goes back instead of passing for saved. + void (async () => { + const results = await Promise.allSettled(next.map((ph, i) => journeyApi.updatePhoto(ph.id, { sort_order: i }))) + const rejected = results.filter(r => r.status === 'rejected') as PromiseRejectedResult[] + if (rejected.length === 0) return + toast.error(getApiErrorMessage(rejected[0].reason, t('common.error'))) + if (rejected.length === results.length) setPhotos(prevOrder) + })() }} className="absolute bottom-[3px] left-[3px] rounded-full bg-black/60 px-[6px] py-[1px] font-geist text-[0.5rem] font-bold text-white" > diff --git a/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx b/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx index 4049ee80d..f68398743 100644 --- a/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx +++ b/client/src/mobile/screens/journey/MJourneySettingsSheet.tsx @@ -14,6 +14,7 @@ import { journeyApi } from '../../../api/client' import { useJourneyStore } from '../../../store/journeyStore' import type { JourneyDetail } from '../../../store/journeyStore' import { normalizeImageFile } from '../../../utils/convertHeic' +import { copyText } from '../../../utils/clipboard' import { pickGradient } from '../../../pages/journeyDetail/JourneyDetailPage.helpers' import { journeyCoverSrc } from './mobileJourneyMeta' @@ -132,24 +133,9 @@ export default function MJourneySettingsSheet({ } const copyShareUrl = async () => { - try { - if (navigator.clipboard && window.isSecureContext) { - await navigator.clipboard.writeText(shareUrl) - } else { - // Fallback for non-secure contexts (plain HTTP) where navigator.clipboard is unavailable - const ta = document.createElement('textarea') - ta.value = shareUrl - ta.style.position = 'fixed' - ta.style.left = '-9999px' - document.body.appendChild(ta) - ta.focus() - ta.select() - document.execCommand('copy') - document.body.removeChild(ta) - } - setCopied(true) - setTimeout(() => setCopied(false), 2000) - } catch { /* ignore */ } + if (!(await copyText(shareUrl))) return + setCopied(true) + setTimeout(() => setCopied(false), 2000) } const toggleSharePerm = async (key: 'share_timeline' | 'share_gallery' | 'share_map') => { 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/tests/unit/mobile/journey/MJourneyDetail.test.tsx b/client/tests/unit/mobile/journey/MJourneyDetail.test.tsx index c653a6812..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', @@ -630,25 +631,24 @@ describe('MJourneyDetail', () => { }); // Leaving the screen mid-probe used to leave the requests running and land a - // setState on a gone component. - it('FE-MOB-JDET-038: leaving the screen aborts the provider probes', async () => { + // 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 }], + addons: [ + { id: 'immich', name: 'Immich', type: 'photo_provider', enabled: true }, + { id: 'synology', name: 'Synology', type: 'photo_provider', enabled: true }, + ], }); - let signal: AbortSignal | undefined; - vi.stubGlobal('fetch', vi.fn(async (_url: string, init?: RequestInit) => { - signal = init?.signal ?? undefined; - return new Promise((_resolve, reject) => { - init?.signal?.addEventListener('abort', () => reject(new Error('aborted'))); - }); - })); + let release: (value: { connected: boolean }) => void = () => {}; + vi.mocked(memoriesApi.status).mockReturnValue(new Promise(resolve => { release = resolve; })); const { unmount } = setup({ view: 'gallery' }); - await waitFor(() => expect(signal).toBeDefined()); - expect(signal!.aborted).toBe(false); + await waitFor(() => expect(memoriesApi.status).toHaveBeenCalledTimes(1)); unmount(); - expect(signal!.aborted).toBe(true); + await act(async () => { release({ connected: true }); }); + expect(memoriesApi.status).toHaveBeenCalledTimes(1); }); it('FE-MOB-JDET-037: the screen measures itself, not the shell', () => { diff --git a/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx b/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx index 94b5fedc7..361fde069 100644 --- a/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx +++ b/client/tests/unit/mobile/journey/MJourneyEntrySheet.test.tsx @@ -687,6 +687,23 @@ describe('MJourneyEntrySheet full editor', () => { 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' })); @@ -783,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']); });