Skip to content

Commit 878abc5

Browse files
committed
fix(org): fail mutations fast offline so a pending dialog can be cancelled
A mutation started offline paused until reconnect: the confirm dialog stayed pending with nothing dismissing it, and the action ran on reconnect regardless. Mutations now fail fast app-wide, and the retention re-read that gates a write does too.
1 parent b882040 commit 878abc5

4 files changed

Lines changed: 96 additions & 1 deletion

File tree

‎apps/sim/app/_shell/providers/get-query-client.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ export function makeQueryClient() {
2929
},
3030
mutations: {
3131
retry: false,
32+
/**
33+
* The default pauses a mutation started offline until reconnect. Nothing here persists or
34+
* shows that queue: the caller just waits, holding a pending dialog open, and the action
35+
* then runs long after the person gave up on it. Failing fast lets them see it and decide.
36+
*/
37+
networkMode: 'always',
3238
},
3339
},
3440
})

‎apps/sim/ee/data-retention/hooks/data-retention.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export function useOrganizationRetention(orgId: string | undefined) {
3737
queryFn: ({ signal }) => fetchDataRetention(orgId as string, signal),
3838
enabled: Boolean(orgId),
3939
staleTime: DATA_RETENTION_STALE_TIME,
40+
// A save re-reads this first: offline, that read must fail rather than wait and write on reconnect.
41+
networkMode: 'always',
4042
})
4143
}
4244

‎apps/sim/platform/app/(shell)/settings/_components/sections/data-retention/data-retention-section.round-trip.test.tsx‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from '@sim/testing/mocks/api-client-request.mock'
99
import { stubResizeObserver } from '@sim/testing/mocks/browser-apis.mock'
1010
import { emcnMock, emcnMockFns } from '@sim/testing/mocks/emcn.mock'
11-
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
11+
import { onlineManager, QueryClient, QueryClientProvider } from '@tanstack/react-query'
1212
import { createRoot, type Root } from 'react-dom/client'
1313
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1414
import {
@@ -161,6 +161,7 @@ describe('DataRetentionSection across saves', () => {
161161
})
162162

163163
afterEach(() => {
164+
onlineManager.setOnline(true)
164165
act(() => root.unmount())
165166
container.remove()
166167
queryClient.clear()
@@ -232,6 +233,18 @@ describe('DataRetentionSection across saves', () => {
232233
)
233234
})
234235

236+
it('stops at once while offline instead of waiting to write on reconnect', async () => {
237+
await click('Organization')
238+
await click('keep logs 7 days')
239+
onlineManager.setOnline(false)
240+
await failNextReads(() => click('Save'))
241+
242+
expect(emcnMockFns.mockToast.error).toHaveBeenCalledWith(
243+
'Could not confirm the latest retention settings. Nothing was saved.'
244+
)
245+
expect(server.writes).toEqual([])
246+
})
247+
235248
it('removes nothing over the document from before the failure', async () => {
236249
await click('Organization')
237250
await failNextReads(() => click('Remove'))
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/** @vitest-environment jsdom */
2+
3+
import { act } from 'react'
4+
import { flushMacrotask } from '@sim/testing/helpers/async'
5+
import { stubDialogMethods, stubResizeObserver } from '@sim/testing/mocks/browser-apis.mock'
6+
import { onlineManager, QueryClientProvider, useMutation } from '@tanstack/react-query'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
import { makeQueryClient } from '@/app/_shell/providers/get-query-client'
10+
import { ConfirmDialog } from '@/platform/design-system/components/modal/confirm-dialog'
11+
12+
const send = vi.fn()
13+
const onClose = vi.fn()
14+
15+
function RemoveDialog() {
16+
const remove = useMutation({ mutationFn: send })
17+
return (
18+
<ConfirmDialog
19+
target
20+
onClose={onClose}
21+
title='Remove'
22+
confirmLabel='Remove it'
23+
pendingLabel='Removing…'
24+
onConfirm={() => remove.mutateAsync(undefined)}
25+
/>
26+
)
27+
}
28+
29+
describe('ConfirmDialog while the browser is offline', () => {
30+
let container: HTMLDivElement
31+
let root: Root
32+
33+
const button = (name: string) =>
34+
Array.from(container.querySelectorAll('button')).find((node) => node.textContent === name)
35+
36+
beforeEach(async () => {
37+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
38+
stubResizeObserver()
39+
stubDialogMethods()
40+
send.mockReset().mockRejectedValue(new TypeError('Failed to fetch'))
41+
onlineManager.setOnline(false)
42+
container = window.document.createElement('div')
43+
window.document.body.appendChild(container)
44+
root = createRoot(container)
45+
await act(async () => {
46+
root.render(
47+
<QueryClientProvider client={makeQueryClient()}>
48+
<RemoveDialog />
49+
</QueryClientProvider>
50+
)
51+
})
52+
})
53+
54+
afterEach(() => {
55+
onlineManager.setOnline(true)
56+
act(() => root.unmount())
57+
container.remove()
58+
})
59+
60+
it('reports the failed action and can still be cancelled', async () => {
61+
await act(async () => {
62+
button('Remove it')?.click()
63+
await flushMacrotask()
64+
})
65+
66+
expect(send).toHaveBeenCalledOnce()
67+
expect(container.querySelector('[role="alert"]')?.textContent).toBe('Failed to fetch')
68+
69+
await act(async () => {
70+
button('Cancel')?.click()
71+
})
72+
expect(onClose).toHaveBeenCalledOnce()
73+
})
74+
})

0 commit comments

Comments
 (0)