Changing logic for connect button to MCP #155
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR changes the logic for the connect button in ControlPlaneCard to use MCP-specific health conditions.
- Introduces a helper function (canConnectToMCP) to check for APIServer, Authentication, and Authorization health.
- Propagates this check to set the disabled state of the ConnectButton.
| @@ -106,7 +123,7 @@ export function ControlPlaneCard({ | |||
| resourceType={'managedcontrolplanes'} | |||
| /> | |||
| <ConnectButton | |||
| disabled={controlPlane.status?.status !== ReadyStatus.Ready} | |||
| disabled={isConnectButtonEnabled} | |||
There was a problem hiding this comment.
It appears that 'isConnectButtonEnabled' represents the connectable status; however, passing it directly to the 'disabled' prop will disable the button when the control plane is connectable. Consider inverting the condition (e.g., disabled={!isConnectButtonEnabled}) to align with the intended behavior.
| disabled={isConnectButtonEnabled} | |
| disabled={!isConnectButtonEnabled} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Can you check @Hubert-Szczepanski-SAP?
There was a problem hiding this comment.
I checked and made corrections. I made mistake in testing, because we receive status which is "True" - it's not boolean, it's string, so it was working for me in this scenario. Now it should be ok
There was a problem hiding this comment.
Ok, maybe we could add a small unit test here to make it clearer what’s happening. This would also help us getting more into the habit of writing tests 😅💪.
We could move the canConnectToMCP function to controlPlanes.ts and test it in isolation. Should be pretty straightforward. No need to write a Cypress test, just a "normal" unit test.
Here's a similar example we could use as reference.
What are your thoughts?
There was a problem hiding this comment.
That's a very good idea. I'll write some tests :)
changing condition
Hubert-Szczepanski-SAP
force-pushed
the
feat/changing-logic-for-mcp-connect-button
branch
from
00d0e06 to
25e5b22
Compare
…github.com/openmcp-project/ui-frontend into feat/changing-logic-for-mcp-connect-button
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: