improvement(knowledge): show selector with saved option in connector edit modal (#4240)

* improvement(knowledge): show selector with saved option in connector edit modal

* fix(kb-connectors): clear canonical siblings when non-canonical dep changes; share selector field

* refactor(kb-connectors): extract canonical-field logic into useConnectorConfigFields hook

* fix(kb-connectors): only merge changed fields into sourceConfig on edit save

Avoids writing spurious empty-string keys for untouched optional fields when
another field triggers a save.

* refactor(kb-connectors): tighten state primitives in modals

- edit modal: replace useMemo([]) + eslint-disable with useState lazy
  initializer for initialSourceConfig — same mount-once semantics
  without the escape hatch.
- add modal: drop useCallback on handleConnectNewAccount (no observer
  saw the reference) and inline the one call site.
This commit is contained in:
Waleed
2026-04-20 20:49:58 -07:00
committed by GitHub
parent 802f4cf0fc
commit 94202ed229
5 changed files with 308 additions and 171 deletions
@@ -1,6 +1,6 @@
'use client'
import { useCallback, useMemo, useState } from 'react'
import { useMemo, useState } from 'react'
import { ArrowLeft, ArrowLeftRight, Loader2, Plus, Search } from 'lucide-react'
import { useParams } from 'next/navigation'
import {
@@ -23,11 +23,11 @@ import { getSubscriptionAccessState } from '@/lib/billing/client'
import { consumeOAuthReturnContext } from '@/lib/credentials/client-state'
import { getProviderIdFromServiceId, type OAuthProvider } from '@/lib/oauth'
import { OAuthModal } from '@/app/workspace/[workspaceId]/components/oauth-modal'
import { ConnectorSelectorField } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/components/connector-selector-field'
import { ConnectorSelectorField } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field'
import { SYNC_INTERVALS } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/consts'
import { MaxBadge } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/max-badge'
import { useConnectorConfigFields } from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields'
import { isBillingEnabled } from '@/app/workspace/[workspaceId]/settings/navigation'
import { getDependsOnFields } from '@/blocks/utils'
import { CONNECTOR_REGISTRY } from '@/connectors/registry'
import type { ConnectorConfig, ConnectorConfigField } from '@/connectors/types'
import { useCreateConnector } from '@/hooks/queries/kb/connectors'
@@ -57,13 +57,11 @@ export function AddConnectorModal({
}: AddConnectorModalProps) {
const [step, setStep] = useState<Step>(() => (initialConnectorType ? 'configure' : 'select-type'))
const [selectedType, setSelectedType] = useState<string | null>(initialConnectorType ?? null)
const [sourceConfig, setSourceConfig] = useState<Record<string, string>>({})
const [syncInterval, setSyncInterval] = useState(1440)
const [selectedCredentialId, setSelectedCredentialId] = useState<string | null>(null)
const [disabledTagIds, setDisabledTagIds] = useState<Set<string>>(() => new Set())
const [error, setError] = useState<string | null>(null)
const [showOAuthModal, setShowOAuthModal] = useState(false)
const [canonicalModes, setCanonicalModes] = useState<Record<string, 'basic' | 'advanced'>>({})
const [apiKeyValue, setApiKeyValue] = useState('')
const [apiKeyFocused, setApiKeyFocused] = useState(false)
@@ -100,54 +98,17 @@ export function AddConnectorModal({
const effectiveCredentialId =
selectedCredentialId ?? (credentials.length === 1 ? credentials[0].id : null)
const canonicalGroups = useMemo(() => {
if (!connectorConfig) return new Map<string, ConnectorConfigField[]>()
const groups = new Map<string, ConnectorConfigField[]>()
for (const field of connectorConfig.configFields) {
if (field.canonicalParamId) {
const existing = groups.get(field.canonicalParamId)
if (existing) {
existing.push(field)
} else {
groups.set(field.canonicalParamId, [field])
}
}
}
return groups
}, [connectorConfig])
const dependentFieldIds = useMemo(() => {
if (!connectorConfig) return new Map<string, string[]>()
const map = new Map<string, string[]>()
for (const field of connectorConfig.configFields) {
const deps = getDependsOnFields(field.dependsOn)
for (const dep of deps) {
const existing = map.get(dep) ?? []
existing.push(field.id)
map.set(dep, existing)
}
}
for (const group of canonicalGroups.values()) {
const allDependents = new Set<string>()
for (const field of group) {
for (const dep of map.get(field.id) ?? []) {
allDependents.add(dep)
const depField = connectorConfig.configFields.find((f) => f.id === dep)
if (depField?.canonicalParamId) {
for (const sibling of canonicalGroups.get(depField.canonicalParamId) ?? []) {
allDependents.add(sibling.id)
}
}
}
}
if (allDependents.size > 0) {
for (const field of group) {
map.set(field.id, [...allDependents])
}
}
}
return map
}, [connectorConfig, canonicalGroups])
const {
sourceConfig,
setSourceConfig,
canonicalModes,
setCanonicalModes,
canonicalGroups,
isFieldVisible,
handleFieldChange,
toggleCanonicalMode,
resolveSourceConfig,
} = useConnectorConfigFields({ connectorConfig })
const handleSelectType = (type: string) => {
setSelectedType(type)
@@ -163,64 +124,6 @@ export function AddConnectorModal({
onConnectorTypeChange?.(type)
}
const handleFieldChange = useCallback(
(fieldId: string, value: string) => {
setSourceConfig((prev) => {
const next = { ...prev, [fieldId]: value }
const toClear = dependentFieldIds.get(fieldId)
if (toClear) {
for (const depId of toClear) {
next[depId] = ''
}
}
return next
})
},
[dependentFieldIds]
)
const toggleCanonicalMode = useCallback((canonicalId: string) => {
setCanonicalModes((prev) => ({
...prev,
[canonicalId]: prev[canonicalId] === 'advanced' ? 'basic' : 'advanced',
}))
}, [])
const isFieldVisible = useCallback(
(field: ConnectorConfigField): boolean => {
if (!field.canonicalParamId || !field.mode) return true
const activeMode = canonicalModes[field.canonicalParamId] ?? 'basic'
return field.mode === activeMode
},
[canonicalModes]
)
const resolveSourceConfig = useCallback((): Record<string, string> => {
const resolved: Record<string, string> = {}
const processedCanonicals = new Set<string>()
if (!connectorConfig) return resolved
for (const field of connectorConfig.configFields) {
if (field.canonicalParamId) {
if (processedCanonicals.has(field.canonicalParamId)) continue
processedCanonicals.add(field.canonicalParamId)
const group = canonicalGroups.get(field.canonicalParamId)
if (!group) continue
const activeMode = canonicalModes[field.canonicalParamId] ?? 'basic'
const activeField = group.find((f) => f.mode === activeMode) ?? group[0]
const value = sourceConfig[activeField.id]
if (value) resolved[field.canonicalParamId] = value
} else {
if (sourceConfig[field.id]) resolved[field.id] = sourceConfig[field.id]
}
}
return resolved
}, [connectorConfig, canonicalGroups, canonicalModes, sourceConfig])
const canSubmit = useMemo(() => {
if (!connectorConfig) return false
if (isApiKeyMode) {
@@ -249,7 +152,10 @@ export function AddConnectorModal({
setError(null)
const resolvedConfig = resolveSourceConfig()
const resolvedConfig: Record<string, unknown> = {}
for (const [key, value] of Object.entries(resolveSourceConfig())) {
if (value) resolvedConfig[key] = value
}
const finalSourceConfig =
disabledTagIds.size > 0
? { ...resolvedConfig, disabledTagIds: Array.from(disabledTagIds) }
@@ -274,10 +180,6 @@ export function AddConnectorModal({
)
}
const handleConnectNewAccount = useCallback(() => {
setShowOAuthModal(true)
}, [])
const filteredEntries = useMemo(() => {
const term = searchTerm.toLowerCase().trim()
if (!term) return CONNECTOR_ENTRIES
@@ -385,9 +287,7 @@ export function AddConnectorModal({
: `Connect ${connectorConfig.name} account`,
value: '__connect_new__',
icon: Plus,
onSelect: () => {
void handleConnectNewAccount()
},
onSelect: () => setShowOAuthModal(true),
},
]}
value={effectiveCredentialId ?? undefined}
@@ -0,0 +1 @@
export { ConnectorSelectorField } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field'
@@ -2,7 +2,7 @@
import { useMemo, useState } from 'react'
import { createLogger } from '@sim/logger'
import { ExternalLink, Loader2, RotateCcw } from 'lucide-react'
import { ArrowLeftRight, ExternalLink, Loader2, RotateCcw } from 'lucide-react'
import {
Button,
ButtonGroup,
@@ -20,13 +20,16 @@ import {
ModalTabsList,
ModalTabsTrigger,
Skeleton,
Tooltip,
} from '@/components/emcn'
import { getSubscriptionAccessState } from '@/lib/billing/client'
import { ConnectorSelectorField } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field'
import { SYNC_INTERVALS } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/consts'
import { MaxBadge } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/max-badge'
import { useConnectorConfigFields } from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields'
import { isBillingEnabled } from '@/app/workspace/[workspaceId]/settings/navigation'
import { CONNECTOR_REGISTRY } from '@/connectors/registry'
import type { ConnectorConfig } from '@/connectors/types'
import type { ConnectorConfig, ConnectorConfigField } from '@/connectors/types'
import type { ConnectorData } from '@/hooks/queries/kb/connectors'
import {
useConnectorDocuments,
@@ -35,6 +38,7 @@ import {
useUpdateConnector,
} from '@/hooks/queries/kb/connectors'
import { useSubscriptionData } from '@/hooks/queries/subscription'
import type { SelectorKey } from '@/hooks/selectors/types'
const logger = createLogger('EditConnectorModal')
@@ -56,21 +60,42 @@ export function EditConnectorModal({
}: EditConnectorModalProps) {
const connectorConfig = CONNECTOR_REGISTRY[connector.connectorType] ?? null
const initialSourceConfig = useMemo(() => {
const config: Record<string, string> = {}
for (const [key, value] of Object.entries(connector.sourceConfig)) {
if (!INTERNAL_CONFIG_KEYS.has(key)) {
config[key] = String(value ?? '')
}
}
return config
}, [connector.sourceConfig])
const [activeTab, setActiveTab] = useState('settings')
const [sourceConfig, setSourceConfig] = useState<Record<string, string>>(initialSourceConfig)
const [syncInterval, setSyncInterval] = useState(connector.syncIntervalMinutes)
const [error, setError] = useState<string | null>(null)
/**
* Seeds from the stored canonical config. For canonical-pair fields (selector +
* manual input), both field IDs get the same value so toggling preserves it.
* Captured once on mount; editing state is owned by the hook afterward.
*/
const [initialSourceConfig] = useState<Record<string, string>>(() => {
const config: Record<string, string> = {}
if (!connectorConfig) {
for (const [key, value] of Object.entries(connector.sourceConfig)) {
if (!INTERNAL_CONFIG_KEYS.has(key)) config[key] = String(value ?? '')
}
return config
}
for (const field of connectorConfig.configFields) {
const canonicalId = field.canonicalParamId ?? field.id
if (INTERNAL_CONFIG_KEYS.has(canonicalId)) continue
const rawValue = connector.sourceConfig[canonicalId]
if (rawValue !== undefined) config[field.id] = String(rawValue ?? '')
}
return config
})
const {
sourceConfig,
canonicalModes,
canonicalGroups,
isFieldVisible,
handleFieldChange,
toggleCanonicalMode,
resolveSourceConfig,
} = useConnectorConfigFields({ connectorConfig, initialSourceConfig })
const { mutate: updateConnector, isPending: isSaving } = useUpdateConnector()
const { data: subscriptionResponse } = useSubscriptionData({ enabled: isBillingEnabled })
@@ -79,11 +104,12 @@ export function EditConnectorModal({
const hasChanges = useMemo(() => {
if (syncInterval !== connector.syncIntervalMinutes) return true
for (const [key, value] of Object.entries(sourceConfig)) {
const resolved = resolveSourceConfig()
for (const [key, value] of Object.entries(resolved)) {
if (String(connector.sourceConfig[key] ?? '') !== value) return true
}
return false
}, [sourceConfig, syncInterval, connector.syncIntervalMinutes, connector.sourceConfig])
}, [resolveSourceConfig, syncInterval, connector.syncIntervalMinutes, connector.sourceConfig])
const handleSave = () => {
setError(null)
@@ -94,11 +120,13 @@ export function EditConnectorModal({
updates.syncIntervalMinutes = syncInterval
}
const configChanged = Object.entries(sourceConfig).some(
([key, value]) => String(connector.sourceConfig[key] ?? '') !== value
)
if (configChanged) {
updates.sourceConfig = { ...connector.sourceConfig, ...sourceConfig }
const resolved = resolveSourceConfig()
const changedEntries: Record<string, string> = {}
for (const [key, value] of Object.entries(resolved)) {
if (String(connector.sourceConfig[key] ?? '') !== value) changedEntries[key] = value
}
if (Object.keys(changedEntries).length > 0) {
updates.sourceConfig = { ...connector.sourceConfig, ...changedEntries }
}
if (Object.keys(updates).length === 0) {
@@ -144,10 +172,16 @@ export function EditConnectorModal({
<SettingsTab
connectorConfig={connectorConfig}
sourceConfig={sourceConfig}
setSourceConfig={setSourceConfig}
credentialId={connector.credentialId}
canonicalGroups={canonicalGroups}
canonicalModes={canonicalModes}
onToggleCanonicalMode={toggleCanonicalMode}
onFieldChange={handleFieldChange}
isFieldVisible={isFieldVisible}
syncInterval={syncInterval}
setSyncInterval={setSyncInterval}
hasMaxAccess={hasMaxAccess}
isSaving={isSaving}
error={error}
/>
</ModalTabsContent>
@@ -183,53 +217,102 @@ export function EditConnectorModal({
interface SettingsTabProps {
connectorConfig: ConnectorConfig | null
sourceConfig: Record<string, string>
setSourceConfig: React.Dispatch<React.SetStateAction<Record<string, string>>>
credentialId: string | null
canonicalGroups: Map<string, ConnectorConfigField[]>
canonicalModes: Record<string, 'basic' | 'advanced'>
onToggleCanonicalMode: (canonicalId: string) => void
onFieldChange: (fieldId: string, value: string) => void
isFieldVisible: (field: ConnectorConfigField) => boolean
syncInterval: number
setSyncInterval: (v: number) => void
hasMaxAccess: boolean
isSaving: boolean
error: string | null
}
function SettingsTab({
connectorConfig,
sourceConfig,
setSourceConfig,
credentialId,
canonicalGroups,
canonicalModes,
onToggleCanonicalMode,
onFieldChange,
isFieldVisible,
syncInterval,
setSyncInterval,
hasMaxAccess,
isSaving,
error,
}: SettingsTabProps) {
return (
<div className='flex flex-col gap-3'>
{connectorConfig?.configFields.map((field) => (
<div key={field.id} className='flex flex-col gap-2'>
<Label>
{field.title}
{field.required && <span className='ml-0.5 text-[var(--text-error)]'>*</span>}
</Label>
{field.description && (
<p className='text-[var(--text-muted)] text-xs'>{field.description}</p>
)}
{field.type === 'dropdown' && field.options ? (
<Combobox
size='sm'
options={field.options.map((opt) => ({
label: opt.label,
value: opt.id,
}))}
value={sourceConfig[field.id] || undefined}
onChange={(value) => setSourceConfig((prev) => ({ ...prev, [field.id]: value }))}
placeholder={field.placeholder || `Select ${field.title.toLowerCase()}`}
/>
) : (
<Input
value={sourceConfig[field.id] || ''}
onChange={(e) => setSourceConfig((prev) => ({ ...prev, [field.id]: e.target.value }))}
placeholder={field.placeholder}
/>
)}
</div>
))}
{connectorConfig?.configFields.map((field) => {
if (!isFieldVisible(field)) return null
const canonicalId = field.canonicalParamId
const hasCanonicalPair =
canonicalId && (canonicalGroups.get(canonicalId)?.length ?? 0) === 2
return (
<div key={field.id} className='flex flex-col gap-2'>
<div className='flex items-center justify-between'>
<Label>
{field.title}
{field.required && <span className='ml-0.5 text-[var(--text-error)]'>*</span>}
</Label>
{hasCanonicalPair && canonicalId && (
<Tooltip.Root>
<Tooltip.Trigger asChild>
<button
type='button'
className='flex h-[18px] w-[18px] items-center justify-center rounded-[3px] text-[var(--text-muted)] transition-colors hover-hover:bg-[var(--surface-3)] hover-hover:text-[var(--text-secondary)]'
onClick={() => onToggleCanonicalMode(canonicalId)}
>
<ArrowLeftRight className='h-[12px] w-[12px]' />
</button>
</Tooltip.Trigger>
<Tooltip.Content side='top'>
{field.mode === 'basic' ? 'Switch to manual input' : 'Switch to selector'}
</Tooltip.Content>
</Tooltip.Root>
)}
</div>
{field.description && (
<p className='text-[var(--text-muted)] text-xs'>{field.description}</p>
)}
{field.type === 'selector' && field.selectorKey ? (
<ConnectorSelectorField
field={field as ConnectorConfigField & { selectorKey: SelectorKey }}
value={sourceConfig[field.id] || ''}
onChange={(value) => onFieldChange(field.id, value)}
credentialId={credentialId}
sourceConfig={sourceConfig}
configFields={connectorConfig.configFields}
canonicalModes={canonicalModes}
disabled={isSaving}
/>
) : field.type === 'dropdown' && field.options ? (
<Combobox
size='sm'
options={field.options.map((opt) => ({
label: opt.label,
value: opt.id,
}))}
value={sourceConfig[field.id] || undefined}
onChange={(value) => onFieldChange(field.id, value)}
placeholder={field.placeholder || `Select ${field.title.toLowerCase()}`}
/>
) : (
<Input
value={sourceConfig[field.id] || ''}
onChange={(e) => onFieldChange(field.id, e.target.value)}
placeholder={field.placeholder}
/>
)}
</div>
)
})}
<div className='flex flex-col gap-2'>
<Label>Sync Frequency</Label>
@@ -0,0 +1,153 @@
'use client'
import { useCallback, useMemo, useState } from 'react'
import { getDependsOnFields } from '@/blocks/utils'
import type { ConnectorConfig, ConnectorConfigField } from '@/connectors/types'
export interface UseConnectorConfigFieldsOptions {
connectorConfig: ConnectorConfig | null
initialSourceConfig?: Record<string, string>
}
export interface UseConnectorConfigFieldsResult {
sourceConfig: Record<string, string>
setSourceConfig: React.Dispatch<React.SetStateAction<Record<string, string>>>
canonicalModes: Record<string, 'basic' | 'advanced'>
setCanonicalModes: React.Dispatch<React.SetStateAction<Record<string, 'basic' | 'advanced'>>>
canonicalGroups: Map<string, ConnectorConfigField[]>
isFieldVisible: (field: ConnectorConfigField) => boolean
handleFieldChange: (fieldId: string, value: string) => void
toggleCanonicalMode: (canonicalId: string) => void
resolveSourceConfig: () => Record<string, string>
}
/**
* Shared state and helpers for connector configuration fields that support
* canonical pairs (selector + manual input sharing a `canonicalParamId`).
*
* - Tracks current field values and active mode (basic/advanced) per canonical group.
* - Computes the dependency graph including canonical-sibling expansion so that
* changing a dependency clears both siblings of any dependent canonical pair.
* - Returns `resolveSourceConfig` which collapses the per-field map back to a
* canonical-keyed object ready to submit.
*/
export function useConnectorConfigFields({
connectorConfig,
initialSourceConfig,
}: UseConnectorConfigFieldsOptions): UseConnectorConfigFieldsResult {
const [sourceConfig, setSourceConfig] = useState<Record<string, string>>(
() => initialSourceConfig ?? {}
)
const [canonicalModes, setCanonicalModes] = useState<Record<string, 'basic' | 'advanced'>>({})
const canonicalGroups = useMemo(() => {
const groups = new Map<string, ConnectorConfigField[]>()
if (!connectorConfig) return groups
for (const field of connectorConfig.configFields) {
if (!field.canonicalParamId) continue
const existing = groups.get(field.canonicalParamId)
if (existing) existing.push(field)
else groups.set(field.canonicalParamId, [field])
}
return groups
}, [connectorConfig])
const dependentFieldIds = useMemo(() => {
const result = new Map<string, string[]>()
if (!connectorConfig) return result
const map = new Map<string, Set<string>>()
for (const field of connectorConfig.configFields) {
const deps = getDependsOnFields(field.dependsOn)
for (const dep of deps) {
const existing = map.get(dep) ?? new Set<string>()
existing.add(field.id)
if (field.canonicalParamId) {
for (const sibling of canonicalGroups.get(field.canonicalParamId) ?? []) {
existing.add(sibling.id)
}
}
map.set(dep, existing)
}
}
for (const group of canonicalGroups.values()) {
const allDependents = new Set<string>()
for (const field of group) {
for (const dep of map.get(field.id) ?? []) {
allDependents.add(dep)
const depField = connectorConfig.configFields.find((f) => f.id === dep)
if (depField?.canonicalParamId) {
for (const sibling of canonicalGroups.get(depField.canonicalParamId) ?? []) {
allDependents.add(sibling.id)
}
}
}
}
if (allDependents.size > 0) {
for (const field of group) map.set(field.id, new Set(allDependents))
}
}
for (const [key, value] of map) result.set(key, [...value])
return result
}, [connectorConfig, canonicalGroups])
const isFieldVisible = useCallback(
(field: ConnectorConfigField): boolean => {
if (!field.canonicalParamId || !field.mode) return true
const activeMode = canonicalModes[field.canonicalParamId] ?? 'basic'
return field.mode === activeMode
},
[canonicalModes]
)
const handleFieldChange = (fieldId: string, value: string) => {
setSourceConfig((prev) => {
const next = { ...prev, [fieldId]: value }
const toClear = dependentFieldIds.get(fieldId)
if (toClear) {
for (const depId of toClear) next[depId] = ''
}
return next
})
}
const toggleCanonicalMode = (canonicalId: string) => {
setCanonicalModes((prev) => ({
...prev,
[canonicalId]: prev[canonicalId] === 'advanced' ? 'basic' : 'advanced',
}))
}
const resolveSourceConfig = useCallback((): Record<string, string> => {
const resolved: Record<string, string> = {}
const processed = new Set<string>()
if (!connectorConfig) return resolved
for (const field of connectorConfig.configFields) {
if (field.canonicalParamId) {
if (processed.has(field.canonicalParamId)) continue
processed.add(field.canonicalParamId)
const group = canonicalGroups.get(field.canonicalParamId)
if (!group) continue
const activeMode = canonicalModes[field.canonicalParamId] ?? 'basic'
const activeField = group.find((f) => f.mode === activeMode) ?? group[0]
resolved[field.canonicalParamId] = sourceConfig[activeField.id] ?? ''
} else {
resolved[field.id] = sourceConfig[field.id] ?? ''
}
}
return resolved
}, [connectorConfig, canonicalGroups, canonicalModes, sourceConfig])
return {
sourceConfig,
setSourceConfig,
canonicalModes,
setCanonicalModes,
canonicalGroups,
isFieldVisible,
handleFieldChange,
toggleCanonicalMode,
resolveSourceConfig,
}
}