Skip to content

Commit 5d807d0

Browse files
committed
FIX: keep a late create response out of a later dialog opening
The success path reset the form unconditionally, so a create response that outlived its opening wiped the opening the user had moved on to: the typed registry name, the selected type and its parameter rows all disappeared, and reset() also cleared a submission error they were reading. Gate the reset on the opening the request was submitted from, the same way the failure path already gates setError, and leave onCreated ungated so a late success still refreshes the list. The submitting flag needed the same treatment, and clearing it when the dialog opens rather than when a stale response lands, so a request from a previous opening no longer re-enables the primary action of an opening that has its own request in flight.
1 parent fdc8f4b commit 5d807d0

2 files changed

Lines changed: 45 additions & 3 deletions

File tree

‎frontend/src/components/Registry/CreateConverterDialog.test.tsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,37 @@ describe('CreateConverterDialog', () => {
315315
await waitFor(() => expect(alert).toHaveFocus())
316316
})
317317

318+
it('should not clear a later opening of the dialog when an earlier creation succeeds', async () => {
319+
let resolveCreate: ((value: { converter_id: string }) => void) | undefined
320+
mockedConvertersApi.createConverter.mockImplementation(
321+
() => new Promise((resolve) => { resolveCreate = resolve }),
322+
)
323+
const onCreated = jest.fn()
324+
const user = userEvent.setup()
325+
const { rerender } = renderDialog({ onCreated })
326+
await submitCaesarConverter(user)
327+
await waitFor(() => expect(mockedConvertersApi.createConverter).toHaveBeenCalled())
328+
329+
// Dismissed while the request was in flight, then opened again and filled in.
330+
rerender(dialogTree({ open: false, onCreated }))
331+
rerender(dialogTree({ open: true, onCreated }))
332+
await screen.findByRole('combobox', { name: /^converter type$/i })
333+
await selectConverterType('CaesarConverter')
334+
// Selecting a type prefills the registry name, so replace it rather than append.
335+
await user.clear(screen.getByLabelText(/registry name/i))
336+
await user.type(screen.getByLabelText(/registry name/i), 'my-second-converter')
337+
338+
await act(async () => {
339+
resolveCreate?.({ converter_id: 'first-converter' })
340+
})
341+
342+
// The list still refreshes, but the opening in front of the user is untouched.
343+
expect(onCreated).toHaveBeenCalledWith('first-converter')
344+
expect(screen.getByLabelText(/registry name/i)).toHaveValue('my-second-converter')
345+
expect(screen.getByLabelText(/caesar_offset/i)).toBeInTheDocument()
346+
expect(screen.getByRole('button', { name: 'Add Converter' })).toBeInTheDocument()
347+
})
348+
318349
it('should not surface a submission error in a later opening of the dialog', async () => {
319350
let rejectCreate: ((reason: Error) => void) | undefined
320351
mockedConvertersApi.createConverter.mockImplementation(

‎frontend/src/components/Registry/CreateConverterDialog.tsx‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ export default function CreateConverterDialog({
184184
const openEpochRef = useRef(0)
185185

186186
useEffect(() => {
187-
// Every change of `open` ends the opening before it.
187+
// Every change of `open` ends the opening before it, so a response from the
188+
// previous one leaves this opening's own state alone.
188189
openEpochRef.current += 1
189190
if (!open) return
190191
let cancelled = false
@@ -193,6 +194,9 @@ export default function CreateConverterDialog({
193194
if (cancelled) return null
194195
setLoading(true)
195196
setError(null)
197+
// A request from the previous opening keeps its own "Adding..." state, so
198+
// clear it here rather than letting that response clear it for this one.
199+
setSubmitting(false)
196200
return Promise.all([
197201
convertersApi.listConverterTypes(),
198202
targetsApi.listTargets(200),
@@ -335,7 +339,12 @@ export default function CreateConverterDialog({
335339
type: selectedType,
336340
params: parameterValues,
337341
})
338-
reset()
342+
// Only the opening this request was submitted from is cleared: a response
343+
// that outlived its opening must not wipe the form the user is filling in
344+
// now. onCreated stays ungated so a late success still refreshes the list.
345+
if (openEpochRef.current === epoch) {
346+
reset()
347+
}
339348
onCreated(response.converter_id)
340349
} catch (err) {
341350
// A failure from an opening the user has already left stays out of the
@@ -344,7 +353,9 @@ export default function CreateConverterDialog({
344353
setError({ message: toApiError(err).detail, fromSubmit: true })
345354
}
346355
} finally {
347-
setSubmitting(false)
356+
if (openEpochRef.current === epoch) {
357+
setSubmitting(false)
358+
}
348359
}
349360
}
350361

0 commit comments

Comments
 (0)