Skip to content

Commit

Permalink
fix(Canvas): Duplicated edge IDs
Browse files Browse the repository at this point in the history
Currently, edge IDs are not scoped to the owner flow, causing to be
duplicated in case two flows use the same step combination.

The fix is to scope the edge ID as well, so we avoid the duplication.

fix: #1897
  • Loading branch information
lordrip committed Jan 8, 2025
1 parent 91ec9a6 commit 2e07ed0
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ describe('Test source code editor', () => {
it('loads the YAML editor and deletes steps, check with visualization', () => {
cy.uploadFixture('flows/kameletBinding/kafkaSourceSink.yaml');
cy.openDesignPage();
cy.get('[data-id^="json-deserialize-action"]').should('exist');
cy.get('[data-id^="Updated integration|json-deserialize-action"]').should('exist');
cy.openSourceCode();
cy.editorDeleteLine(19, 5);

// CHECK that the code editor contains the new timer source step
cy.openDesignPage();
cy.checkNodeExist('json-deserialize-action', 0);
cy.checkNodeExist('Updated integration|json-deserialize-action', 0);
});

it('User adds step to the YAML', () => {
Expand All @@ -36,7 +36,7 @@ describe('Test source code editor', () => {
cy.editorDeleteLine(12, 6);
cy.openDesignPage();
// CHECK the kafka-sink step was removed
cy.checkNodeExist('kafka-sink', 0);
cy.checkNodeExist('integration|kafka-sink', 0);
});

it('User edits step in the YAML', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('Test for Branching actions from the canvas', () => {

cy.checkNodeExist('activemq', 1);
cy.checkEdgeExists(
'eip-action',
'template.from.steps.1.choice.when.0.steps.1.setHeader',
'template.from.steps.1.choice.when.0.steps.2.to',
);
Expand All @@ -77,6 +78,7 @@ describe('Test for Branching actions from the canvas', () => {

cy.checkNodeExist('activemq', 1);
cy.checkEdgeExists(
'eip-action',
'template.from.steps.1.choice.when.0.steps.0.to',
'template.from.steps.1.choice.when.0.steps.1.to',
);
Expand All @@ -91,6 +93,6 @@ describe('Test for Branching actions from the canvas', () => {
cy.chooseFromCatalog('component', 'activemq');

cy.checkNodeExist('activemq', 1);
cy.checkEdgeExists('template.from.steps.2.to', 'template.from.steps.3.filter');
cy.checkEdgeExists('eip-action', 'template.from.steps.2.to', 'template.from.steps.3.filter');
});
});
2 changes: 1 addition & 1 deletion packages/ui-tests/cypress/support/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ declare global {
selectRemoveGroup(groupName: string, nodeIndex?: number): Chainable<JQuery<Element>>;
performNodeAction(nodeName: string, action: ActionType, nodeIndex?: number): Chainable<JQuery<Element>>;
checkNodeExist(inputName: string, nodesCount: number): Chainable<JQuery<Element>>;
checkEdgeExists(sourceName: string, targetName: string): Chainable<JQuery<Element>>;
checkEdgeExists(scope: string, sourceName: string, targetName: string): Chainable<JQuery<Element>>;
deleteBranch(branchIndex: number): Chainable<JQuery<Element>>;
selectCamelRouteType(type: string, subType?: string): Chainable<JQuery<Element>>;
selectRuntimeVersion(type: string): Chainable<JQuery<Element>>;
Expand Down
4 changes: 2 additions & 2 deletions packages/ui-tests/cypress/support/next-commands/design.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ Cypress.Commands.add('checkNodeExist', (inputName, nodesCount) => {
cy.get(`foreignObject[data-nodelabel="${inputName}"]`).should('have.length', nodesCount);
});

Cypress.Commands.add('checkEdgeExists', (sourceName: string, targetName: string) => {
const idPattern = `${sourceName} >>> ${targetName}`;
Cypress.Commands.add('checkEdgeExists', (scope: string, sourceName: string, targetName: string) => {
const idPattern = `${scope}|${sourceName} >>> ${targetName}`;
// Check if an element with the matching id exists
cy.get('g').should(($elements) => {
// Use Cypress commands to check if any element matches the id pattern
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ exports[`FlowService getFlowDiagram should return nodes and edges for a multiple
[
{
"edgeStyle": "solid",
"id": "node >>> set-header",
"id": "test|node >>> set-header",
"source": "test|node",
"target": "test|set-header",
"type": "edge",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,23 @@ describe('FlowService', () => {
expect(group.children).toEqual(['test|route.from', 'test|route.from.steps.0.placeholder']);
expect(group.group).toBeTruthy();
});

it('should scope nodes & edges IDs', () => {
const routeNode = new CamelRouteVisualEntity({
route: { id: 'route-8888', from: { uri: 'timer:clock', steps: [{ to: { uri: 'log' } }] } },
}).toVizNode();

const { nodes, edges } = FlowService.getFlowDiagram('test', routeNode);

expect(nodes).toHaveLength(3);
expect(nodes[0].id).toEqual('test|route.from');
expect(nodes[1].id).toEqual('test|route.from.steps.0.to');
expect(nodes[2].id).toEqual('test|route');

expect(edges).toHaveLength(1);
expect(edges[0].id).toEqual('test|route.from >>> route.from.steps.0.to');
expect(edges[0].source).toEqual('test|route.from');
expect(edges[0].target).toEqual('test|route.from.steps.0.to');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export class FlowService {
node.parentNode = node.parentNode ? `${scope}|${node.parentNode}` : undefined;
});
this.edges.forEach((edge) => {
edge.id = `${scope}|${edge.id}`;
edge.source = `${scope}|${edge.source}`;
edge.target = `${scope}|${edge.target}`;
});
Expand Down

0 comments on commit 2e07ed0

Please sign in to comment.