Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(20321): Refactoring the policy loading #385

Merged
merged 19 commits into from
Apr 29, 2024

Conversation

vanch3d
Copy link
Contributor

@vanch3d vanch3d commented Apr 23, 2024

See https://hivemq.kanbanize.com/ctrl_board/57/cards/20321/details/

The PR refactors how the policy payload is transformed into a designer graph.

The previous process was creating the graph elements (nodes and edges) from the payload and inserting them iteratively into the Design store. This created a re-render race condition, with parent nodes not yet inserted into the graph when their connecting edges were added. The error message at the first loading was the visual result of the faulty operation.

The new process iteratively creates ALL the graph elements BEFORE inserting them in two distinct operations: nodes first then edges.

The PR also fixes a bug with the behaviour policy, in which pipelines were connected to every transition created, regardless of the real dependency.

Before

screenshot-localhost_3000-2024 04 23-13_03_01

screenshot-localhost_3000-2024 04 23-13_03_27

After

screenshot-localhost_3000-2024 04 23-13_02_26

@vanch3d vanch3d requested review from h2xd and sfrehse April 23, 2024 12:14
@vanch3d vanch3d self-assigned this Apr 23, 2024
@cla-bot cla-bot bot added the cla-signed label Apr 23, 2024
@vanch3d vanch3d marked this pull request as ready for review April 23, 2024 12:34
@vanch3d vanch3d force-pushed the fix/20321/refactor-loading branch 2 times, most recently from 8cec8c9 to cb7173c Compare April 25, 2024 09:02
@antpaw antpaw self-requested a review April 26, 2024 13:47
Copy link
Contributor

@antpaw antpaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are a couple of as NodeAddChange and as Connection statements. This is an unsafe way of casting to an object. If this happens to variable declaration or return types you can use satisfies NodeAddChange instead. For return types you could also declare the return type of the function and remove the casting

// example
export const loadBehaviorPolicy = (behaviorPolicy: BehaviorPolicy): NodeAddChange => {
  // ... stuff
  return { item: behaviorPolicyNode, type: 'add' }
}

export const loadClientFilter = (behaviorPolicy: BehaviorPolicy, behaviorPolicyNode: Node<BehaviorPolicyData>): (NodeAddChange | Connection)[] => {
  /// ..stuff
  return [
    { item: topicNode, type: 'add' },
    { source: topicNode.id, target: behaviorPolicyNode.id, sourceHandle: null, targetHandle: null },
  ]
}

@vanch3d vanch3d requested a review from antpaw April 29, 2024 10:21
@vanch3d vanch3d force-pushed the fix/20321/refactor-loading branch from cf14897 to 4feba71 Compare April 29, 2024 10:26
)

store.onNodesChange(nodeChanges)
edgeConnects.map((connection) => store.onConnect(connection))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this a map, i dont see anyone collecting it

@vanch3d vanch3d force-pushed the fix/20321/refactor-loading branch from c66d871 to ff209dd Compare April 29, 2024 12:58
@vanch3d vanch3d merged commit a0e8299 into master Apr 29, 2024
9 of 10 checks passed
@vanch3d vanch3d deleted the fix/20321/refactor-loading branch April 29, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants