Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
effectiveCopyDependentValue,
effectiveDependentValue,
getActionableDependentFields,
getDisplayedDependentFields,
isDependentConfigurationActionable,
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'

Expand Down Expand Up @@ -167,6 +168,46 @@ describe('applyDependentRepick', () => {
[dependentKey(unrelated)]: 'still-keep-me',
})
})

it('does not clear a descendant belonging to another nested tool instance', () => {
const projectOne = field({
subBlockKey: 'tools[0].projectId',
dependencyScope: 'tools[0]',
providesContextKey: 'projectId',
})
const issueOne = field({
subBlockKey: 'tools[0].issueKey',
dependencyScope: 'tools[0]',
consumesContextKeys: ['projectId'],
})
const projectTwo = field({
subBlockKey: 'tools[1].projectId',
dependencyScope: 'tools[1]',
providesContextKey: 'projectId',
})
const issueTwo = field({
subBlockKey: 'tools[1].issueKey',
dependencyScope: 'tools[1]',
consumesContextKeys: ['projectId'],
})
const previous = {
[dependentKey(issueOne)]: 'P1-1',
[dependentKey(issueTwo)]: 'P2-1',
}

expect(
applyDependentRepick(
previous,
projectOne,
[projectOne, issueOne, projectTwo, issueTwo],
'P1-NEW'
)
).toEqual({
[dependentKey(projectOne)]: 'P1-NEW',
[dependentKey(issueOne)]: '',
[dependentKey(issueTwo)]: 'P2-1',
})
})
})

describe('isDependentConfigurationActionable', () => {
Expand Down Expand Up @@ -341,4 +382,63 @@ describe('getActionableDependentFields', () => {
).map((dependent) => dependent.subBlockKey)
).toEqual(['siteId', 'driveId', 'spreadsheetId'])
})

it('finds a required child provider only within the same nested tool instance', () => {
const projectOne = field({
subBlockKey: 'tools[0].projectId',
dependencyScope: 'tools[0]',
providesContextKey: 'projectId',
})
const projectTwo = field({
subBlockKey: 'tools[1].projectId',
dependencyScope: 'tools[1]',
providesContextKey: 'projectId',
})
const issueOne = field({
subBlockKey: 'tools[0].issueKey',
dependencyScope: 'tools[0]',
currentValue: '',
required: true,
consumesContextKeys: ['projectId'],
})

expect(
getActionableDependentFields(
[projectOne, projectTwo, issueOne],
{},
unchangedMappedParent
).map((dependent) => dependent.subBlockKey)
).toEqual(['tools[0].projectId', 'tools[0].issueKey'])
})
})

describe('getDisplayedDependentFields', () => {
const unchangedMappedParent = {
parentResolved: true,
parentChanged: false,
copying: false,
}

it('reveals configured and optional fields only after the explicit edit action', () => {
const configuredRequired = field({ subBlockKey: 'projectId', required: true })
const optional = field({ subBlockKey: 'issueKey', currentValue: '', required: false })

expect(
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, false)
).toEqual([])
expect(
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, true)
).toEqual([configuredRequired, optional])
})

