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
@@ -0,0 +1,110 @@
// @vitest-environment jsdom
import { describe, it, expect } from 'vitest';
import { extendUpstreamNodeIdsForScopePaste } from '../copypaste';
import type { RootState } from '../../..';
import { createWorkflowEdge, createWorkflowNode } from '../../../utils/graph';
import { WORKFLOW_NODE_TYPES } from '@microsoft/logic-apps-shared';

// Graph shape:
// root
// manual (trigger)
// Init_Variable (source node at root, before the parallel split)
// Init_Parallel (parallel branch A — NOT upstream of branches B/C)
// Condition_outer (parallel branch B; scope with a True subgraph — paste site)
// └── Condition_outer-actions (subgraph paste site inside the condition)
// For_each (parallel branch C; loop with an empty body — paste site)
const buildState = (): RootState => {
const rootGraph = {
id: 'root',
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [
createWorkflowNode('manual'),
createWorkflowNode('Init_Variable'),
createWorkflowNode('Init_Parallel'),
{
id: 'Condition_outer',
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [
createWorkflowNode('Condition_outer-#scope', WORKFLOW_NODE_TYPES.SCOPE_CARD_NODE),
{
id: 'Condition_outer-actions',
type: WORKFLOW_NODE_TYPES.SUBGRAPH_NODE,
children: [createWorkflowNode('Condition_outer-actions-#subgraph', WORKFLOW_NODE_TYPES.SUBGRAPH_CARD_NODE)],
edges: [],
},
],
edges: [createWorkflowEdge('Condition_outer-#scope', 'Condition_outer-actions')],
},
{
id: 'For_each',
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [createWorkflowNode('For_each-#scope', WORKFLOW_NODE_TYPES.SCOPE_CARD_NODE)],
edges: [],
},
],
edges: [
createWorkflowEdge('manual', 'Init_Variable'),
createWorkflowEdge('Init_Variable', 'Init_Parallel'),
createWorkflowEdge('Init_Variable', 'Condition_outer'),
createWorkflowEdge('Init_Variable', 'For_each'),
],
} as any;

return {
workflow: {
graph: rootGraph,
nodesMetadata: {
manual: { graphId: 'root', isRoot: true, isTrigger: true },
Init_Variable: { graphId: 'root' },
Init_Parallel: { graphId: 'root' },
Condition_outer: { graphId: 'root' },
'Condition_outer-actions': { graphId: 'Condition_outer', parentNodeId: 'Condition_outer', subgraphType: 'CONDITIONAL_TRUE' },
For_each: { graphId: 'root' },
},
},
tokens: {
outputTokens: {},
},
} as unknown as RootState;
};

const nodeMap: Record<string, string> = {
manual: 'manual',
Init_Variable: 'Init_Variable',
Init_Parallel: 'Init_Parallel',
Condition_outer: 'Condition_outer',
'Condition_outer-actions': 'Condition_outer-actions',
For_each: 'For_each',
};

