-
Notifications
You must be signed in to change notification settings - Fork 17
fix: monitoring show conditions #3080
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
Conversation
|
Skipped: This PR does not contain any of your configured keywords: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a centralized utility function to determine when monitoring features should be displayed for databases, consolidating previously scattered logic across multiple components.
Key changes:
- Created
canShowTenantMonitoringutility to standardize monitoring visibility checks - Applied the new utility across 5 components to ensure consistent behavior
- Monitoring is now shown only when: (1) ControlPlane exists with a non-empty ID, or (2) ControlPlane is absent but cluster-level monitoring metadata exists
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/monitoringVisibility.ts | New utility function to centralize monitoring visibility logic |
| src/containers/Tenant/ObjectSummary/SchemaTree/SchemaTree.tsx | Applies monitoring visibility check to schema tree actions |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantOverview.tsx | Uses new utility to determine monitoring tab availability |
| src/containers/Tenant/Diagnostics/Diagnostics.tsx | Integrates monitoring visibility check in diagnostics page configuration |
| src/containers/Header/Header.tsx | Updates header monitoring link logic with new utility |
| src/components/TenantNameWrapper/TenantNameWrapper.tsx | Filters monitoring links based on visibility check |
Raubzeug
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets do a helper like monitoringTabAvailable, it can be used in 3 places. Why to copy-paste?
We already have helper canShowTenantMonitoring the condition for tab is uiFactory additional renderMonitoring function canShowTenantMonitoring is used in 5 places now implementing your suggestion will give usage of one helper in two places (link) and another helper (tab) in three places Whats the profit? The only copy-past now is uiFactory.renderMonitoring === 'function' |
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔺
Current: 66.09 MB | Main: 66.08 MB
Diff: +4.42 KB (0.01%)
ℹ️ CI Information