it('never shows selectors before their parent mapping is resolved', () => {
expect(
getDisplayedDependentFields(
[field()],
{},
{ ...unchangedMappedParent, parentResolved: false },
true
)
).toEqual([])
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export function dependentKey(dependent: ForkDependentReconfig): string {
return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}`
}

function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentReconfig): boolean {
return left.dependencyScope === right.dependencyScope
}

/**
* Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string
* overrides are intentional: an absent override means "fall back to the stored value", while a
Expand All @@ -28,7 +32,13 @@ export function applyDependentRepick(

for (const field of blockFields) {
const fieldKey = dependentKey(field)
if (visitedFields.has(fieldKey) || !field.consumesContextKeys.includes(contextKey)) continue
if (
!sameDependencyScope(changedField, field) ||
visitedFields.has(fieldKey) ||
!field.consumesContextKeys.includes(contextKey)
) {
continue
}

visitedFields.add(fieldKey)
nextState[fieldKey] = ''
Expand Down Expand Up @@ -106,17 +116,23 @@ export function getActionableDependentFields(
const actionable = new Set(
fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state))
)
const providersByContextKey = new Map<string, ForkDependentReconfig>()
const providersByScope = new Map<string | undefined, Map<string, ForkDependentReconfig>>()
for (const field of fields) {
if (field.providesContextKey) providersByContextKey.set(field.providesContextKey, field)
if (!field.providesContextKey) continue
let providers = providersByScope.get(field.dependencyScope)
if (!providers) {
providers = new Map()
providersByScope.set(field.dependencyScope, providers)
}
providers.set(field.providesContextKey, field)
}

const pending = Array.from(actionable)
for (let index = 0; index < pending.length; index += 1) {
const field = pending[index]
if (!field) continue
for (const contextKey of field.consumesContextKeys) {
const provider = providersByContextKey.get(contextKey)
const provider = providersByScope.get(field.dependencyScope)?.get(contextKey)
if (!provider || actionable.has(provider)) continue
actionable.add(provider)
pending.push(provider)
Expand All @@ -125,3 +141,18 @@ export function getActionableDependentFields(

return fields.filter((field) => actionable.has(field))
}

/**
* Fields rendered in the mapping UI. Required missing fields remain visible by default; an
* explicit edit action reveals every active selector under a resolved parent without changing
* which fields gate Sync.
*/
export function getDisplayedDependentFields(
fields: ForkDependentReconfig[],
reconfig: Record<string, string>,
state: DependentConfigurationState,
showConfigured: boolean
): ForkDependentReconfig[] {
if (!state.parentResolved) return []
return showConfigured ? fields : getActionableDependentFields(fields, reconfig, state)
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ import {
dependentKey,
effectiveCopyDependentValue,
effectiveDependentValue,
getActionableDependentFields,
getDisplayedDependentFields,
isDependentConfigurationActionable,
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
import type {
ForkKindSummary,
Expand Down Expand Up @@ -109,7 +110,8 @@ function groupDependentsByWorkflow(
workflows: ForkResourceUsage['workflows'],
dependents: ForkDependentReconfig[],
reconfig: Record<string, string>,
state: DependentConfigurationState
state: DependentConfigurationState,
showConfigured: boolean
): WorkflowDependents[] {
const byWorkflow = new Map<string, ForkDependentReconfig[]>()
for (const dependent of dependents) {
Expand Down Expand Up @@ -138,7 +140,12 @@ function groupDependentsByWorkflow(
blocks: Array.from(byBlock.values())
.map((block) => ({
...block,
configurableFields: getActionableDependentFields(block.fields, reconfig, state),
configurableFields: getDisplayedDependentFields(
block.fields,
reconfig,
state,
showConfigured
),
}))
.filter((block) => block.configurableFields.length > 0)
.sort((a, b) => a.blockName.localeCompare(b.blockName)),
Expand All @@ -149,11 +156,13 @@ function groupDependentsByWorkflow(
/** Chain state for one block: the SelectorContext values its parent fields provide. */
function blockChainState(
block: DependentBlock,
activeField: ForkDependentReconfig,
effectiveValue: (field: ForkDependentReconfig) => string
) {
const providedValues: Record<string, string> = {}
const providedContextKeys = new Set<string>()
for (const field of block.fields) {
if (field.dependencyScope !== activeField.dependencyScope) continue
if (field.providesContextKey) {
providedContextKeys.add(field.providesContextKey)
const value = effectiveValue(field)
Expand Down Expand Up @@ -199,7 +208,7 @@ function DependentSelector({
copying
? effectiveCopyDependentValue(f, reconfig)
: effectiveDependentValue(f, reconfig, parentChanged)
const { providedValues, providedContextKeys } = blockChainState(block, effectiveValue)
const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue)
// Disabled until every in-block parent it depends on has a value, so a child never queries
// a stale upstream value.
const ready = field.consumesContextKeys.every(
Expand Down Expand Up @@ -230,6 +239,7 @@ function DependentSelector({

interface DependentWorkflowCardProps {
workflow: WorkflowDependents
initiallyExpanded: boolean
target: string
parentChanged: boolean
/** True when the parent is resolved by COPY - the selectors browse the SOURCE parent. */
Expand All @@ -244,10 +254,12 @@ interface DependentWorkflowCardProps {
* One workflow's dependent fields as a collapsible card (the same `CollapsibleCard` the table
* workflow sidebar's input mapping and the enrichment config use): the header names the
* workflow; the body groups fields under block → optional tool → plain field label.
* Cards holding a required field start expanded - a required field is what gates Sync.
* Cards holding a required field start expanded because that field gates Sync. Cards first
* revealed by explicit edit mode also start expanded so the edit action exposes its controls.
*/
function DependentWorkflowCard({
workflow,
initiallyExpanded,
target,
parentChanged,
copying,
Expand All @@ -257,7 +269,9 @@ function DependentWorkflowCard({
setReconfig,
}: DependentWorkflowCardProps) {
const [collapsed, setCollapsed] = useState(
() => !workflow.blocks.some((block) => block.configurableFields.some((field) => field.required))
() =>
!initiallyExpanded &&
!workflow.blocks.some((block) => block.configurableFields.some((field) => field.required))
)
return (
<CollapsibleCard
Expand All @@ -268,14 +282,17 @@ function DependentWorkflowCard({
<div className='flex flex-col gap-3'>
{workflow.blocks.map((block) => {
const topLevel = block.configurableFields.filter((field) => !field.toolName)
const byTool = new Map<string, ForkDependentReconfig[]>()
Comment thread
icecrasher321 marked this conversation as resolved.
const byTool = new Map<string, { name: string; fields: ForkDependentReconfig[] }>()
for (const field of block.configurableFields) {
if (!field.toolName) continue
const list = byTool.get(field.toolName)
if (list) list.push(field)
else byTool.set(field.toolName, [field])
const scope = field.dependencyScope ?? field.toolName
const group = byTool.get(scope)
if (group) group.fields.push(field)
else byTool.set(scope, { name: field.toolName, fields: [field] })
}
const toolGroups = Array.from(byTool.entries()).sort(([a], [b]) => a.localeCompare(b))
const toolGroups = Array.from(byTool.entries()).sort(([, a], [, b]) =>
a.name.localeCompare(b.name)
)

return (
<div key={block.targetBlockId} className='flex flex-col gap-2'>
Expand All @@ -299,10 +316,10 @@ function DependentWorkflowCard({
/>
</div>
))}
{toolGroups.map(([toolName, fields]) => (
<div key={toolName} className='flex flex-col gap-1.5 pl-2'>
<span className='text-[var(--text-muted)] text-small'>{toolName}</span>
{fields.map((field) => (
{toolGroups.map(([scope, tool]) => (
<div key={scope} className='flex flex-col gap-1.5 pl-2'>
<span className='text-[var(--text-muted)] text-small'>{tool.name}</span>
{tool.fields.map((field) => (
<div key={dependentKey(field)} className='flex flex-col gap-1'>
<Label className='text-[var(--text-muted)] text-caption'>
{field.title}
Expand Down Expand Up @@ -346,6 +363,7 @@ interface MappingEntryProps {
* Workflows with nothing to configure are named in a muted note so the usage stays visible.
*/
function MappingEntry({ controller, group, entry }: MappingEntryProps) {
const [showConfigured, setShowConfigured] = useState(false)
const target = controller.targetFor(entry)
const takenOwners = controller.takenOwnersFor(entry, group.items)
const parentChanged = controller.parentChangedFor(entry)
Expand All @@ -354,17 +372,34 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {

const usages = controller.usagesForEntry(entry)
const dependents = controller.dependentsForEntry(entry)
const parentResolved = target !== '' || copying
const workflows = useMemo(
() =>
groupDependentsByWorkflow(usages, dependents, controller.reconfig, {
parentResolved: target !== '' || copying,
parentChanged,
copying,
}),
[usages, dependents, controller.reconfig, target, parentChanged, copying]
groupDependentsByWorkflow(
usages,
dependents,
controller.reconfig,
{ parentResolved, parentChanged, copying },
showConfigured
),
[
usages,
dependents,
controller.reconfig,
parentResolved,
parentChanged,
copying,
showConfigured,
]
)
const configurable = workflows.filter((workflow) => workflow.blocks.length > 0)
const usedOnly = workflows.filter((workflow) => workflow.blocks.length === 0)
const configurationState = { parentResolved, parentChanged, copying }
const hasHiddenConfigured = dependents.some(
(field) => !isDependentConfigurationActionable(field, controller.reconfig, configurationState)
)
const canEditConfigured =
parentResolved && !parentChanged && !copying && (showConfigured || hasHiddenConfigured)

return (
<div className='flex flex-col gap-2'>
Expand Down Expand Up @@ -422,10 +457,18 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
</p>
) : null}
</div>
{canEditConfigured ? (
<div className='flex justify-end'>
<Chip active={showConfigured} onClick={() => setShowConfigured((value) => !value)}>
{showConfigured ? 'Done editing' : 'Edit configuration'}
</Chip>
Comment thread
icecrasher321 marked this conversation as resolved.
</div>
) : null}
{configurable.map((workflow) => (
<DependentWorkflowCard
key={workflow.workflowId}
workflow={workflow}
initiallyExpanded={showConfigured}
target={target}
parentChanged={parentChanged}
copying={copying}
Expand All @@ -437,8 +480,8 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
))}
{usedOnly.length > 0 ? (
<p className='text-[var(--text-tertiary)] text-caption'>
Also used in {usedOnly.map((workflow) => workflow.workflowName).join(', ')} — nothing to
configure there.
Also used in {usedOnly.map((workflow) => workflow.workflowName).join(', ')} — no changes
required.
</p>
) : null}
</div>
Expand Down
Loading
Loading