refactor: replace RouterUtils with ObservabilityRouterClassBase for navigation paths#27956
Open
ShaileshParmar11 wants to merge 5 commits intomainfrom
Open
refactor: replace RouterUtils with ObservabilityRouterClassBase for navigation paths#27956ShaileshParmar11 wants to merge 5 commits intomainfrom
ShaileshParmar11 wants to merge 5 commits intomainfrom
Conversation
github-actions
Bot
added
safe to test
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors UI navigation to route through ObservabilityRouterClassBase (instead of calling RouterUtils helpers or ROUTES constants directly) for observability + data quality related paths, centralizing route generation behind a single abstraction.
Changes:
- Extend
ObservabilityRouterClassBasewith additional helpers for alerts list, incident manager, and test suite/test case navigation. - Update multiple pages/components to use
observabilityRouterClassBasefor navigation/breadcrumb URLs (test suites, test cases, incident manager, observability alerts). - Remove now-unneeded direct
RouterUtilsimports /ROUTESusages in the touched call sites.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/ObservabilityRouterClassBase.ts | Adds new route helper methods to centralize observability/data-quality navigation paths. |
| openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsx | Switches breadcrumb/navigation URL generation for test suite to use the router class base. |
| openmetadata-ui/src/main/resources/ui/src/pages/IncidentManager/IncidentManagerDetailPage/IncidentManagerDetailPage.tsx | Replaces direct ROUTES/RouterUtils calls with router class base methods for incident/test-case navigation. |
| openmetadata-ui/src/main/resources/ui/src/pages/DataQuality/DataQualityPage.tsx | Uses router class base for navigating to test suite/test case after form actions. |
| openmetadata-ui/src/main/resources/ui/src/pages/AlertDetailsPage/AlertDetailsPage.tsx | Uses router class base for observability alerts list breadcrumb + post-delete navigation. |
| openmetadata-ui/src/main/resources/ui/src/pages/AddObservabilityPage/AddObservabilityPage.tsx | Uses router class base for alerts list breadcrumb and post-save navigation to alert details. |
| openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManager.component.tsx | Uses router class base for linking to test case detail pages from incident manager list. |
| openmetadata-ui/src/main/resources/ui/src/components/Explore/EntitySummaryPanel/DataQualityTab/DataQualityTab.tsx | Uses router class base for linking from summary panel test case cards. |
| openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuiteList/TestSuites.component.tsx | Uses router class base for test suite link navigation. |
| openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsx | Uses router class base for navigating to the test case SQL query tab. |
| openmetadata-ui/src/main/resources/ui/src/components/DataQuality/DataQualityDashboard/DataQualityDashboard.component.tsx | Uses router class base for incident manager redirect paths in dashboard widgets. |
| openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddToBundleSuiteModal/AddToBundleSuiteModal.component.tsx | Uses router class base to navigate to a selected bundle suite after adding test cases. |
| openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsx | Uses router class base for linking to test case “Issues” tab from profiler tooltip. |
| openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx | Uses router class base for navigation to test suite/test case views from profiler DQ tab. |
Comment on lines
+47
to
+88
| public getObservabilityAlertsListPath(): string { | ||
| return ROUTES.OBSERVABILITY_ALERTS; | ||
| } | ||
|
|
||
| public getIncidentManagerPath(): string { | ||
| return ROUTES.INCIDENT_MANAGER; | ||
| } | ||
|
|
||
| public getObservabilityAlertsEditPath(fqn: string): string { | ||
| return getObservabilityAlertsEditPath(fqn); | ||
| } | ||
|
|
||
| public getObservabilityAlertDetailsPath(fqn: string, tab?: string): string { | ||
| return getObservabilityAlertDetailsPath(fqn, tab); | ||
| } | ||
|
|
||
| public getTestSuitePath(testSuiteFqn: string): string { | ||
| return getTestSuitePath(testSuiteFqn); | ||
| } | ||
|
|
||
| public getTestCaseDetailPagePath( | ||
| fqn: string, | ||
| tab: TestCasePageTabs = TestCasePageTabs.TEST_CASE_RESULTS | ||
| ): string { | ||
| return getTestCaseDetailPagePath(fqn, tab); | ||
| } | ||
|
|
||
| public getTestCaseVersionPath( | ||
| fqn: string, | ||
| version: string, | ||
| tab?: string | ||
| ): string { | ||
| return getTestCaseVersionPath(fqn, version, tab); | ||
| } | ||
|
|
||
| public getTestCaseDimensionsDetailPagePath( | ||
| fqn: string, | ||
| dimensionKey: string, | ||
| tab: TestCasePageTabs = TestCasePageTabs.TEST_CASE_RESULTS | ||
| ): string { | ||
| return getTestCaseDimensionsDetailPagePath(fqn, dimensionKey, tab); | ||
| } |
Comment on lines
43
to
+49
| public getAddObservabilityAlertsPath(): string { | ||
| return ROUTES.ADD_OBSERVABILITY_ALERTS; | ||
| } | ||
|
|
||
| public getObservabilityAlertsListPath(): string { | ||
| return ROUTES.OBSERVABILITY_ALERTS; | ||
| } |
Contributor
Jest test CoverageUI tests summary
|
Contributor
🔴 Playwright Results — 1 failure(s), 13 flaky✅ 4014 passed · ❌ 1 failed · 🟡 13 flaky · ⏭️ 86 skipped
Genuine Failures (failed on all attempts)❌
|
…ty and IncidentManager components
Code Review ✅ ApprovedRefactors navigation paths for observability and data-quality components by migrating from raw utilities to the ObservabilityRouterClassBase singleton. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
ObservabilityRouterClassBasewith six new path helpers and migrates all observability + data-quality callers from rawRouterUtilshelpers /ROUTES.*literals to the singleton, so navigation paths for these pages now flow through a single abstraction.This is the OpenMetadata-side change that enables downstream embedding of observability pages inside Collate's AskCollate experience, where a Collate-side override of
ObservabilityRouterClassBaserewrites paths to a different prefix when in embedded mode. Stand-alone OpenMetadata behavior is unchanged — the base class just delegates to the existingRouterUtilshelpers andROUTESconstants.What changed
New methods on
ObservabilityRouterClassBase(each delegates to its existingRouterUtils/ROUTEScounterpart):getTestSuitePath(fqn)getTestCaseDetailPagePath(fqn, tab?)getTestCaseVersionPath(fqn, version, tab?)getTestCaseDimensionsDetailPagePath(fqn, dimensionKey, tab?)getIncidentManagerPath()getObservabilityAlertsListPath()Call-site migrations (raw helper/constant → singleton method) across:
pages/DataQuality/DataQualityPage.tsxpages/IncidentManager/IncidentManagerDetailPage/IncidentManagerDetailPage.tsxpages/AddObservabilityPage/AddObservabilityPage.tsxpages/AlertDetailsPage/AlertDetailsPage.tsxpages/TestSuiteDetailsPage/TestSuiteDetailsPage.component.tsxcomponents/IncidentManager/IncidentManager.component.tsxcomponents/DataQuality/DataQualityDashboard/DataQualityDashboard.component.tsxcomponents/DataQuality/TestSuite/TestSuiteList/TestSuites.component.tsxcomponents/DataQuality/AddToBundleSuiteModal/AddToBundleSuiteModal.component.tsxcomponents/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsxcomponents/Database/Profiler/DataQualityTab/DataQualityTab.tsxcomponents/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsxcomponents/Explore/EntitySummaryPanel/DataQualityTab/DataQualityTab.tsxThe standalone helpers in
RouterUtilsare left exported and untouched — only the callers we wanted centralized were migrated. Out-of-scope callers (utility modules, contracts UI, destination pages outside the observability tree) continue to use the standalone helpers.Type of change
Test plan
Unit tests extended to cover every new method and every migrated call site:
ObservabilityRouterClassBase.test.ts: 20 cases (was 11) — covers all six new wrapper methods including default-tab fallback paths and singleton identity.TestSuites,AddToBundleSuiteModal,IncidentManager,FailedTestCaseSampleData(new test file),TestSummaryCustomTooltip,DataQualityDashboard(covers bothIncidentTypeAreaChartWidgetdrill-down paths), andTestSuiteDetailsPage— each asserts the migrated value matches whatobservabilityRouterClassBase.<method>(...)produces.make ui-checkstyle-changed.Why now
The base/override pattern is already in place (
setEmbeddedMode/isEmbeddedModeand four pre-existing wrapper methods). This PR finishes the surface area so consumers don't need to mix singleton calls and rawRouterUtilscalls within the same page.Summary by Gitar
DataQualityTabto standardize container overflow, border, and shadow properties for tables.This will update automatically on new commits.