Skip to content

Commit 98be968

Browse files
authored
improvement(secrets): parallelize save mutations and add admin visibility for workspace secrets (#4032)
* improvement(secrets): parallelize save mutations and add admin visibility for workspace secrets * fix(secrets): sequence workspace upsert/delete to avoid read-modify-write race * fix(secrets): use Promise.allSettled to ensure credential invalidation after all mutations settle
1 parent e0f5cf8 commit 98be968

File tree

2 files changed

+65
-47
lines changed

2 files changed

+65
-47
lines changed

apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
5+
import { useQueryClient } from '@tanstack/react-query'
56
import { Check, Clipboard, Key, Search } from 'lucide-react'
67
import { useParams, useRouter } from 'next/navigation'
78
import {
@@ -42,6 +43,7 @@ import {
4243
useWorkspaceCredentials,
4344
type WorkspaceCredential,
4445
type WorkspaceCredentialRole,
46+
workspaceCredentialKeys,
4547
} from '@/hooks/queries/credentials'
4648
import {
4749
usePersonalEnvironment,
@@ -125,6 +127,7 @@ interface WorkspaceVariableRowProps {
125127
renamingKey: string | null
126128
pendingKeyValue: string
127129
hasCredential: boolean
130+
isAdmin: boolean
128131
onRenameStart: (key: string) => void
129132
onPendingKeyChange: (value: string) => void
130133
onRenameEnd: (key: string, value: string) => void
@@ -138,12 +141,18 @@ function WorkspaceVariableRow({
138141
renamingKey,
139142
pendingKeyValue,
140143
hasCredential,
144+
isAdmin,
141145
onRenameStart,
142146
onPendingKeyChange,
143147
onRenameEnd,
144148
onDelete,
145149
onViewDetails,
146150
}: WorkspaceVariableRowProps) {
151+
const [valueFocused, setValueFocused] = useState(false)
152+
153+
const maskedValueStyle =
154+
isAdmin && !valueFocused ? ({ WebkitTextSecurity: 'disc' } as React.CSSProperties) : undefined
155+
147156
return (
148157
<div className='contents'>
149158
<EmcnInput
@@ -163,12 +172,19 @@ function WorkspaceVariableRow({
163172
/>
164173
<div />
165174
<EmcnInput
166-
value={value ? '\u2022'.repeat(value.length) : ''}
175+
value={isAdmin ? value : value ? '\u2022'.repeat(value.length) : ''}
167176
readOnly
177+
onFocus={() => {
178+
if (isAdmin) setValueFocused(true)
179+
}}
180+
onBlur={() => {
181+
if (isAdmin) setValueFocused(false)
182+
}}
168183
autoComplete='off'
169184
autoCorrect='off'
170185
autoCapitalize='off'
171186
spellCheck='false'
187+
style={maskedValueStyle}
172188
className='h-9'
173189
/>
174190
<Button
@@ -298,6 +314,14 @@ export function CredentialsManager() {
298314
)
299315

300316
const { data: workspacePermissions } = useWorkspacePermissionsQuery(workspaceId || null)
317+
const queryClient = useQueryClient()
318+
319+
const isAdmin = useMemo(() => {
320+
const userId = session?.user?.id
321+
if (!userId || !workspacePermissions?.users) return false
322+
const currentUser = workspacePermissions.users.find((user) => user.userId === userId)
323+
return currentUser?.permissionType === 'admin'
324+
}, [session?.user?.id, workspacePermissions?.users])
301325

302326
const isLoading = isPersonalLoading || isWorkspaceLoading
303327
const variables = useMemo(() => personalEnvData || {}, [personalEnvData])
@@ -923,6 +947,7 @@ export function CredentialsManager() {
923947

924948
const prevInitialVars = [...initialVarsRef.current]
925949
const prevInitialWorkspaceVars = { ...initialWorkspaceVarsRef.current }
950+
const mutations: Promise<unknown>[] = []
926951

927952
try {
928953
setShowUnsavedChanges(false)
@@ -944,8 +969,6 @@ export function CredentialsManager() {
944969
.filter((v) => v.key && v.value)
945970
.reduce<Record<string, string>>((acc, { key, value }) => ({ ...acc, [key]: value }), {})
946971

947-
await savePersonalMutation.mutateAsync({ variables: validVariables })
948-
949972
const before = prevInitialWorkspaceVars
950973
const after = mergedWorkspaceVars
951974
const toUpsert: Record<string, string> = {}
@@ -961,33 +984,52 @@ export function CredentialsManager() {
961984
if (!(k in after)) toDelete.push(k)
962985
}
963986

964-
if (workspaceId) {
965-
if (Object.keys(toUpsert).length) {
966-
await upsertWorkspaceMutation.mutateAsync({ workspaceId, variables: toUpsert })
967-
}
968-
if (toDelete.length) {
969-
await removeWorkspaceMutation.mutateAsync({ workspaceId, keys: toDelete })
987+
const personalChanged = (() => {
988+
const initialMap = new Map(
989+
prevInitialVars.filter((v) => v.key && v.value).map((v) => [v.key, v.value])
990+
)
991+
const currentKeys = Object.keys(validVariables)
992+
if (initialMap.size !== currentKeys.length) return true
993+
for (const [key, value] of Object.entries(validVariables)) {
994+
if (initialMap.get(key) !== value) return true
970995
}
996+
return false
997+
})()
998+
999+
if (personalChanged) {
1000+
mutations.push(savePersonalMutation.mutateAsync({ variables: validVariables }))
1001+
}
1002+
if (workspaceId && (Object.keys(toUpsert).length || toDelete.length)) {
1003+
mutations.push(
1004+
(async () => {
1005+
if (Object.keys(toUpsert).length) {
1006+
await upsertWorkspaceMutation.mutateAsync({ workspaceId, variables: toUpsert })
1007+
}
1008+
if (toDelete.length) {
1009+
await removeWorkspaceMutation.mutateAsync({ workspaceId, keys: toDelete })
1010+
}
1011+
})()
1012+
)
9711013
}
9721014

1015+
const results = await Promise.allSettled(mutations)
1016+
const firstFailure = results.find((r): r is PromiseRejectedResult => r.status === 'rejected')
1017+
if (firstFailure) throw firstFailure.reason
1018+
9731019
setWorkspaceVars(mergedWorkspaceVars)
9741020
setNewWorkspaceRows([createEmptyEnvVar()])
9751021
} catch (error) {
9761022
hasSavedRef.current = false
9771023
initialVarsRef.current = prevInitialVars
9781024
initialWorkspaceVarsRef.current = prevInitialWorkspaceVars
9791025
logger.error('Failed to save environment variables:', error)
1026+
} finally {
1027+
if (mutations.length > 0) {
1028+
queryClient.invalidateQueries({ queryKey: workspaceCredentialKeys.lists() })
1029+
}
9801030
}
981-
}, [
982-
isListSaving,
983-
envVars,
984-
workspaceVars,
985-
newWorkspaceRows,
986-
workspaceId,
987-
savePersonalMutation,
988-
upsertWorkspaceMutation,
989-
removeWorkspaceMutation,
990-
])
1031+
// eslint-disable-next-line react-hooks/exhaustive-deps -- mutation objects and queryClient are stable (TanStack Query v5)
1032+
}, [isListSaving, envVars, workspaceVars, newWorkspaceRows, workspaceId])
9911033

9921034
const handleDiscardAndNavigate = useCallback(() => {
9931035
shouldBlockNavRef.current = false
@@ -1494,6 +1536,7 @@ export function CredentialsManager() {
14941536
renamingKey={renamingKey}
14951537
pendingKeyValue={pendingKeyValue}
14961538
hasCredential={envKeyToCredential.has(key)}
1539+
isAdmin={isAdmin}
14971540
onRenameStart={setRenamingKey}
14981541
onPendingKeyChange={setPendingKeyValue}
14991542
onRenameEnd={handleWorkspaceKeyRename}

apps/sim/hooks/queries/environment.ts

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { keepPreviousData, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
3-
import type { EnvironmentVariable, WorkspaceEnvironmentData } from '@/lib/environment/api'
3+
import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
44
import { fetchPersonalEnvironment, fetchWorkspaceEnvironment } from '@/lib/environment/api'
5-
import { workspaceCredentialKeys } from '@/hooks/queries/credentials'
65
import { API_ENDPOINTS } from '@/stores/constants'
76

87
const logger = createLogger('EnvironmentQueries')
@@ -56,40 +55,20 @@ export function useSavePersonalEnvironment() {
5655

5756
return useMutation({
5857
mutationFn: async ({ variables }: SavePersonalEnvironmentParams) => {
59-
const transformedVariables = Object.entries(variables).reduce(
60-
(acc, [key, value]) => ({
61-
...acc,
62-
[key]: { key, value },
63-
}),
64-
{}
65-
)
66-
6758
const response = await fetch(API_ENDPOINTS.ENVIRONMENT, {
6859
method: 'POST',
69-
headers: {
70-
'Content-Type': 'application/json',
71-
},
72-
body: JSON.stringify({
73-
variables: Object.entries(transformedVariables).reduce(
74-
(acc, [key, value]) => ({
75-
...acc,
76-
[key]: (value as EnvironmentVariable).value,
77-
}),
78-
{}
79-
),
80-
}),
60+
headers: { 'Content-Type': 'application/json' },
61+
body: JSON.stringify({ variables }),
8162
})
8263

8364
if (!response.ok) {
8465
throw new Error(`Failed to save environment variables: ${response.statusText}`)
8566
}
8667

8768
logger.info('Saved personal environment variables')
88-
return transformedVariables
8969
},
9070
onSettled: () => {
9171
queryClient.invalidateQueries({ queryKey: environmentKeys.personal() })
92-
queryClient.invalidateQueries({ queryKey: workspaceCredentialKeys.lists() })
9372
},
9473
})
9574
}
@@ -124,8 +103,6 @@ export function useUpsertWorkspaceEnvironment() {
124103
queryClient.invalidateQueries({
125104
queryKey: environmentKeys.workspace(variables.workspaceId),
126105
})
127-
queryClient.invalidateQueries({ queryKey: environmentKeys.personal() })
128-
queryClient.invalidateQueries({ queryKey: workspaceCredentialKeys.lists() })
129106
},
130107
})
131108
}
@@ -160,8 +137,6 @@ export function useRemoveWorkspaceEnvironment() {
160137
queryClient.invalidateQueries({
161138
queryKey: environmentKeys.workspace(variables.workspaceId),
162139
})
163-
queryClient.invalidateQueries({ queryKey: environmentKeys.personal() })
164-
queryClient.invalidateQueries({ queryKey: workspaceCredentialKeys.lists() })
165140
},
166141
})
167142
}

0 commit comments

Comments
 (0)