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.
2026-10-18 21:16:47 +00:00 · 2026-10-09 17:34:51 +02:00
parent 33db4ea7cb
commit 694e11c117
6 changed files with 34 additions and 32 deletions
@@ -33,7 +33,6 @@ export default function VacayPersons() {
    submit: invite,
    successKey: 'vacay.inviteSent',
    errorKey: 'vacay.inviteError',
-    clearOnLoadError: false,
  })
  const inviteLabelId = useId()
  const colorLabelId = useId()
@@ -26,7 +26,6 @@ export default function VacaySharedCalendars() {
    submit: shareWith,
    successKey: 'vacay.shareSent',
    errorKey: 'vacay.shareFailed',
-    clearOnLoadError: false,
  })
  const shareLabelId = useId()

@@ -15,14 +15,13 @@ const USERS = [

 const submit = vi.fn<(userId: number) => Promise<void>>();

-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);
@@ -12,11 +12,6 @@ interface VacayUserPickerOptions {
  submit: (userId: number) => Promise<void>;
  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<U extends { id: number; username: string }>({
  submit,
  successKey,
  errorKey,
-  clearOnLoadError,
  sheetOpen,
 }: VacayUserPickerOptions) {
  const { t } = useTranslation();
@@ -50,9 +44,11 @@ export function useVacayUserPicker<U extends { id: number; username: string }>({
      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;
@@ -25,7 +25,6 @@ export default function MVacayInviteSheet({ open, onClose }: MVacayInviteSheetPr
    submit: invite,
    successKey: 'vacay.inviteSent',
    errorKey: 'vacay.inviteError',
-    clearOnLoadError: true,
    sheetOpen: open,
  })

@@ -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()