From 634dd411c540679cb6d4999832efad7c8adca3bb Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 10:05:57 +0200 Subject: [PATCH] refactor(collections): share the copy to trip picker between desktop and phone useTripCopyPicker loads the trips on every opening, filters them by the search, formats the date range and turns the copy result into toasts for both the dialog and the sheet. The dialog keeps its guard against an empty selection. --- .../Collections/CopyToTripModal.tsx | 71 +------- .../Collections/useTripCopyPicker.test.ts | 154 ++++++++++++++++++ .../Collections/useTripCopyPicker.ts | 101 ++++++++++++ .../collections/MCollTripPickerSheet.tsx | 64 +------- 4 files changed, 262 insertions(+), 128 deletions(-) create mode 100644 client/src/components/Collections/useTripCopyPicker.test.ts create mode 100644 client/src/components/Collections/useTripCopyPicker.ts diff --git a/client/src/components/Collections/CopyToTripModal.tsx b/client/src/components/Collections/CopyToTripModal.tsx index b5973748b..b7f941d0d 100644 --- a/client/src/components/Collections/CopyToTripModal.tsx +++ b/client/src/components/Collections/CopyToTripModal.tsx @@ -1,21 +1,9 @@ -import React, { useEffect, useId, useMemo, useState } from 'react' +import React, { useId } from 'react' import { Search, MapPin, Loader2, Copy, CalendarDays } from 'lucide-react' import { DialogButton, DialogFooter, DialogHeader, DialogShell, DialogTile, FooterSpacer, NEUTRAL_TINT, fs } from '../shared/DialogShell' import { INPUT } from '../shared/dialogParts' -import { useToast } from '../shared/Toast' -import { tripsApi } from '../../api/client' -import { getApiErrorMessage } from '../../utils/apiError' -import { formatDate } from '../../utils/formatters' -import { useTranslation } from '../../i18n' import type { TranslationFn } from '../../types' - -interface TripOption { - id: number - title: string - start_date?: string | null - end_date?: string | null - cover_image?: string | null -} +import { useTripCopyPicker } from './useTripCopyPicker' interface CopyToTripModalProps { isOpen: boolean @@ -34,63 +22,12 @@ interface CopyToTripModalProps { * place (detail panel) and bulk select-mode ("Copy N to trip"). */ export default function CopyToTripModal({ isOpen, onClose, placeIds, onCopy, t }: CopyToTripModalProps): React.ReactElement | null { - const toast = useToast() - const { language } = useTranslation() const labelId = useId() - const [trips, setTrips] = useState([]) - const [loading, setLoading] = useState(false) - const [search, setSearch] = useState('') - const [busyTripId, setBusyTripId] = useState(null) - - useEffect(() => { - if (!isOpen) return - let cancelled = false - setLoading(true) - setSearch('') - tripsApi.list() - .then((res: { trips?: TripOption[] }) => { if (!cancelled) setTrips(res.trips ?? []) }) - .catch(() => { if (!cancelled) setTrips([]) }) - .finally(() => { if (!cancelled) setLoading(false) }) - return () => { cancelled = true } - }, [isOpen]) - - const filtered = useMemo(() => { - const q = search.trim().toLowerCase() - if (!q) return trips - return trips.filter(tr => (tr.title ?? '').toLowerCase().includes(q)) - }, [trips, search]) - - const dateRange = (tr: TripOption): string => { - const s = formatDate(tr.start_date, language) - const e = formatDate(tr.end_date, language) - if (s && e) return `${s} – ${e}` - return s || e || '' - } + const { loading, search, setSearch, filtered, busyTripId, dateRange, handleCopy } = + useTripCopyPicker({ open: isOpen, onCopy, onClose, t, emptySelection: placeIds.length === 0 }) if (!isOpen) return null - const handleCopy = async (tripId: number) => { - if (busyTripId != null || placeIds.length === 0) return - setBusyTripId(tripId) - try { - const res = await onCopy(tripId) - if (res.copied > 0) { - toast.success(t('collections.copiedCount', { count: res.copied })) - } - if (res.skipped.length > 0) { - toast.info(t('collections.skippedDuplicates', { count: res.skipped.length })) - } - if (res.copied === 0 && res.skipped.length === 0) { - toast.info(t('collections.copyNothing')) - } - onClose() - } catch (err) { - toast.error(getApiErrorMessage(err, t('common.error'))) - } finally { - setBusyTripId(null) - } - } - return ( ({ + useTranslation: () => ({ language: 'en' }), +})); + +const t = (k: string, p?: Record) => (p ? `${k}:${JSON.stringify(p)}` : k); + +const PARIS: TripOption = { id: 1, title: 'Paris', start_date: '2031-05-01', end_date: '2031-05-04' }; +const ROME: TripOption = { id: 2, title: 'Rome', start_date: '2031-06-01', end_date: null }; +const OSLO: TripOption = { id: 3, title: 'Oslo' }; + +let addToast: Mock>; +let list: MockInstance; + +function setup(over: Partial = {}) { + const props: TripCopyPickerOptions = { + open: true, + onCopy: vi.fn().mockResolvedValue({ copied: 2, skipped: [] }), + onClose: vi.fn(), + t, + ...over, + }; + const hook = renderHook((p: TripCopyPickerOptions) => useTripCopyPicker(p), { initialProps: props }); + return { ...hook, props }; +} + +beforeEach(() => { + addToast = vi.fn>(); + window.__addToast = addToast; + list = vi.spyOn(tripsApi, 'list').mockResolvedValue({ trips: [PARIS, ROME, OSLO] }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + delete window.__addToast; +}); + +describe('filterTripOptions', () => { + it('FE-COMP-TRIPCOPYHOOK-001: matches the trimmed, case folded query against the title', () => { + expect(filterTripOptions([PARIS, ROME], ' ')).toEqual([PARIS, ROME]); + expect(filterTripOptions([PARIS, ROME], ' rO ')).toEqual([ROME]); + }); +}); + +describe('useTripCopyPicker', () => { + it('FE-COMP-TRIPCOPYHOOK-002: loads the trips on open and narrows them by the search', async () => { + const { result } = setup(); + expect(result.current.loading).toBe(true); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.filtered).toEqual([PARIS, ROME, OSLO]); + act(() => result.current.setSearch('osl')); + expect(result.current.filtered).toEqual([OSLO]); + }); + + it('FE-COMP-TRIPCOPYHOOK-003: nothing loads while closed, and reopening clears the search and reloads', async () => { + const { result, rerender, props } = setup({ open: false }); + expect(list).not.toHaveBeenCalled(); + rerender({ ...props, open: true }); + await waitFor(() => expect(result.current.loading).toBe(false)); + act(() => result.current.setSearch('rome')); + rerender({ ...props, open: false }); + rerender({ ...props, open: true }); + expect(result.current.search).toBe(''); + await waitFor(() => expect(list).toHaveBeenCalledTimes(2)); + }); + + it('FE-COMP-TRIPCOPYHOOK-004: an empty list when loading fails', async () => { + list.mockRejectedValueOnce(new Error('offline')); + const { result } = setup(); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.filtered).toEqual([]); + }); + + it('FE-COMP-TRIPCOPYHOOK-005: the date range joins both ends, or shows the one there is', () => { + const { result } = setup(); + expect(result.current.dateRange(PARIS)).toBe( + `${formatDate('2031-05-01', 'en')} – ${formatDate('2031-05-04', 'en')}` + ); + expect(result.current.dateRange(ROME)).toBe(formatDate('2031-06-01', 'en')); + expect(result.current.dateRange(OSLO)).toBe(''); + }); + + it('FE-COMP-TRIPCOPYHOOK-006: a pick copies, toasts the copied count and closes', async () => { + const { result, props } = setup(); + await act(async () => { + await result.current.handleCopy(1); + }); + expect(props.onCopy).toHaveBeenCalledWith(1); + expect(addToast).toHaveBeenCalledWith('collections.copiedCount:{"count":2}', 'success', undefined); + expect(addToast).toHaveBeenCalledTimes(1); + expect(props.onClose).toHaveBeenCalledTimes(1); + expect(result.current.busyTripId).toBeNull(); + }); + + it('FE-COMP-TRIPCOPYHOOK-007: skipped duplicates and an empty result get their own notes', async () => { + const onCopy = vi + .fn() + .mockResolvedValueOnce({ copied: 0, skipped: [{ id: 5, name: 'Louvre' }] }) + .mockResolvedValueOnce({ copied: 0, skipped: [] }); + const { result } = setup({ onCopy }); + await act(async () => { + await result.current.handleCopy(1); + }); + expect(addToast).toHaveBeenCalledWith('collections.skippedDuplicates:{"count":1}', 'info', undefined); + await act(async () => { + await result.current.handleCopy(1); + }); + expect(addToast).toHaveBeenLastCalledWith('collections.copyNothing', 'info', undefined); + }); + + it('FE-COMP-TRIPCOPYHOOK-008: a failed copy toasts the server message and stays open', async () => { + const onCopy = vi.fn().mockRejectedValue({ response: { data: { error: 'No access' } } }); + const { result, props } = setup({ onCopy }); + await act(async () => { + await result.current.handleCopy(2); + }); + expect(addToast).toHaveBeenCalledWith('No access', 'error', undefined); + expect(props.onClose).not.toHaveBeenCalled(); + }); + + it('FE-COMP-TRIPCOPYHOOK-009: one copy at a time, and none at all for an empty selection', async () => { + let finish: (v: { copied: number; skipped: [] }) => void = () => {}; + const onCopy = vi.fn().mockReturnValueOnce( + new Promise((r) => { + finish = r; + }) + ); + const { result } = setup({ onCopy }); + act(() => { + void result.current.handleCopy(1); + }); + expect(result.current.busyTripId).toBe(1); + await act(async () => { + await result.current.handleCopy(2); + }); + expect(onCopy).toHaveBeenCalledTimes(1); + act(() => finish({ copied: 1, skipped: [] })); + await waitFor(() => expect(result.current.busyTripId).toBeNull()); + + const empty = setup({ emptySelection: true }); + await act(async () => { + await empty.result.current.handleCopy(1); + }); + expect(empty.props.onCopy).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/Collections/useTripCopyPicker.ts b/client/src/components/Collections/useTripCopyPicker.ts new file mode 100644 index 000000000..9e98901b8 --- /dev/null +++ b/client/src/components/Collections/useTripCopyPicker.ts @@ -0,0 +1,101 @@ +import { useEffect, useMemo, useState } from 'react'; + +import { tripsApi } from '../../api/client'; +import { useTranslation } from '../../i18n'; +import type { TranslationFn } from '../../types'; +import { getApiErrorMessage } from '../../utils/apiError'; +import { formatDate } from '../../utils/formatters'; +import { useToast } from '../shared/Toast'; + +export interface TripOption { + id: number; + title: string; + start_date?: string | null; + end_date?: string | null; + cover_image?: string | null; +} + +export interface CopyResult { + copied: number; + skipped: { id: number; name: string }[]; +} + +export interface TripCopyPickerOptions { + open: boolean; + /** Copies the selected places into the trip; resolves with the server's reconcile result. */ + onCopy: (tripId: number) => Promise; + onClose: () => void; + t: TranslationFn; + /** True when nothing is selected: a pick then does nothing (the desktop dialog's guard). */ + emptySelection?: boolean; +} + +/** Trips whose title holds the trimmed, case folded query; all of them for an empty query. */ +export function filterTripOptions(trips: TripOption[], search: string): TripOption[] { + const q = search.trim().toLowerCase(); + if (!q) return trips; + return trips.filter((tr) => (tr.title ?? '').toLowerCase().includes(q)); +} + +/** + * The "Copy to trip" picker behind both the desktop dialog and the phone sheet, which + * render their own markup over it. Every opening loads the user's trips afresh and + * clears the search; a pick copies the selected places and turns the server's dedup + * result into copied / skipped duplicates toasts before closing. + */ +export function useTripCopyPicker({ open, onCopy, onClose, t, emptySelection = false }: TripCopyPickerOptions) { + const toast = useToast(); + const { language } = useTranslation(); + const [trips, setTrips] = useState([]); + const [loading, setLoading] = useState(false); + const [search, setSearch] = useState(''); + const [busyTripId, setBusyTripId] = useState(null); + + useEffect(() => { + if (!open) return; + let cancelled = false; + setLoading(true); + setSearch(''); + tripsApi + .list() + .then((res: { trips?: TripOption[] }) => { + if (!cancelled) setTrips(res.trips ?? []); + }) + .catch(() => { + if (!cancelled) setTrips([]); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [open]); + + const filtered = useMemo(() => filterTripOptions(trips, search), [trips, search]); + + const dateRange = (tr: TripOption): string => { + const s = formatDate(tr.start_date, language); + const e = formatDate(tr.end_date, language); + if (s && e) return `${s} – ${e}`; + return s || e || ''; + }; + + const handleCopy = async (tripId: number) => { + if (busyTripId != null || emptySelection) return; + setBusyTripId(tripId); + try { + const res = await onCopy(tripId); + if (res.copied > 0) toast.success(t('collections.copiedCount', { count: res.copied })); + if (res.skipped.length > 0) toast.info(t('collections.skippedDuplicates', { count: res.skipped.length })); + if (res.copied === 0 && res.skipped.length === 0) toast.info(t('collections.copyNothing')); + onClose(); + } catch (err) { + toast.error(getApiErrorMessage(err, t('common.error'))); + } finally { + setBusyTripId(null); + } + }; + + return { loading, search, setSearch, filtered, busyTripId, dateRange, handleCopy }; +} diff --git a/client/src/mobile/screens/collections/MCollTripPickerSheet.tsx b/client/src/mobile/screens/collections/MCollTripPickerSheet.tsx index dddce24d7..1459f438a 100644 --- a/client/src/mobile/screens/collections/MCollTripPickerSheet.tsx +++ b/client/src/mobile/screens/collections/MCollTripPickerSheet.tsx @@ -1,22 +1,9 @@ -import { useEffect, useMemo, useState } from 'react' import { CalendarDays, Copy, Loader2, MapPin, Search } from 'lucide-react' import type { TranslationFn } from '../../../types' -import { tripsApi } from '../../../api/client' -import { useTranslation } from '../../../i18n' -import { useToast } from '../../../components/shared/Toast' -import { getApiErrorMessage } from '../../../utils/apiError' -import { formatDate } from '../../../utils/formatters' +import { useTripCopyPicker } from '../../../components/Collections/useTripCopyPicker' import MSheet from '../../components/MSheet' import { SheetHeader } from './MCollSheetKit' -interface TripOption { - id: number - title: string - start_date?: string | null - end_date?: string | null - cover_image?: string | null -} - interface MCollTripPickerSheetProps { open: boolean count: number @@ -30,53 +17,8 @@ interface MCollTripPickerSheetProps { * selected places and reconciles the dedup result into toasts. */ export default function MCollTripPickerSheet({ open, count, onCopy, onClose, t }: MCollTripPickerSheetProps) { - const toast = useToast() - const { language } = useTranslation() - const [trips, setTrips] = useState([]) - const [loading, setLoading] = useState(false) - const [search, setSearch] = useState('') - const [busyTripId, setBusyTripId] = useState(null) - - useEffect(() => { - if (!open) return - let cancelled = false - setLoading(true) - setSearch('') - tripsApi.list() - .then((res: { trips?: TripOption[] }) => { if (!cancelled) setTrips(res.trips ?? []) }) - .catch(() => { if (!cancelled) setTrips([]) }) - .finally(() => { if (!cancelled) setLoading(false) }) - return () => { cancelled = true } - }, [open]) - - const filtered = useMemo(() => { - const q = search.trim().toLowerCase() - if (!q) return trips - return trips.filter(tr => (tr.title ?? '').toLowerCase().includes(q)) - }, [trips, search]) - - const dateRange = (tr: TripOption): string => { - const s = formatDate(tr.start_date, language) - const e = formatDate(tr.end_date, language) - if (s && e) return `${s} – ${e}` - return s || e || '' - } - - const handleCopy = async (tripId: number) => { - if (busyTripId != null) return - setBusyTripId(tripId) - try { - const res = await onCopy(tripId) - if (res.copied > 0) toast.success(t('collections.copiedCount', { count: res.copied })) - if (res.skipped.length > 0) toast.info(t('collections.skippedDuplicates', { count: res.skipped.length })) - if (res.copied === 0 && res.skipped.length === 0) toast.info(t('collections.copyNothing')) - onClose() - } catch (err) { - toast.error(getApiErrorMessage(err, t('common.error'))) - } finally { - setBusyTripId(null) - } - } + const { loading, search, setSearch, filtered, busyTripId, dateRange, handleCopy } = + useTripCopyPicker({ open, onCopy, onClose, t }) const title = t('collections.copyN', { count })