Skip to content

Commit 5230c29

Browse files
committed
fix(queries): give optimistic rows collision-free ids
generateTempId used Date.now(), so two rows created in the same millisecond shared an id and the first server response overwrote both — leaving one row duplicated and the other's real id lost until a refetch. Now uses generateId(), matching what the workflow mutations already do. Reachable by double-clicking create, or by any scripted or bulk create. Also documents the contract of fetchOAuthConnections, which reports an unknown connection state as disconnected. No consumer reads that field today — both read names and icons, and connection state comes from useWorkspaceCredentials — so letting the query reject would blank the suggested-action rows and drop the credential page to raw provider ids. The note is what stops a future consumer branching on it silently.
1 parent fda3082 commit 5230c29

3 files changed

Lines changed: 44 additions & 1 deletion

File tree

‎apps/sim/hooks/queries/oauth/oauth-connections.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ function defineServices(): ServiceInfo[] {
5656
return servicesList
5757
}
5858

59+
/**
60+
* Resolves the service catalog merged with the caller's connections.
61+
*
62+
* A failed request resolves with the bare catalog rather than rejecting, so
63+
* consumers keep correct service names and ids when the merge data is
64+
* unavailable. The cost is that `isConnected`/`accounts` then report *unknown*
65+
* as *disconnected*, which the result cannot distinguish. Read connection
66+
* state from the workspace credentials query (`useWorkspaceCredentials`), which
67+
* surfaces its own errors; a consumer that must branch on `isConnected` here
68+
* needs this fallback removed first, or it will tell a connected user they are
69+
* not.
70+
*/
5971
async function fetchOAuthConnections(signal?: AbortSignal): Promise<ServiceInfo[]> {
6072
try {
6173
const serviceDefinitions = defineServices()
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
import { generateTempId } from '@/hooks/queries/utils/optimistic-mutation'
6+
7+
describe('generateTempId', () => {
8+
afterEach(() => {
9+
vi.useRealTimers()
10+
})
11+
12+
it('is unique for ids created within the same millisecond', () => {
13+
vi.useFakeTimers()
14+
vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z'))
15+
16+
const ids = new Set(Array.from({ length: 100 }, () => generateTempId('temp-folder')))
17+
18+
expect(ids.size).toBe(100)
19+
})
20+
21+
it('keeps the prefix so callers can identify optimistic rows', () => {
22+
expect(generateTempId('temp-folder').startsWith('temp-folder-')).toBe(true)
23+
})
24+
})

‎apps/sim/hooks/queries/utils/optimistic-mutation.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { generateId } from '@sim/utils/id'
23
import type { QueryClient } from '@tanstack/react-query'
34

45
const logger = createLogger('OptimisticMutation')
@@ -72,6 +73,12 @@ export function createOptimisticMutationHandlers<TData, TVariables, TItem>(
7273
}
7374
}
7475

76+
/**
77+
* Placeholder id for an optimistic row, held only until the server response
78+
* replaces it. Uses `generateId()` rather than a timestamp so two rows created
79+
* in the same millisecond cannot collide — a collision would make
80+
* `replaceOptimisticEntry` overwrite both entries with one server row.
81+
*/
7582
export function generateTempId(prefix: string): string {
76-
return `${prefix}-${Date.now()}`
83+
return `${prefix}-${generateId()}`
7784
}

0 commit comments

Comments
 (0)