Skip to content

Commit 2732ab7

Browse files
fix(forking): keep dependent overrides editable (#6776)
* fix(forking): keep dependent overrides editable * fix(forking): expand configured edit cards
1 parent 75718ab commit 2732ab7

6 files changed

Lines changed: 311 additions & 46 deletions

File tree

apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
effectiveCopyDependentValue,
1010
effectiveDependentValue,
1111
getActionableDependentFields,
12+
getDisplayedDependentFields,
1213
isDependentConfigurationActionable,
1314
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
1415

@@ -167,6 +168,46 @@ describe('applyDependentRepick', () => {
167168
[dependentKey(unrelated)]: 'still-keep-me',
168169
})
169170
})
171+
172+
it('does not clear a descendant belonging to another nested tool instance', () => {
173+
const projectOne = field({
174+
subBlockKey: 'tools[0].projectId',
175+
dependencyScope: 'tools[0]',
176+
providesContextKey: 'projectId',
177+
})
178+
const issueOne = field({
179+
subBlockKey: 'tools[0].issueKey',
180+
dependencyScope: 'tools[0]',
181+
consumesContextKeys: ['projectId'],
182+
})
183+
const projectTwo = field({
184+
subBlockKey: 'tools[1].projectId',
185+
dependencyScope: 'tools[1]',
186+
providesContextKey: 'projectId',
187+
})
188+
const issueTwo = field({
189+
subBlockKey: 'tools[1].issueKey',
190+
dependencyScope: 'tools[1]',
191+
consumesContextKeys: ['projectId'],
192+
})
193+
const previous = {
194+
[dependentKey(issueOne)]: 'P1-1',
195+
[dependentKey(issueTwo)]: 'P2-1',
196+
}
197+
198+
expect(
199+
applyDependentRepick(
200+
previous,
201+
projectOne,
202+
[projectOne, issueOne, projectTwo, issueTwo],
203+
'P1-NEW'
204+
)
205+
).toEqual({
206+
[dependentKey(projectOne)]: 'P1-NEW',
207+
[dependentKey(issueOne)]: '',
208+
[dependentKey(issueTwo)]: 'P2-1',
209+
})
210+
})
170211
})
171212

172213
describe('isDependentConfigurationActionable', () => {
@@ -341,4 +382,63 @@ describe('getActionableDependentFields', () => {
341382
).map((dependent) => dependent.subBlockKey)
342383
).toEqual(['siteId', 'driveId', 'spreadsheetId'])
343384
})
385+
386+
it('finds a required child provider only within the same nested tool instance', () => {
387+
const projectOne = field({
388+
subBlockKey: 'tools[0].projectId',
389+
dependencyScope: 'tools[0]',
390+
providesContextKey: 'projectId',
391+
})
392+
const projectTwo = field({
393+
subBlockKey: 'tools[1].projectId',
394+
dependencyScope: 'tools[1]',
395+
providesContextKey: 'projectId',
396+
})
397+
const issueOne = field({
398+
subBlockKey: 'tools[0].issueKey',
399+
dependencyScope: 'tools[0]',
400+
currentValue: '',
401+
required: true,
402+
consumesContextKeys: ['projectId'],
403+
})
404+
405+
expect(
406+
getActionableDependentFields(
407+
[projectOne, projectTwo, issueOne],
408+
{},
409+
unchangedMappedParent
410+
).map((dependent) => dependent.subBlockKey)
411+
).toEqual(['tools[0].projectId', 'tools[0].issueKey'])
412+
})
413+
})
414+
415+
describe('getDisplayedDependentFields', () => {
416+
const unchangedMappedParent = {
417+
parentResolved: true,
418+
parentChanged: false,
419+
copying: false,
420+
}
421+
422+
it('reveals configured and optional fields only after the explicit edit action', () => {
423+
const configuredRequired = field({ subBlockKey: 'projectId', required: true })
424+
const optional = field({ subBlockKey: 'issueKey', currentValue: '', required: false })
425+
426+
expect(
427+
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, false)
428+
).toEqual([])
429+
expect(
430+
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, true)
431+
).toEqual([configuredRequired, optional])
432+
})
433+
434+
it('never shows selectors before their parent mapping is resolved', () => {
435+
expect(
436+
getDisplayedDependentFields(
437+
[field()],
438+
{},
439+
{ ...unchangedMappedParent, parentResolved: false },
440+
true
441+
)
442+
).toEqual([])
443+
})
344444
})

apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ export function dependentKey(dependent: ForkDependentReconfig): string {
55
return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}`
66
}
77

8+
function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentReconfig): boolean {
9+
return left.dependencyScope === right.dependencyScope
10+
}
11+
812
/**
913
* Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string
1014
* overrides are intentional: an absent override means "fall back to the stored value", while a
@@ -28,7 +32,13 @@ export function applyDependentRepick(
2832

2933
for (const field of blockFields) {
3034
const fieldKey = dependentKey(field)
31-
if (visitedFields.has(fieldKey) || !field.consumesContextKeys.includes(contextKey)) continue
35+
if (
36+
!sameDependencyScope(changedField, field) ||
37+
visitedFields.has(fieldKey) ||
38+
!field.consumesContextKeys.includes(contextKey)
39+
) {
40+
continue
41+
}
3242

3343
visitedFields.add(fieldKey)
3444
nextState[fieldKey] = ''
@@ -106,17 +116,23 @@ export function getActionableDependentFields(
106116
const actionable = new Set(
107117
fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state))
108118
)
109-
const providersByContextKey = new Map<string, ForkDependentReconfig>()
119+
const providersByScope = new Map<string | undefined, Map<string, ForkDependentReconfig>>()
110120
for (const field of fields) {
111-
if (field.providesContextKey) providersByContextKey.set(field.providesContextKey, field)
121+
if (!field.providesContextKey) continue
122+
let providers = providersByScope.get(field.dependencyScope)
123+
if (!providers) {
124+
providers = new Map()
125+
providersByScope.set(field.dependencyScope, providers)
126+
}
127+
providers.set(field.providesContextKey, field)
112128
}
113129

114130
const pending = Array.from(actionable)
115131
for (let index = 0; index < pending.length; index += 1) {
116132
const field = pending[index]
117133
if (!field) continue
118134
for (const contextKey of field.consumesContextKeys) {
119-
const provider = providersByContextKey.get(contextKey)
135+
const provider = providersByScope.get(field.dependencyScope)?.get(contextKey)
120136
if (!provider || actionable.has(provider)) continue
121137
actionable.add(provider)
122138
pending.push(provider)
@@ -125,3 +141,18 @@ export function getActionableDependentFields(
125141

126142
return fields.filter((field) => actionable.has(field))
127143
}
144+
145+
/**
146+
* Fields rendered in the mapping UI. Required missing fields remain visible by default; an
147+
* explicit edit action reveals every active selector under a resolved parent without changing
148+
* which fields gate Sync.
149+
*/
150+
export function getDisplayedDependentFields(
151+
fields: ForkDependentReconfig[],
152+
reconfig: Record<string, string>,
153+
state: DependentConfigurationState,
154+
showConfigured: boolean
155+
): ForkDependentReconfig[] {
156+
if (!state.parentResolved) return []
157+
return showConfigured ? fields : getActionableDependentFields(fields, reconfig, state)
158+
}

apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx

Lines changed: 66 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ import {
3939
dependentKey,
4040
effectiveCopyDependentValue,
4141
effectiveDependentValue,
42-
getActionableDependentFields,
42+
getDisplayedDependentFields,
43+
isDependentConfigurationActionable,
4344
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
4445
import type {
4546
ForkKindSummary,
@@ -109,7 +110,8 @@ function groupDependentsByWorkflow(
109110
workflows: ForkResourceUsage['workflows'],
110111
dependents: ForkDependentReconfig[],
111112
reconfig: Record<string, string>,
112-
state: DependentConfigurationState
113+
state: DependentConfigurationState,
114+
showConfigured: boolean
113115
): WorkflowDependents[] {
114116
const byWorkflow = new Map<string, ForkDependentReconfig[]>()
115117
for (const dependent of dependents) {
@@ -138,7 +140,12 @@ function groupDependentsByWorkflow(
138140
blocks: Array.from(byBlock.values())
139141
.map((block) => ({
140142
...block,
141-
configurableFields: getActionableDependentFields(block.fields, reconfig, state),
143+
configurableFields: getDisplayedDependentFields(
144+
block.fields,
145+
reconfig,
146+
state,
147+
showConfigured
148+
),
142149
}))
143150
.filter((block) => block.configurableFields.length > 0)
144151
.sort((a, b) => a.blockName.localeCompare(b.blockName)),
@@ -149,11 +156,13 @@ function groupDependentsByWorkflow(
149156
/** Chain state for one block: the SelectorContext values its parent fields provide. */
150157
function blockChainState(
151158
block: DependentBlock,
159+
activeField: ForkDependentReconfig,
152160
effectiveValue: (field: ForkDependentReconfig) => string
153161
) {
154162
const providedValues: Record<string, string> = {}
155163
const providedContextKeys = new Set<string>()
156164
for (const field of block.fields) {
165+
if (field.dependencyScope !== activeField.dependencyScope) continue
157166
if (field.providesContextKey) {
158167
providedContextKeys.add(field.providesContextKey)
159168
const value = effectiveValue(field)
@@ -199,7 +208,7 @@ function DependentSelector({
199208
copying
200209
? effectiveCopyDependentValue(f, reconfig)
201210
: effectiveDependentValue(f, reconfig, parentChanged)
202-
const { providedValues, providedContextKeys } = blockChainState(block, effectiveValue)
211+
const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue)
203212
// Disabled until every in-block parent it depends on has a value, so a child never queries
204213
// a stale upstream value.
205214
const ready = field.consumesContextKeys.every(
@@ -230,6 +239,7 @@ function DependentSelector({
230239

231240
interface DependentWorkflowCardProps {
232241
workflow: WorkflowDependents
242+
initiallyExpanded: boolean
233243
target: string
234244
parentChanged: boolean
235245
/** True when the parent is resolved by COPY - the selectors browse the SOURCE parent. */
@@ -244,10 +254,12 @@ interface DependentWorkflowCardProps {
244254
* One workflow's dependent fields as a collapsible card (the same `CollapsibleCard` the table
245255
* workflow sidebar's input mapping and the enrichment config use): the header names the
246256
* workflow; the body groups fields under block → optional tool → plain field label.
247-
* Cards holding a required field start expanded - a required field is what gates Sync.
257+
* Cards holding a required field start expanded because that field gates Sync. Cards first
258+
* revealed by explicit edit mode also start expanded so the edit action exposes its controls.
248259
*/
249260
function DependentWorkflowCard({
250261
workflow,
262+
initiallyExpanded,
251263
target,
252264
parentChanged,
253265
copying,
@@ -257,7 +269,9 @@ function DependentWorkflowCard({
257269
setReconfig,
258270
}: DependentWorkflowCardProps) {
259271
const [collapsed, setCollapsed] = useState(
260-
() => !workflow.blocks.some((block) => block.configurableFields.some((field) => field.required))
272+
() =>
273+
!initiallyExpanded &&
274+
!workflow.blocks.some((block) => block.configurableFields.some((field) => field.required))
261275
)
262276
return (
263277
<CollapsibleCard
@@ -268,14 +282,17 @@ function DependentWorkflowCard({
268282
<div className='flex flex-col gap-3'>
269283
{workflow.blocks.map((block) => {
270284
const topLevel = block.configurableFields.filter((field) => !field.toolName)
271-
const byTool = new Map<string, ForkDependentReconfig[]>()
285+
const byTool = new Map<string, { name: string; fields: ForkDependentReconfig[] }>()
272286
for (const field of block.configurableFields) {
273287
if (!field.toolName) continue
274-
const list = byTool.get(field.toolName)
275-
if (list) list.push(field)
276-
else byTool.set(field.toolName, [field])
288+
const scope = field.dependencyScope ?? field.toolName
289+
const group = byTool.get(scope)
290+
if (group) group.fields.push(field)
291+
else byTool.set(scope, { name: field.toolName, fields: [field] })
277292
}
278-
const toolGroups = Array.from(byTool.entries()).sort(([a], [b]) => a.localeCompare(b))
293+
const toolGroups = Array.from(byTool.entries()).sort(([, a], [, b]) =>
294+
a.name.localeCompare(b.name)
295+
)
279296

280297
return (
281298
<div key={block.targetBlockId} className='flex flex-col gap-2'>
@@ -299,10 +316,10 @@ function DependentWorkflowCard({
299316
/>
300317
</div>
301318
))}
302-
{toolGroups.map(([toolName, fields]) => (
303-
<div key={toolName} className='flex flex-col gap-1.5 pl-2'>
304-
<span className='text-[var(--text-muted)] text-small'>{toolName}</span>
305-
{fields.map((field) => (
319+
{toolGroups.map(([scope, tool]) => (
320+
<div key={scope} className='flex flex-col gap-1.5 pl-2'>
321+
<span className='text-[var(--text-muted)] text-small'>{tool.name}</span>
322+
{tool.fields.map((field) => (
306323
<div key={dependentKey(field)} className='flex flex-col gap-1'>
307324
<Label className='text-[var(--text-muted)] text-caption'>
308325
{field.title}
@@ -346,6 +363,7 @@ interface MappingEntryProps {
346363
* Workflows with nothing to configure are named in a muted note so the usage stays visible.
347364
*/
348365
function MappingEntry({ controller, group, entry }: MappingEntryProps) {
366+
const [showConfigured, setShowConfigured] = useState(false)
349367
const target = controller.targetFor(entry)
350368
const takenOwners = controller.takenOwnersFor(entry, group.items)
351369
const parentChanged = controller.parentChangedFor(entry)
@@ -354,17 +372,34 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
354372

355373
const usages = controller.usagesForEntry(entry)
356374
const dependents = controller.dependentsForEntry(entry)
375+
const parentResolved = target !== '' || copying
357376
const workflows = useMemo(
358377
() =>
359-
groupDependentsByWorkflow(usages, dependents, controller.reconfig, {
360-
parentResolved: target !== '' || copying,
361-
parentChanged,
362-
copying,
363-
}),
364-
[usages, dependents, controller.reconfig, target, parentChanged, copying]
378+
groupDependentsByWorkflow(
379+
usages,
380+
dependents,
381+
controller.reconfig,
382+
{ parentResolved, parentChanged, copying },
383+
showConfigured
384+
),
385+
[
386+
usages,
387+
dependents,
388+
controller.reconfig,
389+
parentResolved,
390+
parentChanged,
391+
copying,
392+
showConfigured,
393+
]
365394
)
366395
const configurable = workflows.filter((workflow) => workflow.blocks.length > 0)
367396
const usedOnly = workflows.filter((workflow) => workflow.blocks.length === 0)
397+
const configurationState = { parentResolved, parentChanged, copying }
398+
const hasHiddenConfigured = dependents.some(
399+
(field) => !isDependentConfigurationActionable(field, controller.reconfig, configurationState)
400+
)
401+
const canEditConfigured =
402+
parentResolved && !parentChanged && !copying && (showConfigured || hasHiddenConfigured)
368403

369404
return (
370405
<div className='flex flex-col gap-2'>
@@ -422,10 +457,18 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
422457
</p>
423458
) : null}
424459
</div>
460+
{canEditConfigured ? (
461+
<div className='flex justify-end'>
462+
<Chip active={showConfigured} onClick={() => setShowConfigured((value) => !value)}>
463+
{showConfigured ? 'Done editing' : 'Edit configuration'}
464+
</Chip>
465+
</div>
466+
) : null}
425467
{configurable.map((workflow) => (
426468
<DependentWorkflowCard
427469
key={workflow.workflowId}
428470
workflow={workflow}
471+
initiallyExpanded={showConfigured}
429472
target={target}
430473
parentChanged={parentChanged}
431474
copying={copying}
@@ -437,8 +480,8 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
437480
))}
438481
{usedOnly.length > 0 ? (
439482
<p className='text-[var(--text-tertiary)] text-caption'>
440-
Also used in {usedOnly.map((workflow) => workflow.workflowName).join(', ')}nothing to
441-
configure there.
483+
Also used in {usedOnly.map((workflow) => workflow.workflowName).join(', ')}no changes
484+
required.
442485
</p>
443486
) : null}
444487
</div>

0 commit comments

Comments
 (0)