From 694e11c1179b7cc5b26c87530cebfb866372f005 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:34:51 +0200 Subject: [PATCH] fix(vacay): a failed user load in the desktop invite and share dialogs no longer shows a stale list When loading the users that can be invited to a fusion or given a calendar share failed on desktop, the dialog kept showing the list from the last load, so a user could pick someone who is no longer available and only learn it from the failed send. The phone sheets already emptied the list and showed their "no users available" state. Cause: useVacayUserPicker only emptied the list on a failed load when its clearOnLoadError option was true; the desktop VacayPersons and VacaySharedCalendars dialogs passed false, a leftover of the old silent catch in the desktop dialogs. Fix: the hook always empties the list when the load fails. The option only existed to keep the desktop behaviour, so it is removed from the hook and from all four callers. Red test: FE-COMP-VACAYPICK-002 (useVacayUserPicker.test.ts) failed on the unfixed code with "AssertionError: expected [ { id: 2, username: 'bob' }, ...(1) ] to deeply equal []". Changed assertions: - useVacayUserPicker.test.ts FE-COMP-VACAYPICK-002: was "a failed load keeps the last list on desktop" expecting available to equal USERS; now expects an empty list. - FE-COMP-VACAYPICK-003: was "a failed load empties the list on the phone" through load() with clearOnLoadError true; now drives the same expectation (empty list) through a phone sheet reopening, since the option is gone. --- client/src/components/Vacay/VacayPersons.tsx | 1 - .../components/Vacay/VacaySharedCalendars.tsx | 1 - .../Vacay/useVacayUserPicker.test.ts | 50 +++++++++++-------- .../components/Vacay/useVacayUserPicker.ts | 12 ++--- .../screens/vacay/MVacayInviteSheet.tsx | 1 - .../mobile/screens/vacay/MVacayShareSheet.tsx | 1 - 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/client/src/components/Vacay/VacayPersons.tsx b/client/src/components/Vacay/VacayPersons.tsx index d78c41aac..d5a64113f 100644 --- a/client/src/components/Vacay/VacayPersons.tsx +++ b/client/src/components/Vacay/VacayPersons.tsx @@ -33,7 +33,6 @@ export default function VacayPersons() { submit: invite, successKey: 'vacay.inviteSent', errorKey: 'vacay.inviteError', - clearOnLoadError: false, }) const inviteLabelId = useId() const colorLabelId = useId() diff --git a/client/src/components/Vacay/VacaySharedCalendars.tsx b/client/src/components/Vacay/VacaySharedCalendars.tsx index eed85b01c..0b4b4127a 100644 --- a/client/src/components/Vacay/VacaySharedCalendars.tsx +++ b/client/src/components/Vacay/VacaySharedCalendars.tsx @@ -26,7 +26,6 @@ export default function VacaySharedCalendars() { submit: shareWith, successKey: 'vacay.shareSent', errorKey: 'vacay.shareFailed', - clearOnLoadError: false, }) const shareLabelId = useId() diff --git a/client/src/components/Vacay/useVacayUserPicker.test.ts b/client/src/components/Vacay/useVacayUserPicker.test.ts index 2bffd6103..73410266f 100644 --- a/client/src/components/Vacay/useVacayUserPicker.test.ts +++ b/client/src/components/Vacay/useVacayUserPicker.test.ts @@ -15,14 +15,13 @@ const USERS = [ const submit = vi.fn<(userId: number) => Promise>(); -function picker(clearOnLoadError: boolean) { +function picker() { return renderHook(() => useVacayUserPicker<{ id: number; username: string }>({ endpoint: '/addons/vacay/shares/available-users', submit, successKey: 'vacay.shareSent', errorKey: 'vacay.shareFailed', - clearOnLoadError, }) ); } @@ -37,7 +36,7 @@ afterEach(() => vi.restoreAllMocks()); describe('useVacayUserPicker', () => { it('FE-COMP-VACAYPICK-001: loads the users from the endpoint and resolves the picked one', async () => { const get = vi.spyOn(apiClient, 'get').mockResolvedValue({ data: { users: USERS } }); - const { result } = picker(false); + const { result } = picker(); await act(() => result.current.load()); expect(get).toHaveBeenCalledWith('/addons/vacay/shares/available-users'); expect(result.current.available).toEqual(USERS); @@ -45,27 +44,39 @@ describe('useVacayUserPicker', () => { expect(result.current.selectedUser).toEqual(USERS[1]); }); - it('FE-COMP-VACAYPICK-002: a failed load keeps the last list on desktop', async () => { + it('FE-COMP-VACAYPICK-002: a failed load in a desktop dialog empties the list instead of showing the stale one', async () => { const get = vi.spyOn(apiClient, 'get').mockResolvedValueOnce({ data: { users: USERS } }); - const { result } = picker(false); - await act(() => result.current.load()); - get.mockRejectedValueOnce(new Error('down')); - await act(() => result.current.load()); - expect(result.current.available).toEqual(USERS); - }); - - it('FE-COMP-VACAYPICK-003: a failed load empties the list on the phone', async () => { - const get = vi.spyOn(apiClient, 'get').mockResolvedValueOnce({ data: { users: USERS } }); - const { result } = picker(true); + const { result } = picker(); await act(() => result.current.load()); get.mockRejectedValueOnce(new Error('down')); await act(() => result.current.load()); expect(result.current.available).toEqual([]); }); + it('FE-COMP-VACAYPICK-003: a failed load on reopening a phone sheet empties the list', async () => { + const get = vi.spyOn(apiClient, 'get').mockResolvedValueOnce({ data: { users: USERS } }); + const { result, rerender } = renderHook( + ({ open }: { open: boolean }) => + useVacayUserPicker<{ id: number; username: string }>({ + endpoint: '/addons/vacay/available-users', + submit, + successKey: 'vacay.inviteSent', + errorKey: 'vacay.inviteError', + sheetOpen: open, + }), + { initialProps: { open: true } } + ); + await act(async () => {}); + expect(result.current.available).toEqual(USERS); + get.mockRejectedValueOnce(new Error('down')); + await act(async () => rerender({ open: false })); + await act(async () => rerender({ open: true })); + expect(result.current.available).toEqual([]); + }); + it('FE-COMP-VACAYPICK-004: sending without a pick does nothing', async () => { const onSent = vi.fn(); - const { result } = picker(false); + const { result } = picker(); await act(() => result.current.send(onSent)); expect(submit).not.toHaveBeenCalled(); expect(onSent).not.toHaveBeenCalled(); @@ -74,7 +85,7 @@ describe('useVacayUserPicker', () => { it('FE-COMP-VACAYPICK-005: a sent pick toasts success, then runs onSent', async () => { const order: string[] = []; toast.success.mockImplementation(() => order.push('toast')); - const { result } = picker(false); + const { result } = picker(); act(() => result.current.setSelected(2)); await act(() => result.current.send(() => order.push('sent'))); expect(submit).toHaveBeenCalledWith(2); @@ -86,7 +97,7 @@ describe('useVacayUserPicker', () => { it('FE-COMP-VACAYPICK-006: a failed send toasts the server error and skips onSent', async () => { submit.mockRejectedValue(new Error('already shared')); const onSent = vi.fn(); - const { result } = picker(false); + const { result } = picker(); act(() => result.current.setSelected(2)); await act(() => result.current.send(onSent)); expect(toast.error).toHaveBeenCalledWith('already shared'); @@ -96,7 +107,7 @@ describe('useVacayUserPicker', () => { it('FE-COMP-VACAYPICK-007: the desktop dialogs (no sheetOpen) never load on their own', () => { const get = vi.spyOn(apiClient, 'get').mockResolvedValue({ data: { users: USERS } }); - picker(false); + picker(); expect(get).not.toHaveBeenCalled(); }); @@ -109,7 +120,6 @@ describe('useVacayUserPicker', () => { submit, successKey: 'vacay.inviteSent', errorKey: 'vacay.inviteError', - clearOnLoadError: true, sheetOpen: open, }), { initialProps: { open: false } } @@ -131,7 +141,7 @@ describe('useVacayUserPicker', () => { }); it('FE-COMP-VACAYPICK-009: toggling unfolds the inline list and picking a user folds it again', () => { - const { result } = picker(true); + const { result } = picker(); expect(result.current.pickerOpen).toBe(false); act(() => result.current.togglePicker()); expect(result.current.pickerOpen).toBe(true); diff --git a/client/src/components/Vacay/useVacayUserPicker.ts b/client/src/components/Vacay/useVacayUserPicker.ts index fed1b2642..384cc8aef 100644 --- a/client/src/components/Vacay/useVacayUserPicker.ts +++ b/client/src/components/Vacay/useVacayUserPicker.ts @@ -12,11 +12,6 @@ interface VacayUserPickerOptions { submit: (userId: number) => Promise; successKey: string; errorKey: string; - /** - * Empty the list when loading it fails (the phone sheets). Otherwise the list - * from the last load stays. - */ - clearOnLoadError: boolean; /** * The phone sheets pass their open flag: every opening clears the pick, folds * the inline user list and reloads the users. The desktop dialogs leave it out @@ -34,7 +29,6 @@ export function useVacayUserPicker({ submit, successKey, errorKey, - clearOnLoadError, sheetOpen, }: VacayUserPickerOptions) { const { t } = useTranslation(); @@ -50,9 +44,11 @@ export function useVacayUserPicker({ const data = await apiClient.get(endpoint).then((r) => r.data); setAvailable(data.users); } catch { - if (clearOnLoadError) setAvailable([]); + // A failed load empties the list, so nobody picks from a stale one and the + // dialog or sheet shows its "no users available" state. + setAvailable([]); } - }, [endpoint, clearOnLoadError]); + }, [endpoint]); useEffect(() => { if (!sheetOpen) return; diff --git a/client/src/mobile/screens/vacay/MVacayInviteSheet.tsx b/client/src/mobile/screens/vacay/MVacayInviteSheet.tsx index 1f258d664..501e78baf 100644 --- a/client/src/mobile/screens/vacay/MVacayInviteSheet.tsx +++ b/client/src/mobile/screens/vacay/MVacayInviteSheet.tsx @@ -25,7 +25,6 @@ export default function MVacayInviteSheet({ open, onClose }: MVacayInviteSheetPr submit: invite, successKey: 'vacay.inviteSent', errorKey: 'vacay.inviteError', - clearOnLoadError: true, sheetOpen: open, }) diff --git a/client/src/mobile/screens/vacay/MVacayShareSheet.tsx b/client/src/mobile/screens/vacay/MVacayShareSheet.tsx index b1ab621f0..b31f9238b 100644 --- a/client/src/mobile/screens/vacay/MVacayShareSheet.tsx +++ b/client/src/mobile/screens/vacay/MVacayShareSheet.tsx @@ -25,7 +25,6 @@ export default function MVacayShareSheet({ open, onClose }: MVacayShareSheetProp submit: shareWith, successKey: 'vacay.shareSent', errorKey: 'vacay.shareFailed', - clearOnLoadError: true, sheetOpen: open, }) const { toggleHidden, remove } = useVacayShareActions()