From 7c6a17c0200fdeba8ac0792e8c915c44f18462d3 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 23 Aug 2026 11:23:09 +0200 Subject: [PATCH] fix(client): make the new rollbacks roll back only what they wrote The rollbacks this branch added were snapshot-shaped: capture the whole collection before the write, restore it on any failure. That is wrong in two ways at once. It undoes work the server already accepted when a later step is the one that fails, and it throws away whatever a collaborator's socket event applied while the requests were in flight. The day-plan reorder is the clearest case: five awaits, and a failing day-note write reverted transport positions the server had already stored. It now records only the fields it is about to overwrite, per booking, and drops each one as its request lands - so a note failure reaches the catch with nothing to undo, and a booking that arrived mid-write survives. The notification toggle had the same shape underneath: the rollback was made surgical but the request still shipped the whole matrix built from the pre-toggle render, so two overlapping toggles overwrote each other anyway. It sends the one cell that changed. The test pinned the disagreement as correct; it asserts the payload now. Photo reorder awaits allSettled rather than all, so the UI is not snapped back while the remaining writes are still landing - a partial run really does leave the server holding part of the new order, and only a total refusal justifies restoring the old one. Both the desktop editor and the mobile sheet, which was still swallowing every rejection. Also: the geolocation start guard is a run generation now, because a start issued after a stop re-sets a plain flag while the first run is still parked in the iOS permission prompt; a category rename reports once for the batch and rethrows, so the settings dialog stops closing as if it had worked; and a 404'd import job stops telling the user their file was invalid when it had simply expired. The clipboard fallback for plain-HTTP installs was pasted into three files by this branch. It is one helper now, and the two copy buttons in the members modal that still called navigator.clipboard unguarded use it too. Same for the fourth local-date helper written next to the three that already import the existing one. --- client/src/App.test.tsx | 16 +++-- .../BackgroundTasksWidget.extra.test.tsx | 3 + .../BackgroundTasks/BackgroundTasksWidget.tsx | 3 +- .../src/components/Budget/CostsPanel.test.tsx | 32 ++++++++++ client/src/components/Budget/CostsPanel.tsx | 13 +++- .../components/Collab/CollabNotes.test.tsx | 39 +++++++++++- client/src/components/Collab/CollabNotes.tsx | 19 ++++-- .../CollabNotesCategorySettingsModal.tsx | 11 +++- .../src/components/Collab/WhatsNextWidget.tsx | 10 ++- .../JourneyDetailPageEntryEditor.test.tsx | 20 ++++++ .../Journey/JourneyDetailPageEntryEditor.tsx | 19 ++++-- .../Journey/JourneyShareSection.tsx | 22 ++----- .../Planner/DayPlanSidebar.test.tsx | 55 ++++++++++++++++ .../src/components/Planner/DayPlanSidebar.tsx | 33 ++++++++-- .../Settings/NotificationsTab.test.tsx | 9 ++- .../components/Settings/NotificationsTab.tsx | 14 +++-- .../Trips/TripMembersModal.test.tsx | 53 +++++++++++++--- .../src/components/Trips/TripMembersModal.tsx | 18 +++--- client/src/hooks/useGeolocation.test.ts | 35 ++++++++++- client/src/hooks/useGeolocation.ts | 9 +++ .../mobile/screens/journey/MJourneyDetail.tsx | 13 ++-- .../screens/journey/MJourneyEntrySheet.tsx | 12 +++- .../screens/journey/MJourneySettingsSheet.tsx | 22 ++----- client/src/utils/clipboard.test.ts | 63 +++++++++++++++++++ client/src/utils/clipboard.ts | 35 +++++++++++ .../mobile/journey/MJourneyDetail.test.tsx | 42 ++++++------- .../journey/MJourneyEntrySheet.test.tsx | 32 ++++++++-- 27 files changed, 522 insertions(+), 130 deletions(-) create mode 100644 client/src/utils/clipboard.test.ts create mode 100644 client/src/utils/clipboard.ts 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']); });