describe('extendUpstreamNodeIdsForScopePaste', () => {
it('returns only the caller ids when there is no enclosing graph or parent', () => {
const state = buildState();
const result = extendUpstreamNodeIdsForScopePaste(['Init_Variable'], undefined, undefined, state, nodeMap);
expect(result).toEqual(['Init_Variable']);
});

it('surfaces the ancestor variable when pasting into a Condition subgraph', () => {
// Pasting a scope into the True branch of Condition_outer. graphId is the subgraph node.
const state = buildState();
const result = extendUpstreamNodeIdsForScopePaste([], 'Condition_outer-actions', 'Condition_outer', state, nodeMap);
expect(result).toContain('Init_Variable');
expect(result).not.toContain('Init_Parallel');
});

it('surfaces the ancestor variable when pasting into a For each body even if parentId is undefined', () => {
// Regression: pasting a Condition as the first/only action inside a For each. There is no
// predecessor action, so parentId is undefined. The enclosing graphId (For_each) must still
// surface the upstream Init_Variable while excluding the parallel-branch Init_Parallel.
const state = buildState();
const result = extendUpstreamNodeIdsForScopePaste([], 'For_each', undefined, state, nodeMap);
expect(result).toContain('Init_Variable');
expect(result).not.toContain('Init_Parallel');
});

it('deduplicates ids that already appear in the caller-provided upstream list', () => {
const state = buildState();
const result = extendUpstreamNodeIdsForScopePaste(['Init_Variable'], 'For_each', undefined, state, nodeMap);
expect(result.filter((id) => id === 'Init_Variable')).toHaveLength(1);
});
});
54 changes: 52 additions & 2 deletions libs/designer-v2/src/lib/core/actions/bjsworkflow/copypaste.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import type { ActionDefinition } from '@microsoft/logic-apps-shared/src/utils/sr
import { initializeDynamicDataInNodes, initializeOperationMetadata } from './operationdeserializer';
import type { NodesMetadata } from '../../state/workflow/workflowInterfaces';
import { updateAllUpstreamNodes } from './initialize';
import { getUpstreamNodeIds } from '../../utils/graph';
import type { WorkflowNode } from '../../parsers/models/workflowNode';
import type { NodeTokens } from '../../state/tokens/tokensSlice';
import { addDynamicTokens } from '../../state/tokens/tokensSlice';
import { getConnectionReferenceForNodeId } from '../../state/connection/connectionSelector';
Expand Down Expand Up @@ -315,6 +317,7 @@ export const pasteOperation = createAsyncThunk('pasteOperation', async (payload:
dispatch(setNodeDescription({ nodeId, description: comment }));
}

updateAllUpstreamNodes(getState() as RootState, dispatch);
dispatch(setIsPanelLoading(false));

return nodeId;
Expand Down Expand Up @@ -363,7 +366,13 @@ export const pasteScopeOperation = createAsyncThunk(
true /* shouldAppendAddCase */,
pasteParams
);
actionNodesMetadata[nodeId] = { ...actionNodesMetadata[actionId], isRoot: false, parentNodeId: parentId, graphId };
// parentNodeId must reference a nodesMetadata key (the containing graph/scope) so the
// parent-chain walk in getUpstreamNodeIds can climb to ancestor scopes. `parentId` from
// relationshipIds is an edge-placement node (e.g. a `<scope>-#subgraph` card) and is not a
// valid metadata key, so use `graphId` (the containing graph) instead, matching how
// pasteScopeInWorkflow finalizes this node's metadata.
const scopeParentNodeId = graphId && graphId !== 'root' ? graphId : undefined;
actionNodesMetadata[nodeId] = { ...actionNodesMetadata[actionId], isRoot: false, parentNodeId: scopeParentNodeId, graphId };
if (Object.keys(allConnectionData).length > 0) {
dispatch(initScopeCopiedConnections(replaceIdsOfExistingNodes(allConnectionData, pasteParams.renamedNodes)));
}
Expand Down Expand Up @@ -393,7 +402,8 @@ export const pasteScopeOperation = createAsyncThunk(
for (const id of Object.keys(operations)) {
nodeMap[id] = id;
}
const upstreamOutputTokens = replaceIdsOfExistingNodes(filterRecordByArray(state.tokens.outputTokens, upstreamNodeIds), idReplacements);
const extendedIds = extendUpstreamNodeIdsForScopePaste(upstreamNodeIds, graphId, parentId, state, nodeMap);
const upstreamOutputTokens = replaceIdsOfExistingNodes(filterRecordByArray(state.tokens.outputTokens, extendedIds), idReplacements);
const triggerId = getTriggerNodeId(state.workflow);

await Promise.all([
Expand Down Expand Up @@ -468,6 +478,46 @@ const filterRecordByArray = (record: Record<string, NodeTokens>, upstreamNodeIds
return filteredRecord;
};

/**
* Extends the upstream node id set used to seed a pasted scope's existing output tokens.
*
* When we paste a scope (e.g. a Condition) inside another scope, the `upstreamNodeIds`
* we receive from the caller only covers upstream nodes of the paste-site child/parent
* edge. It does not include the enclosing scope container itself or its ancestor chain,
* so nodes declared alongside (or outside) that container — such as an `InitializeVariable`
* at the workflow root — are absent from the fragment's initial upstream token slice.
*
* We fold in the enclosing scope container (`graphId`) and its full upstream context, plus
* the paste-site predecessor (`parentId`) when present. Relying on `graphId` is essential:
* when a scope is pasted as the first/only action inside a container (e.g. a For each body),
* `parentId` is `undefined` or a non-chaining header node, so extending only via `parentId`
* would miss the ancestor variables. The enclosing `graphId` always resolves to a node whose
* parent chain reaches the workflow root, so its upstream reliably surfaces ancestor tokens.
*/
export const extendUpstreamNodeIdsForScopePaste = (
upstreamNodeIds: string[],
graphId: string | undefined,
parentId: string | undefined,
state: RootState,
operationMap: Record<string, string>
): string[] => {
const extended = new Set(upstreamNodeIds);
const rootGraph = state.workflow.graph as WorkflowNode | null;
if (!rootGraph) {
return Array.from(extended);
}
for (const containerId of [graphId, parentId]) {
if (!containerId || containerId === 'root') {
continue;
}
extended.add(containerId);
for (const id of getUpstreamNodeIds(containerId, rootGraph, state.workflow.nodesMetadata, operationMap)) {
extended.add(id);
}
}
return Array.from(extended);
};

type DuplicateOperationsPayload = {
nodeIds: string[];
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// @vitest-environment jsdom
import { describe, it, expect } from 'vitest';
import { pasteScopeInWorkflow } from '../pasteScopeInWorkflow';
import { createWorkflowNode } from '../../utils/graph';
import { WORKFLOW_NODE_TYPES } from '@microsoft/logic-apps-shared';

// Reproduces the customer bug: a Condition (containing a Set Variable) is pasted into the else
// branch of an existing Condition. The pasted top node arrives with a placeholder parentNodeId
// that points at the subgraph card (`Condition-elseActions-#subgraph`) — an edge-placement node
// that is NOT a nodesMetadata key. The reducer must overwrite that with the containing graph id
// (`Condition-elseActions`) *in state* so the parent-chain walk can climb to the outer scope and
// surface ancestor-scope variables in the Set Variable dropdown.
describe('pasteScopeInWorkflow parentNodeId persistence', () => {
const buildFixture = () => {
const pastedNodeId = 'Condition_2';
const graphId = 'Condition-elseActions';
const subgraphCardId = 'Condition-elseActions-#subgraph';

const elseSubgraph = {
id: graphId,
type: WORKFLOW_NODE_TYPES.SUBGRAPH_NODE,
children: [createWorkflowNode(subgraphCardId, WORKFLOW_NODE_TYPES.SUBGRAPH_CARD_NODE)],
edges: [],
} as any;

const scopeNode = {
id: pastedNodeId,
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [createWorkflowNode(`${pastedNodeId}-#scope`, WORKFLOW_NODE_TYPES.SCOPE_CARD_NODE)],
edges: [],
} as any;

// Metadata handed to the reducer for the pasted subtree. The top node carries the broken
// placeholder parentNodeId (the subgraph card id) that must be corrected in state.
const pasteNodesMetadata = {
[pastedNodeId]: { graphId, parentNodeId: subgraphCardId, isRoot: false },
} as any;

const state = {
operations: {
[pastedNodeId]: {},
},
nodesMetadata: {
Initialize_variables: { graphId: 'root' },
Condition: { graphId: 'root' },
[graphId]: { graphId: 'Condition', parentNodeId: 'Condition', subgraphType: 'CONDITIONAL_FALSE' },
},
newlyAddedOperations: {},
} as any;

const relationshipIds = { graphId, parentId: subgraphCardId, childId: undefined } as any;

return { pastedNodeId, graphId, elseSubgraph, scopeNode, pasteNodesMetadata, state, relationshipIds };
};

it('writes the corrected parentNodeId (the containing graph id) into state', () => {
const { pastedNodeId, graphId, elseSubgraph, scopeNode, pasteNodesMetadata, state, relationshipIds } = buildFixture();

pasteScopeInWorkflow(scopeNode, elseSubgraph, relationshipIds, {}, pasteNodesMetadata, [pastedNodeId], state, false);

expect(state.nodesMetadata[pastedNodeId]).toBeDefined();
expect(state.nodesMetadata[pastedNodeId].graphId).toBe(graphId);
// The placeholder subgraph-card parentNodeId must not survive in state.
expect(state.nodesMetadata[pastedNodeId].parentNodeId).toBe(graphId);
expect(state.nodesMetadata[pastedNodeId].parentNodeId).not.toBe(relationshipIds.parentId);
});

it('leaves parentNodeId undefined when pasting at the workflow root', () => {
const pastedNodeId = 'Condition_2';
const rootGraph = {
id: 'root',
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [],
edges: [],
} as any;
const scopeNode = {
id: pastedNodeId,
type: WORKFLOW_NODE_TYPES.GRAPH_NODE,
children: [createWorkflowNode(`${pastedNodeId}-#scope`, WORKFLOW_NODE_TYPES.SCOPE_CARD_NODE)],
edges: [],
} as any;
const state = {
operations: { [pastedNodeId]: {} },
nodesMetadata: { Initialize_variables: { graphId: 'root' } },
newlyAddedOperations: {},
} as any;
const pasteNodesMetadata = { [pastedNodeId]: { graphId: 'root', parentNodeId: undefined } } as any;
const relationshipIds = { graphId: 'root', parentId: undefined, childId: undefined } as any;

pasteScopeInWorkflow(scopeNode, rootGraph, relationshipIds, {}, pasteNodesMetadata, [pastedNodeId], state, false);

expect(state.nodesMetadata[pastedNodeId].graphId).toBe('root');
expect(state.nodesMetadata[pastedNodeId].parentNodeId).toBeUndefined();
});
});
Loading
Loading