From 1e674f040a0d769f451337d906dbf9b63ea2e982 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:43:24 +0200 Subject: [PATCH] fix(planner): a create expense click that saves nothing no longer opens the editor on the next save In the desktop booking and transport dialogs, clicking "Create expense" (or editing a linked expense) before the booking had a title saved nothing, which is right, but the wish was kept. The next ordinary save then opened the expense editor out of nowhere. The same happened after a save that threw. useBookingExpenseIntent stored the wish in a ref and only cleared it when the form read it, which the desktop forms do after a successful save. The hook now clears the wish once the save its click started has settled, unless a newer click replaced it in the meantime. The phone sheets read it at the start of the save and are unaffected. Red tests: - FE-PLANNER-RESMODAL-107 in ReservationModal.test.tsx: "expected "vi.fn()" to not be called at all, but actually been called 1 times". - FE-PLANNER-BOOKINGEXPENSE-005 in useBookingExpenseIntent.test.ts: "expected { create: true } to be null". - FE-PLANNER-BOOKINGEXPENSE-006: "expected { editItem: { id: 9, ... } } to be null". No existing assertions changed. --- .../Planner/ReservationModal.test.tsx | 18 ++++++++++- .../Planner/useBookingExpenseIntent.test.ts | 31 ++++++++++++++++++- .../Planner/useBookingExpenseIntent.ts | 20 +++++++----- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/client/src/components/Planner/ReservationModal.test.tsx b/client/src/components/Planner/ReservationModal.test.tsx index a051bfb7c..f11bfc782 100644 --- a/client/src/components/Planner/ReservationModal.test.tsx +++ b/client/src/components/Planner/ReservationModal.test.tsx @@ -1,4 +1,4 @@ -// FE-PLANNER-RESMODAL-001 to FE-PLANNER-RESMODAL-106 +// FE-PLANNER-RESMODAL-001 to FE-PLANNER-RESMODAL-107 import { render, screen, waitFor, fireEvent, within, act } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; @@ -1441,6 +1441,22 @@ describe('ReservationModal', () => { ); }); + it('FE-PLANNER-RESMODAL-107: a create-expense click that saves nothing does not open the editor on the next plain save', async () => { + budgetEnabled(); + const onSave = vi.fn().mockResolvedValue({ id: 85 }); + const onOpenExpense = vi.fn(); + render(); + + // No title yet: the save returns early and nothing is created. + await userEvent.click(screen.getByRole('button', { name: /Create expense/i })); + expect(onSave).not.toHaveBeenCalled(); + + await userEvent.type(screen.getByPlaceholderText(/e\.g\. Lufthansa/i), 'Dinner'); + await userEvent.click(screen.getByRole('button', { name: /^Add$/i })); + await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1)); + expect(onOpenExpense).not.toHaveBeenCalled(); + }); + // ── File error paths ──────────────────────────────────────────────────────── it('FE-PLANNER-RESMODAL-077: a cancelled file dialog changes nothing', () => { diff --git a/client/src/components/Planner/useBookingExpenseIntent.test.ts b/client/src/components/Planner/useBookingExpenseIntent.test.ts index c3c0f2f30..872da5bc8 100644 --- a/client/src/components/Planner/useBookingExpenseIntent.test.ts +++ b/client/src/components/Planner/useBookingExpenseIntent.test.ts @@ -1,4 +1,4 @@ -// FE-PLANNER-BOOKINGEXPENSE-001 to -004: the expense wish of the booking and +// FE-PLANNER-BOOKINGEXPENSE-001 to -006: the expense wish of the booking and // transport forms, desktop and phone alike. import { act, renderHook } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; @@ -51,4 +51,33 @@ describe('useBookingExpenseIntent', () => { await expect(result.current.remove(buildBudgetItem())).resolves.toBeUndefined(); expect(submit).not.toHaveBeenCalled(); }); + + it('FE-PLANNER-BOOKINGEXPENSE-005: a save that ends without reading the wish does not leave it for the next save', async () => { + // The desktop reads the wish only after a successful save; an empty title + // returns before that point. + const submit = vi.fn(async () => undefined); + const { result } = renderHook(() => useBookingExpenseIntent(submit)); + await act(async () => result.current.create()); + expect(result.current.take()).toBeNull(); + }); + + it('FE-PLANNER-BOOKINGEXPENSE-006: a save that throws does not leave the wish for the next save', async () => { + let fail: (e: Error) => void = () => undefined; + const submit = vi.fn( + () => + new Promise((_resolve, reject) => { + fail = reject; + }) + ); + const unhandled = vi.fn(); + process.on('unhandledRejection', unhandled); + try { + const { result } = renderHook(() => useBookingExpenseIntent(submit)); + act(() => result.current.edit(buildBudgetItem({ id: 9 }))); + await act(async () => fail(new Error('save failed'))); + expect(result.current.take()).toBeNull(); + } finally { + process.off('unhandledRejection', unhandled); + } + }); }); diff --git a/client/src/components/Planner/useBookingExpenseIntent.ts b/client/src/components/Planner/useBookingExpenseIntent.ts index 01937019f..31f2a48b7 100644 --- a/client/src/components/Planner/useBookingExpenseIntent.ts +++ b/client/src/components/Planner/useBookingExpenseIntent.ts @@ -20,6 +20,16 @@ export interface BookingExpenseRemoval { export function useBookingExpenseIntent(submit: () => Promise, removal?: BookingExpenseRemoval) { const intentRef = useRef(null); + // The wish belongs to the save its click starts. A save that returns early + // (no title) or throws never reads it, and it must not open the editor on + // the next, unrelated save. A newer click's wish is left alone. + const saveWith = (intent: BookingExpenseIntent) => { + intentRef.current = intent; + void submit().finally(() => { + if (intentRef.current === intent) intentRef.current = null; + }); + }; + return { /** Forgets a wish left from an earlier opening. */ reset: () => { @@ -31,15 +41,9 @@ export function useBookingExpenseIntent(submit: () => Promise, removal? intentRef.current = null; return intent; }, - create: () => { - intentRef.current = { create: true }; - void submit(); - }, + create: () => saveWith({ create: true }), /** Saves the booking first, then opens the linked expense. */ - edit: (item: BudgetItem) => { - intentRef.current = { editItem: item }; - void submit(); - }, + edit: (item: BudgetItem) => saveWith({ editItem: item }), remove: async (item: BudgetItem) => { if (!removal) return; try {