diff --git a/client/src/components/Planner/useListImport.test.ts b/client/src/components/Planner/useListImport.test.ts new file mode 100644 index 000000000..6209f57cc --- /dev/null +++ b/client/src/components/Planner/useListImport.test.ts @@ -0,0 +1,273 @@ +// FE-PLANNER-LISTIMP-001 to FE-PLANNER-LISTIMP-014: the shared-list import logic behind +// the desktop dialog (no onDone) and the phone's import step (onDone). +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { resetAllStores, seedStore } from '../../../tests/helpers/store'; +import { placesApi } from '../../api/client'; +import { useAuthStore } from '../../store/authStore'; +import { useListImport, type ListImportOptions } from './useListImport'; + +const t = (key: string, params?: Record) => + params ? `${key}:${Object.values(params).join(',')}` : key; + +function setup(over: Partial = {}) { + const toast = { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() }; + const loadTrip = vi.fn(async (_tripId: number) => undefined); + const pushUndo = vi.fn((_label: string, _undo: () => Promise | void) => {}); + const options: ListImportOptions = { tripId: 1, t, toast, loadTrip, pushUndo, ...over }; + const hook = renderHook(() => useListImport(options)); + return { ...hook, toast, loadTrip, pushUndo }; +} + +async function importUrl(result: { current: ReturnType }, url: string) { + act(() => result.current.setUrl(url)); + await act(async () => { + await result.current.handleImport(); + }); +} + +beforeEach(() => { + resetAllStores(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('useListImport', () => { + it('FE-PLANNER-LISTIMP-001: starts closed, on Google, with an empty link and enrichment off', () => { + const { result } = setup(); + expect(result.current.open).toBe(false); + expect(result.current.provider).toBe('google'); + expect(result.current.url).toBe(''); + expect(result.current.enrich).toBe(false); + expect(result.current.loading).toBe(false); + expect(result.current.canEnrich).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-002: an empty or blank link starts nothing', async () => { + const google = vi.spyOn(placesApi, 'importGoogleList'); + const { result, loadTrip } = setup(); + await importUrl(result, ' '); + expect(google).not.toHaveBeenCalled(); + expect(loadTrip).not.toHaveBeenCalled(); + expect(result.current.loading).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-003: the desktop dialog imports the trimmed link, reloads, toasts, closes and clears', async () => { + const google = vi + .spyOn(placesApi, 'importGoogleList') + .mockResolvedValue({ count: 2, skipped: 0, listName: 'Tokyo', places: [{ id: 20 }, { id: 21 }] }); + const { result, toast, loadTrip, pushUndo } = setup(); + act(() => result.current.setOpen(true)); + + await importUrl(result, ' https://maps.app.goo.gl/abc '); + + expect(google).toHaveBeenCalledWith(1, 'https://maps.app.goo.gl/abc', false); + expect(loadTrip).toHaveBeenCalledWith(1); + expect(toast.success).toHaveBeenCalledWith('places.googleListImported:2,Tokyo'); + expect(pushUndo).toHaveBeenCalledWith('undo.importGoogleList', expect.any(Function)); + expect(result.current.open).toBe(false); + expect(result.current.url).toBe(''); + expect(result.current.loading).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-004: the desktop dialog is closed before the undo step goes in', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ + count: 1, + skipped: 0, + listName: 'L', + places: [{ id: 7 }], + }); + const { result, toast, pushUndo } = setup(); + pushUndo.mockImplementation(() => { + throw new Error('history full'); + }); + act(() => result.current.setOpen(true)); + + await importUrl(result, 'https://maps.app.goo.gl/abc'); + + // The close ran before the throwing undo step, so it stands. + expect(result.current.open).toBe(false); + expect(result.current.url).toBe(''); + expect(toast.error).toHaveBeenCalledWith('places.googleListError'); + }); + + it('FE-PLANNER-LISTIMP-005: the phone step hands over through onDone after the undo step and keeps its link', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ + count: 1, + skipped: 0, + listName: 'L', + places: [{ id: 7 }], + }); + const onDone = vi.fn(); + const { result, pushUndo } = setup({ onDone }); + act(() => result.current.setOpen(true)); + + await importUrl(result, 'https://maps.app.goo.gl/abc'); + + expect(onDone).toHaveBeenCalledTimes(1); + expect(pushUndo.mock.invocationCallOrder[0]).toBeLessThan(onDone.mock.invocationCallOrder[0]); + expect(result.current.url).toBe('https://maps.app.goo.gl/abc'); + expect(result.current.open).toBe(true); + }); + + it('FE-PLANNER-LISTIMP-006: the undo step bulk-deletes the imported places and reloads, even when the delete fails', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ + count: 2, + skipped: 0, + listName: 'L', + places: [{ id: 71 }, { id: 72 }], + }); + const bulkDelete = vi + .spyOn(placesApi, 'bulkDelete') + .mockResolvedValueOnce({ deleted: 2 }) + .mockRejectedValueOnce(new Error('gone')); + const { result, loadTrip, pushUndo } = setup(); + await importUrl(result, 'https://maps.app.goo.gl/abc'); + + const revert = pushUndo.mock.calls[0][1] as () => Promise; + await revert(); + expect(bulkDelete).toHaveBeenCalledWith(1, [71, 72]); + expect(loadTrip).toHaveBeenCalledTimes(2); + + await expect(revert()).resolves.toBeUndefined(); + expect(loadTrip).toHaveBeenCalledTimes(3); + }); + + it('FE-PLANNER-LISTIMP-007: nothing created means no undo step, and an all-skipped import warns', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ count: 0, skipped: 4, listName: 'L', places: [] }); + const { result, toast, pushUndo } = setup(); + await importUrl(result, 'https://maps.app.goo.gl/abc'); + expect(toast.warning).toHaveBeenCalledWith('places.importAllSkipped'); + expect(toast.success).not.toHaveBeenCalled(); + expect(pushUndo).not.toHaveBeenCalled(); + }); + + it('FE-PLANNER-LISTIMP-008: Naver goes to its own endpoint with its own toast and undo label', async () => { + const naver = vi + .spyOn(placesApi, 'importNaverList') + .mockResolvedValue({ count: 2, skipped: 0, listName: 'Seoul', places: [{ id: 81 }] }); + const { result, toast, pushUndo } = setup(); + act(() => result.current.setProvider('naver')); + await importUrl(result, 'https://naver.me/xyz'); + expect(naver).toHaveBeenCalledWith(1, 'https://naver.me/xyz', false); + expect(toast.success).toHaveBeenCalledWith('places.naverListImported:2,Seoul'); + expect(pushUndo).toHaveBeenCalledWith('undo.importNaverList', expect.any(Function)); + }); + + it('FE-PLANNER-LISTIMP-009: enrichment is sent only when asked for and a maps key exists', async () => { + const google = vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ count: 1, skipped: 0, places: [] }); + const first = setup(); + act(() => first.result.current.setEnrich(true)); + await importUrl(first.result, 'https://maps.app.goo.gl/a'); + expect(google).toHaveBeenLastCalledWith(1, 'https://maps.app.goo.gl/a', false); + first.unmount(); + + seedStore(useAuthStore, { hasMapsKey: true }); + const second = setup(); + expect(second.result.current.canEnrich).toBe(true); + await importUrl(second.result, 'https://maps.app.goo.gl/b'); + expect(google).toHaveBeenLastCalledWith(1, 'https://maps.app.goo.gl/b', false); + act(() => second.result.current.setEnrich(true)); + await importUrl(second.result, 'https://maps.app.goo.gl/c'); + expect(google).toHaveBeenLastCalledWith(1, 'https://maps.app.goo.gl/c', true); + }); + + it('FE-PLANNER-LISTIMP-010: a failure shows the server message and leaves the dialog open with its link', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockRejectedValue({ response: { data: { error: 'List is private' } } }); + const onDone = vi.fn(); + const { result, toast, loadTrip } = setup({ onDone }); + act(() => result.current.setOpen(true)); + await importUrl(result, 'https://maps.app.goo.gl/abc'); + expect(toast.error).toHaveBeenCalledWith('List is private'); + expect(onDone).not.toHaveBeenCalled(); + expect(loadTrip).not.toHaveBeenCalled(); + expect(result.current.open).toBe(true); + expect(result.current.url).toBe('https://maps.app.goo.gl/abc'); + expect(result.current.loading).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-011: without a server message the provider error key is shown', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockRejectedValue(new Error('network')); + vi.spyOn(placesApi, 'importNaverList').mockRejectedValue(new Error('network')); + const { result, toast } = setup(); + await importUrl(result, 'https://maps.app.goo.gl/abc'); + expect(toast.error).toHaveBeenLastCalledWith('places.googleListError'); + act(() => result.current.setProvider('naver')); + await importUrl(result, 'https://naver.me/xyz'); + expect(toast.error).toHaveBeenLastCalledWith('places.naverListError'); + }); + + it('FE-PLANNER-LISTIMP-012: on the phone a second import while one is in flight is refused', async () => { + let release: (value: unknown) => void = () => {}; + const google = vi.spyOn(placesApi, 'importGoogleList').mockReturnValue( + new Promise((resolve) => { + release = resolve; + }) + ); + const onDone = vi.fn(); + const { result } = setup({ onDone }); + act(() => result.current.setUrl('https://maps.app.goo.gl/abc')); + + let first!: Promise; + act(() => { + first = result.current.handleImport(); + }); + expect(result.current.loading).toBe(true); + await act(async () => { + await result.current.handleImport(); + }); + expect(google).toHaveBeenCalledTimes(1); + + await act(async () => { + release({ count: 1, skipped: 0, listName: 'L', places: [] }); + await first; + }); + expect(onDone).toHaveBeenCalledTimes(1); + expect(result.current.loading).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-013: works without a pushUndo', async () => { + vi.spyOn(placesApi, 'importGoogleList').mockResolvedValue({ + count: 1, + skipped: 0, + listName: 'L', + places: [{ id: 1 }], + }); + const { result, toast } = setup({ pushUndo: undefined }); + await importUrl(result, 'https://maps.app.goo.gl/abc'); + expect(toast.success).toHaveBeenCalledWith('places.googleListImported:1,L'); + expect(result.current.open).toBe(false); + }); + + it('FE-PLANNER-LISTIMP-014: the desktop dialog leaves the in-flight guard to its button', async () => { + const releases: ((value: unknown) => void)[] = []; + const google = vi.spyOn(placesApi, 'importGoogleList').mockImplementation( + () => + new Promise((resolve) => { + releases.push(resolve); + }) + ); + const { result } = setup(); + act(() => result.current.setUrl('https://maps.app.goo.gl/abc')); + + let first!: Promise; + act(() => { + first = result.current.handleImport(); + }); + expect(result.current.loading).toBe(true); + let second!: Promise; + act(() => { + second = result.current.handleImport(); + }); + expect(google).toHaveBeenCalledTimes(2); + + await act(async () => { + releases.forEach((release) => release({ count: 1, skipped: 0, listName: 'L', places: [] })); + await Promise.all([first, second]); + }); + expect(result.current.loading).toBe(false); + }); +}); diff --git a/client/src/components/Planner/useListImport.ts b/client/src/components/Planner/useListImport.ts new file mode 100644 index 000000000..008b8f20d --- /dev/null +++ b/client/src/components/Planner/useListImport.ts @@ -0,0 +1,94 @@ +import { useState } from 'react'; + +import { placesApi } from '../../api/client'; +import type { useTranslation } from '../../i18n'; +import { useAuthStore } from '../../store/authStore'; +import type { useToast } from '../shared/Toast'; + +export type ListImportProvider = 'google' | 'naver'; + +export interface ListImportOptions { + tripId: number; + t: ReturnType['t']; + toast: ReturnType; + loadTrip: (tripId: number) => Promise; + pushUndo?: (label: string, undoFn: () => Promise | void) => void; + /** + * Where a successful import hands over, after its undo step is registered: the phone's + * import sheet closes here. Without it the import runs as the desktop dialog, which + * closes itself and clears the link before the undo step goes in. + */ + onDone?: () => void; +} + +/** + * Importing a shared Google Maps or Naver list into the trip's places: the one logic + * path behind the desktop dialog and the phone's import step, which render their own + * markup over it. Same endpoints, the same optional Google enrichment, a toast for what + * came in (or a warning when every entry was already there), and an undo step that + * deletes the imported places again. + */ +export function useListImport({ tripId, t, toast, loadTrip, pushUndo, onDone }: ListImportOptions) { + const [open, setOpen] = useState(false); + const [url, setUrl] = useState(''); + const [loading, setLoading] = useState(false); + const [provider, setProvider] = useState('google'); + const [enrich, setEnrich] = useState(false); + // Places-API enrichment (#886) needs a Google Maps key. Not the places + // *provider* choice: enrichment's photos and summary come from Google (and, + // keyless, from Wikimedia), which is independent of which provider answers + // search: an Amap install with a Google key still enriches through Google. + const canEnrich = useAuthStore((s) => s.hasMapsKey); + + /** The desktop dialog's way out: closed, and the link it held gone with it. */ + const close = () => { + setOpen(false); + setUrl(''); + }; + + const handleImport = async () => { + const trimmed = url.trim(); + // Only the phone step refuses a second import itself; the desktop dialog gates its + // button and Enter key on the loading flag instead. + if (!trimmed || (onDone && loading)) return; + setLoading(true); + try { + const withEnrich = enrich && canEnrich; + const result = + provider === 'google' + ? await placesApi.importGoogleList(tripId, trimmed, withEnrich) + : await placesApi.importNaverList(tripId, trimmed, withEnrich); + await loadTrip(tripId); + if (result.count === 0 && result.skipped > 0) { + toast.warning(t('places.importAllSkipped')); + } else { + toast.success( + t(provider === 'google' ? 'places.googleListImported' : 'places.naverListImported', { + count: result.count, + list: result.listName, + }) + ); + } + if (!onDone) close(); + if (result.places?.length > 0) { + const importedIds: number[] = result.places.map((p: { id: number }) => p.id); + pushUndo?.(t(provider === 'google' ? 'undo.importGoogleList' : 'undo.importNaverList'), async () => { + try { + await placesApi.bulkDelete(tripId, importedIds); + } catch { + // best effort: the trip reload below reflects whatever happened + } + await loadTrip(tripId); + }); + } + onDone?.(); + } catch (err: unknown) { + const message = (err as { response?: { data?: { error?: string } } })?.response?.data?.error; + toast.error(message || t(provider === 'google' ? 'places.googleListError' : 'places.naverListError')); + } finally { + setLoading(false); + } + }; + + return { open, setOpen, url, setUrl, loading, provider, setProvider, enrich, setEnrich, canEnrich, handleImport }; +} diff --git a/client/src/components/Planner/usePlacesSidebar.ts b/client/src/components/Planner/usePlacesSidebar.ts index 4ca56c0fd..30885b36d 100644 --- a/client/src/components/Planner/usePlacesSidebar.ts +++ b/client/src/components/Planner/usePlacesSidebar.ts @@ -7,11 +7,9 @@ import { Pencil, Trash2, ExternalLink, Navigation, CalendarDays, Bookmark } from import { useTranslation } from '../../i18n' import { useToast } from '../shared/Toast' import { useContextMenu } from '../shared/ContextMenu' -import { placesApi } from '../../api/client' import { collectionsApi } from '../../api/collections' import { useTripStore } from '../../store/tripStore' import { useCanDo } from '../../store/permissionsStore' -import { useAuthStore } from '../../store/authStore' import { useAddonStore } from '../../store/addonStore' import { useSaveToCollectionStore } from '../../store/saveToCollectionStore' import { placeToSaveTarget } from '../Collections/saveTarget' @@ -22,6 +20,7 @@ import { matchesCategoryFilter, matchesPlacesFilter } from '../../utils/placesFi import { safeHttpUrl } from '../../utils/safeUrl' import { plannedPlaceIds, plannedPlaceIdsForDay, type PlannedAccommodation } from '../../utils/plannedPlaces' import type { MenuEntry } from './planParts' +import { useListImport, type ListImportProvider } from './useListImport' /** Stable identity — a fresh [] default would invalidate the planned memo on every render. */ const NO_ACCOMMODATIONS: PlannedAccommodation[] = [] @@ -99,11 +98,6 @@ export function usePlacesSidebar(props: PlacesSidebarProps) { const can = useCanDo() const canEditPlaces = can('place_edit', trip) const collectionsEnabled = useAddonStore((s) => s.isEnabled('collections')) - // Places-API enrichment (#886) needs a Google Maps key. Not the places - // *provider* choice: enrichment's photos and summary come from Google (and, - // keyless, from Wikimedia), which is independent of which provider answers - // search — an Amap install with a Google key still enriches through Google. - const canEnrichImport = useAuthStore((s) => s.hasMapsKey) const [fileImportOpen, setFileImportOpen] = useState(false) const [sidebarDropFile, setSidebarDropFile] = useState(null) @@ -146,45 +140,10 @@ export function usePlacesSidebar(props: PlacesSidebarProps) { setFileImportOpen(true) } - const [listImportOpen, setListImportOpen] = useState(false) - const [listImportUrl, setListImportUrl] = useState('') - const [listImportLoading, setListImportLoading] = useState(false) - const [listImportProvider, setListImportProvider] = useState<'google' | 'naver'>('google') - const [listImportEnrich, setListImportEnrich] = useState(false) - const availableListImportProviders: Array<'google' | 'naver'> = ['google', 'naver'] + const listImport = useListImport({ tripId, t, toast, loadTrip, pushUndo }) + const availableListImportProviders: ListImportProvider[] = ['google', 'naver'] const hasMultipleListImportProviders = availableListImportProviders.length > 1 - const handleListImport = async () => { - if (!listImportUrl.trim()) return - setListImportLoading(true) - const provider = listImportProvider - try { - const enrich = listImportEnrich && canEnrichImport - const result = provider === 'google' - ? await placesApi.importGoogleList(tripId, listImportUrl.trim(), enrich) - : await placesApi.importNaverList(tripId, listImportUrl.trim(), enrich) - await loadTrip(tripId) - if (result.count === 0 && result.skipped > 0) { - toast.warning(t('places.importAllSkipped')) - } else { - toast.success(t(provider === 'google' ? 'places.googleListImported' : 'places.naverListImported', { count: result.count, list: result.listName })) - } - setListImportOpen(false) - setListImportUrl('') - if (result.places?.length > 0) { - const importedIds: number[] = result.places.map((p: { id: number }) => p.id) - pushUndo?.(t(provider === 'google' ? 'undo.importGoogleList' : 'undo.importNaverList'), async () => { - try { await placesApi.bulkDelete(tripId, importedIds) } catch {} - await loadTrip(tripId) - }) - } - } catch (err: any) { - toast.error(err?.response?.data?.error || t(provider === 'google' ? 'places.googleListError' : 'places.naverListError')) - } finally { - setListImportLoading(false) - } - } - const [search, setSearch] = useState('') // Filter state lives in the trip store so it survives the Plan tab // unmounting (tab switch, mobile sheet close) and stays in lockstep with the @@ -401,10 +360,11 @@ export function usePlacesSidebar(props: PlacesSidebarProps) { fileImportOpen, setFileImportOpen, sidebarDropFile, setSidebarDropFile, sidebarDragOver, handleSidebarDragEnter, handleSidebarDragOver, handleSidebarDragLeave, handleSidebarDrop, scrollContainerRef, onScrollTopChange, - listImportOpen, setListImportOpen, listImportUrl, setListImportUrl, - listImportLoading, listImportProvider, setListImportProvider, - listImportEnrich, setListImportEnrich, canEnrichImport, - availableListImportProviders, hasMultipleListImportProviders, handleListImport, + listImportOpen: listImport.open, setListImportOpen: listImport.setOpen, + listImportUrl: listImport.url, setListImportUrl: listImport.setUrl, + listImportLoading: listImport.loading, listImportProvider: listImport.provider, setListImportProvider: listImport.setProvider, + listImportEnrich: listImport.enrich, setListImportEnrich: listImport.setEnrich, canEnrichImport: listImport.canEnrich, + availableListImportProviders, hasMultipleListImportProviders, handleListImport: listImport.handleImport, search, setSearch, filter, setFilter, pickFilter, filterCounts, categoryFilters, setCategoryFilters, ratingFilter, setRatingFilter, diff --git a/client/src/mobile/screens/trip/sheets/ImpListStep.tsx b/client/src/mobile/screens/trip/sheets/ImpListStep.tsx index 1f882417a..b8c89d045 100644 --- a/client/src/mobile/screens/trip/sheets/ImpListStep.tsx +++ b/client/src/mobile/screens/trip/sheets/ImpListStep.tsx @@ -1,12 +1,8 @@ -import { useState } from 'react' -import { placesApi } from '../../../../api/client' -import { useAuthStore } from '../../../../store/authStore' import MChip from '../../../components/MChip' import MToggle from '../../../components/MToggle' import { FIELD_CLS, FormSheetFooter } from './PlSheetChrome' import type { TripPlanner } from '../MTripShell' - -type ListProvider = 'google' | 'naver' +import { useListImport } from '../../../../components/Planner/useListImport' interface ImpListStepProps { planner: TripPlanner @@ -23,52 +19,9 @@ interface ImpListStepProps { */ export default function ImpListStep({ planner, onBack, onDone }: ImpListStepProps) { const { t, toast, tripId, tripActions, pushUndo } = planner - const canEnrich = useAuthStore(s => s.hasMapsKey) - - const [provider, setProvider] = useState('google') - const [url, setUrl] = useState('') - const [enrich, setEnrich] = useState(false) - const [loading, setLoading] = useState(false) - - const handleImport = async () => { - const trimmed = url.trim() - if (!trimmed || loading) return - setLoading(true) - try { - const result = - provider === 'google' - ? await placesApi.importGoogleList(tripId, trimmed, enrich && canEnrich) - : await placesApi.importNaverList(tripId, trimmed, enrich && canEnrich) - await tripActions.loadTrip(tripId) - if (result.count === 0 && result.skipped > 0) { - toast.warning(t('places.importAllSkipped')) - } else { - toast.success( - t(provider === 'google' ? 'places.googleListImported' : 'places.naverListImported', { - count: result.count, - list: result.listName, - }), - ) - } - if (result.places?.length > 0) { - const importedIds: number[] = result.places.map((p: { id: number }) => p.id) - pushUndo(t(provider === 'google' ? 'undo.importGoogleList' : 'undo.importNaverList'), async () => { - try { - await placesApi.bulkDelete(tripId, importedIds) - } catch { - // best effort — the trip reload below reflects whatever happened - } - await tripActions.loadTrip(tripId) - }) - } - onDone() - } catch (err: unknown) { - const message = (err as { response?: { data?: { error?: string } } })?.response?.data?.error - toast.error(message || t(provider === 'google' ? 'places.googleListError' : 'places.naverListError')) - } finally { - setLoading(false) - } - } + const { provider, setProvider, url, setUrl, enrich, setEnrich, loading, canEnrich, handleImport } = useListImport({ + tripId, t, toast, loadTrip: tripActions.loadTrip, pushUndo, onDone, + }) return ( <>