fix(secrets): secrets/integrations component code cleanup (#4003)

* fix(secrets): secrets/integrations component code cleanup

* address comments
This commit is contained in:
Vikhyath Mondreti
2026-04-06 18:42:30 -07:00
committed by GitHub
parent 8df3f207d6
commit 5eb494de0c
3 changed files with 66 additions and 24 deletions

View File

@@ -323,7 +323,6 @@ export function CredentialsManager() {
const [selectedDescriptionDraft, setSelectedDescriptionDraft] = useState('')
const [copyIdSuccess, setCopyIdSuccess] = useState(false)
const [detailsError, setDetailsError] = useState<string | null>(null)
const [isSavingDetails, setIsSavingDetails] = useState(false)
const [showDetailUnsavedChanges, setShowDetailUnsavedChanges] = useState(false)
const [memberUserId, setMemberUserId] = useState('')
const [memberRole, setMemberRole] = useState<WorkspaceCredentialRole>('member')
@@ -350,8 +349,9 @@ export function CredentialsManager() {
[envCredentials, selectedCredentialId]
)
if (selectedCredential?.id !== prevSelectedCredentialId) {
setPrevSelectedCredentialId(selectedCredential?.id ?? null)
const currentCredentialId = selectedCredential?.id ?? null
if (currentCredentialId !== prevSelectedCredentialId) {
setPrevSelectedCredentialId(currentCredentialId)
if (!selectedCredential) {
setSelectedDescriptionDraft('')
setSelectedDisplayNameDraft('')
@@ -474,6 +474,11 @@ export function CredentialsManager() {
return personalInvalid || workspaceInvalid
}, [envVars, newWorkspaceRows])
const isListSaving =
savePersonalMutation.isPending ||
upsertWorkspaceMutation.isPending ||
removeWorkspaceMutation.isPending
hasChangesRef.current = hasChanges
shouldBlockNavRef.current = hasChanges || isDetailsDirty
@@ -652,12 +657,12 @@ export function CredentialsManager() {
)
const handleBackAttempt = useCallback(() => {
if (isDetailsDirty && !isSavingDetails) {
if (isDetailsDirty && !updateCredential.isPending) {
setShowDetailUnsavedChanges(true)
} else {
setSelectedCredentialId(null)
}
}, [isDetailsDirty, isSavingDetails])
}, [isDetailsDirty, updateCredential.isPending])
const handleDiscardDetailChanges = useCallback(() => {
setShowDetailUnsavedChanges(false)
@@ -667,9 +672,9 @@ export function CredentialsManager() {
}, [selectedCredential])
const handleSaveDetails = useCallback(async () => {
if (!selectedCredential || !isSelectedAdmin || !isDetailsDirty) return
if (!selectedCredential || !isSelectedAdmin || !isDetailsDirty || updateCredential.isPending)
return
setDetailsError(null)
setIsSavingDetails(true)
try {
if (isDisplayNameDirty || isDescriptionDirty) {
@@ -683,8 +688,6 @@ export function CredentialsManager() {
const message = error instanceof Error ? error.message : 'Failed to save changes'
setDetailsError(message)
logger.error('Failed to save secret details', error)
} finally {
setIsSavingDetails(false)
}
}, [
selectedCredential,
@@ -906,6 +909,8 @@ export function CredentialsManager() {
const handleCancel = resetToSaved
const handleSave = useCallback(async () => {
if (isListSaving) return
const prevInitialVars = [...initialVarsRef.current]
const prevInitialWorkspaceVars = { ...initialWorkspaceVarsRef.current }
@@ -964,6 +969,7 @@ export function CredentialsManager() {
logger.error('Failed to save environment variables:', error)
}
}, [
isListSaving,
envVars,
workspaceVars,
newWorkspaceRows,
@@ -1316,9 +1322,9 @@ export function CredentialsManager() {
<Button
variant='primary'
onClick={handleSaveDetails}
disabled={!isDetailsDirty || isSavingDetails}
disabled={!isDetailsDirty || updateCredential.isPending}
>
{isSavingDetails ? 'Saving...' : 'Save'}
{updateCredential.isPending ? 'Saving...' : 'Save'}
</Button>
)}
</div>
@@ -1416,11 +1422,13 @@ export function CredentialsManager() {
<Tooltip.Trigger asChild>
<Button
onClick={handleSave}
disabled={isLoading || !hasChanges || hasConflicts || hasInvalidKeys}
disabled={
isLoading || !hasChanges || hasConflicts || hasInvalidKeys || isListSaving
}
variant='primary'
className={`${hasConflicts || hasInvalidKeys ? 'cursor-not-allowed opacity-50' : ''}`}
>
Save
{isListSaving ? 'Saving...' : 'Save'}
</Button>
</Tooltip.Trigger>
{hasConflicts && <Tooltip.Content>Resolve all conflicts before saving</Tooltip.Content>}

View File

@@ -246,12 +246,11 @@ export function IntegrationsManager() {
}, [selectedCredential, selectedDisplayNameDraft])
const isDetailsDirty = isDescriptionDirty || isDisplayNameDirty
const [isSavingDetails, setIsSavingDetails] = useState(false)
const handleSaveDetails = async () => {
if (!selectedCredential || !isSelectedAdmin || !isDetailsDirty) return
if (!selectedCredential || !isSelectedAdmin || !isDetailsDirty || updateCredential.isPending)
return
setDetailsError(null)
setIsSavingDetails(true)
try {
if (isDisplayNameDirty || isDescriptionDirty) {
@@ -263,26 +262,22 @@ export function IntegrationsManager() {
if (isDisplayNameDirty) setSelectedDisplayNameDraft((v) => v.trim())
if (isDescriptionDirty) setSelectedDescriptionDraft((v) => v.trim())
}
await refetchCredentials()
} catch (error: unknown) {
const message = error instanceof Error ? error.message : 'Failed to save changes'
setDetailsError(message)
logger.error('Failed to save credential details', error)
} finally {
setIsSavingDetails(false)
}
}
const handleBackAttempt = useCallback(() => {
if (isDetailsDirty && !isSavingDetails) {
if (isDetailsDirty && !updateCredential.isPending) {
setShowUnsavedChangesAlert(true)
} else {
setSelectedCredentialId(null)
setSelectedDescriptionDraft('')
setSelectedDisplayNameDraft('')
}
}, [isDetailsDirty, isSavingDetails])
}, [isDetailsDirty, updateCredential.isPending])
const handleDiscardChanges = useCallback(() => {
setShowUnsavedChangesAlert(false)
@@ -1430,9 +1425,9 @@ export function IntegrationsManager() {
<Button
variant='primary'
onClick={handleSaveDetails}
disabled={!isDetailsDirty || isSavingDetails}
disabled={!isDetailsDirty || updateCredential.isPending}
>
{isSavingDetails ? 'Saving...' : 'Save'}
{updateCredential.isPending ? 'Saving...' : 'Save'}
</Button>
)}
</div>

View File

@@ -223,6 +223,45 @@ export function useUpdateWorkspaceCredential() {
}
return response.json()
},
onMutate: async (variables) => {
await queryClient.cancelQueries({
queryKey: workspaceCredentialKeys.detail(variables.credentialId),
})
await queryClient.cancelQueries({ queryKey: workspaceCredentialKeys.lists() })
const previousLists = queryClient.getQueriesData<WorkspaceCredential[]>({
queryKey: workspaceCredentialKeys.lists(),
})
queryClient.setQueriesData<WorkspaceCredential[]>(
{ queryKey: workspaceCredentialKeys.lists() },
(old) => {
if (!old) return old
return old.map((cred) =>
cred.id === variables.credentialId
? {
...cred,
...(variables.displayName !== undefined
? { displayName: variables.displayName }
: {}),
...(variables.description !== undefined
? { description: variables.description ?? null }
: {}),
}
: cred
)
}
)
return { previousLists }
},
onError: (_err, _variables, context) => {
if (context?.previousLists) {
for (const [queryKey, data] of context.previousLists) {
queryClient.setQueryData(queryKey, data)
}
}
},
onSettled: (_data, _error, variables) => {
queryClient.invalidateQueries({
queryKey: workspaceCredentialKeys.detail(variables.credentialId),