Feat/adding graph to mcp #210

Hubert-Szczepanski-SAP · 2025-07-30T09:15:49Z

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Copilot

Pull Request Overview

This PR adds a graph visualization feature to the control plane view, displaying managed resources and their relationships in an interactive graph format. The graph shows resource dependencies and status information with configurable color coding.

Adds a new Graph component with interactive visualization using ReactFlow
Integrates the graph as a new section in the ControlPlaneView
Implements color-coded visualization options (by provider or provider config)

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/views/ControlPlanes/ControlPlaneView.tsx	Adds graph section and reformats import statements
src/components/Yaml/YamlViewButton.tsx	Reformats import statement for consistency
src/components/Graphs/*.tsx	New graph components including main Graph, CustomNode, StatusIcon, and Legend
src/components/Graphs/*.ts	Graph utility functions and type definitions
src/components/Graphs/*.css	Styling for graph components
public/locales/en.json	Adds translation for graph title
package.json	Adds dependencies for ReactFlow and Dagre graph layout

src/components/Graphs/types.ts

src/components/Graphs/graphUtils.ts

src/components/Graphs/Graph.tsx

src/components/Graphs/CustomNode.module.css

src/components/Graphs/graphUtils.ts

We want to get them manually, because there are many references, that are not related to resources references and there are a lot of issues with that. If there will be any not working reference, we will need to fix it my providing correct path or name (example there is orgRef, and not organizationalRef - kind + Ref name.) (different path in cloudManagementRef - not in spec.ForProviders, but in spec.

andreaskienle

UI looks very nice 💅, and great job piecing together all the connections between the nodes! 👍🏻

I have a few suggestions:

Missing “translation” of strings (i.e. missing use of t()). I've marked some examples, not all
Instead of inline styles (style={{}}) we could use CSS modules for better clarity and consistency. Here too I highlighted a few spots.
I noticed there’s a newer version of react-flow (v12) available. It wasn’t immediately obvious because they changed the package name. Since we’re starting fresh, it might make sense to use the latest version. There is a migration guide from v11 to v12.
I haven’t looked at the business logic to transform the resources and providers into a graph in close detail yet. One suggestion might be to move logic out of the <Graph> component into a useGraph() hook and then further out into a pure function. This might make it easier to understand and maintain. Could also add unit tests then which test the transformation into UI-agnostic nodes and edges given a set of managedResources and providerConfigsList (see comments below in the coding).

You’ve been deep in this topic for a while now. If you feel like bouncing ideas around or doing a bit of pair programming, just let me know, I’m happy to lend a second pair of eyes 🙂

BTW: I added a follow-up task to the epic to support proper theming (light and dark mode).

src/components/Graphs/CustomNode.tsx

src/components/Graphs/Graph.tsx

src/components/Graphs/types.ts

src/components/Graphs/CustomNode.tsx

src/components/Graphs/Legend.module.css

…p-project/ui-frontend into feat/adding-graphs-to-mcp

andreaskienle

Hey there! Thanks for the changes, it’s better now. I noticed a few things though (see coding for details):

Missing error handling in useGraph, it used to be there in the previous revision
There's a warning in the console because of memoization issues (see https://reactflow.dev/error#002). Could we add that?
Types: Instead of creating new type definitions for provider config and managed resources (in the new types.ts file), I think we’d better refine our existing types to avoid duplication of types. If we also move the NodeData type definitions e.g. to the useGraph file, we might be able to remove types.ts entirely which I wouldn’t mind.
If you have time and agree: To make the code more maintainable, we could separate the “node data” that’s used internally (to construct the graph) from the data structure that’s eventually passed to the custom nodes and shown on the UI. I think it makes sense to define a type, e.g. NodeDataUi or similar, which only contains the relevant fields, e.g. without providerConfigName, providerType etc. (which are only used internally for calculating the graph).

Let me know what you think about these suggestions! Happy to discuss alternatives.

src/components/Graphs/useGraph.ts

src/components/Graphs/Graph.tsx

andreaskienle · 2025-08-05T15:04:01Z

src/components/Graphs/Graph.tsx

+    setYamlDialogOpen(true);
+  };
+
+  const nodeTypes = {


This needs to be memoized, otherwise there is a warning on the console (documentation).

src/components/Graphs/Graph.tsx

andreaskienle · 2025-08-05T15:28:13Z

src/components/Graphs/types.ts

@@ -0,0 +1,64 @@
+export interface Condition {


Merge these types with the types already defined in listManagedResoueces.ts and shared/types.ts. They should be consistent across our app. Feel free to break them up into smaller subtypes as you did here.

andreaskienle · 2025-08-05T15:32:42Z

src/components/Graphs/types.ts

+  items?: ProviderConfigItem[];
+}
+
+export interface NodeData {


This type contains internal logic to build the graph as well as logic which is only used on the UI.

Maybe we should try separating this a bit better by introducing another type just for the UI representation (i.e. without provider… fields or extraRefs). Also [key: string] doesn't seem to be necessary then. This way, it’ll be easier to understand that the fields are just internal implementation details.

andreaskienle · 2025-08-05T16:23:52Z

src/components/Graphs/useGraph.ts

+  treeData: NodeData[],
+  colorBy: 'provider' | 'source',
+  colorMap: Record<string, string>,
+): { nodes: Node<NodeData>[]; edges: Edge[] } {


If we go for different types (internal and UI), this should return the UI structure then, e.g.

Suggested change

): { nodes: Node<NodeData>[]; edges: Edge[] } {

): { nodes: Node<NodeDataUi>[]; edges: Edge[] } {

andreaskienle · 2025-08-05T16:25:45Z

src/components/Graphs/useGraph.ts

+
+  const loading = managedResourcesLoading || providerConfigsLoading;
+
+  const treeData = useMemo(() => {


Did you find a need to use memoization? Also for building the color map below. For me it seems to be quite fast to work without memoization (if this was the reason to use it).

Also: Can we move this function to the utils file? (Or vice versa integrate the other helper functions (resolveProviderType etc) inside this hook)? I'm thinking of just extracting this whole thing into a function which gets the managedResources and providerConfigsList and returns the treedata (graphData 🤔)

andreaskienle · 2025-08-05T16:26:39Z

src/components/Graphs/useGraph.ts

+  const [nodes, setNodes] = useState<Node<NodeData>[]>([]);
+  const [edges, setEdges] = useState<Edge[]>([]);
+
+  useEffect(() => {
+    if (!treeData.length) {
+      setNodes([]);
+      setEdges([]);
+      return;
+    }
+    const { nodes, edges } = buildGraph(treeData, colorBy, colorMap);
+    setNodes(nodes);
+    setEdges(edges);
+  }, [treeData, colorBy, colorMap]);


Any reason why we hold nodes and edges as state like this? Would it also work if we just returned the calculated nodes and edges “on the fly” (directly)? (i.e. without useState)

Hubert-Szczepanski-SAP · 2025-08-05T19:56:21Z

@andreaskienle I fixed as much as I could now. Maybe we can move this to another PR which I'm preparing (adding another colorization)? Maybe we can discuss this on daily, but most of the change are just some code cleaning - I thought that we are aiming for "fast solution" to enable this feature to the stakeholders ASAP. I think we can polish this after that if you agree - but finally decision we can discuss tomorrow.

andreaskienle requested a review from Copilot July 30, 2025 09:42

Copilot AI reviewed Jul 30, 2025

View reviewed changes

Hubert-Szczepanski-SAP added 4 commits July 30, 2025 11:59

WiP

78ee948

working graph

a0d93b0

Fixing according to copilot review

822c966

Hubert-Szczepanski-SAP force-pushed the feat/adding-graphs-to-mcp branch from 7f6e5e5 to 822c966 Compare July 30, 2025 10:03

Hubert-Szczepanski-SAP added 5 commits July 30, 2025 12:10

removing unused providers

9669463

linter issues + adding command to package.json

940c0c5

fixing tests after removing providers

1b62db8

adding smaller yaml button

25eb7d3

temporary fix - can be deployed

487c72a

andreaskienle self-requested a review July 31, 2025 06:15

moving yalm viewer outside of Reactflow to fix bug

ce602d6

andreaskienle requested changes Jul 31, 2025

View reviewed changes

lucasgoral and others added 8 commits August 1, 2025 08:51

Merge branch 'main' into feat/adding-graphs-to-mcp

8933543

PR comments

10c515b

linter

6e1d5c5

Merge branch 'feat/adding-graphs-to-mcp' of https://github.com/openmc…

2531e39

…p-project/ui-frontend into feat/adding-graphs-to-mcp

react flow version upgrade + css changes

3fd97f2

linting

3c6d6e3

Merge branch 'main' into feat/adding-graphs-to-mcp

0500962

moving logic to hook

39abbe5

Hubert-Szczepanski-SAP requested a review from andreaskienle August 4, 2025 18:18

andreaskienle requested changes Aug 5, 2025

View reviewed changes

Hubert-Szczepanski-SAP added 5 commits August 5, 2025 20:19

Adding error handling for useGraph

45a61e9

Adding radio button from UI5 + linting

2198689

memo + removing as string

2315123

PR changes

10e657f

changing to Map from Record

cd52c2c

Hubert-Szczepanski-SAP added 2 commits August 5, 2025 21:48

linter

833ea18

removing unused tests after removing configName check

bcb8c6d

Merge branch 'main' into feat/adding-graphs-to-mcp

d9ab2b8

Hubert-Szczepanski-SAP mentioned this pull request Aug 6, 2025

Task: Add color by Helm Chart / Flux - Resources Graph + Code cleaning openmcp-project/backlog#215

Open

8 tasks

andreaskienle approved these changes Aug 6, 2025

View reviewed changes

Hubert-Szczepanski-SAP merged commit f6db3ca into main Aug 6, 2025
5 checks passed

Hubert-Szczepanski-SAP deleted the feat/adding-graphs-to-mcp branch August 6, 2025 07:50

	): { nodes: Node<NodeData>[]; edges: Edge[] } {
	): { nodes: Node<NodeDataUi>[]; edges: Edge[] } {


		const loading = managedResourcesLoading \|\| providerConfigsLoading;

		const treeData = useMemo(() => {

Feat/adding graph to mcp #210

Feat/adding graph to mcp #210

Uh oh!

Conversation

Hubert-Szczepanski-SAP commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andreaskienle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andreaskienle left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

andreaskienle Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Hubert-Szczepanski-SAP commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

Hubert-Szczepanski-SAP commented Jul 30, 2025 •

edited

Loading

andreaskienle left a comment •

edited

Loading