From 3143e74c6de30ccf789d600ac42ee647c70e8496 Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Mon, 10 Aug 2026 20:24:01 +0530 Subject: [PATCH 1/2] fix(ui): stop deep links 404ing on cold boot and fix incident table column sizing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects surfaced while reviewing the AI-mode Observability pages. All three live in shared UI, so Classic mode benefits from the last two as well. **Deep links / reloads landing on /404** `AppRouter` resolved routes as `ModeRoutes ?? AuthenticatedRoutes`. A non-default app mode registers its routes from the plugin that owns the mode, in an effect gated on `applications` loading, so for the first few renders the registry is legitimately empty. Falling through to the default routes in that window mounts their `path="*"` catch-all, which navigates to /404 and destroys the requested URL before the real routes ever mount — non-deterministically, depending on which request settles first. `useResolvedAppMode` already computes a `registrySettled` flag for exactly this window (it deliberately refuses to clear a valid session before it flips). It now returns that flag, and `AppRouter` holds a loader while a non-default mode is active but unregistered. Once settled and still unregistered — the plugin is genuinely uninstalled — it falls back to the default routes as before. The loader sits INSIDE `AuthenticatedApp` on purpose: `ApplicationsProvider` lives there and is what flips `applicationsLoaded`, so short-circuiting above it would deadlock. **Incident table: a long test case name stretched the name column** The cell declares `w-72`, but the table lays out `auto` and `overflow-wrap: break-word` does not shrink a word's min-content contribution — so the column grew to fit the longest name (~900px on a 1033px container). `wrap-anywhere` does shrink it; the floor is pinned to the same 18rem so auto-layout cannot then collapse the column and wrap every name onto three lines. **Incident table: the Assignee column collapsed when every row was unassigned** antd's `.ant-typography` sets `word-break: break-word`, which drops the "No Assignee" placeholder's min-content contribution to a single character. Owner names already render nowrap + ellipsis, so once every row is unassigned nothing holds the column open and it collapsed until the placeholder stacked one letter per line. The cell is now `whitespace-nowrap`, matching the Last Updated cell. Tests: two new AppRouter cases covering the registration window; both fail without the router change. Co-Authored-By: Claude Opus 5 --- .../components/AppRouter/AppRouter.test.tsx | 48 ++++++++++++++++++- .../ui/src/components/AppRouter/AppRouter.tsx | 24 +++++++++- .../IncidentManagerTable.component.tsx | 16 +++++-- .../ui/src/hooks/useResolvedAppMode.ts | 11 +++-- 4 files changed, 89 insertions(+), 10 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.test.tsx index c9bcaa6ff2d2..86514373e3df 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.test.tsx @@ -75,15 +75,25 @@ const setAuthState = (overrides: { isAuthenticated?: boolean; isApplicationLoading?: boolean; isAuthenticating?: boolean; + applicationsLoaded?: boolean; currentUser?: Record; }) => { - mockUseApplicationStore.mockImplementation(() => ({ + const state = { currentUser: {}, isAuthenticated: true, isApplicationLoading: false, isAuthenticating: false, + applicationsLoaded: true, ...overrides, - })); + }; + + // Honour the selector so individual fields (notably `applicationsLoaded`, + // which gates route registration) can be varied independently. + mockUseApplicationStore.mockImplementation((selector?: unknown) => + typeof selector === 'function' + ? (selector as (s: typeof state) => unknown)(state) + : state + ); }; const ModeRoutesMock: ComponentType = () => ( @@ -162,6 +172,40 @@ describe('AppRouter — App Mode routing integration', () => { expect(screen.queryByTestId('custom-mode-routes')).not.toBeInTheDocument(); }); + it('holds a loader instead of the default routes while a non-default mode is still registering', async () => { + // `applications` not loaded yet — the owning plugin registers its routes in + // an effect gated on that, so the registry is legitimately empty here. + setAuthState({ isAuthenticated: true, applicationsLoaded: false }); + writeAppMode('ai'); + + renderRouter(); + + expect(await screen.findByTestId('full-screen-loader')).toBeInTheDocument(); + // Rendering the default routes here would mount their `path="*"` + // catch-all, which navigates to /404 and destroys the requested URL. + expect( + screen.queryByTestId('default-authenticated-routes') + ).not.toBeInTheDocument(); + }); + + it('renders the mode routes once the owning plugin registers them after applications load', async () => { + setAuthState({ isAuthenticated: true, applicationsLoaded: false }); + writeAppMode('ai'); + + renderRouter(); + + expect(await screen.findByTestId('full-screen-loader')).toBeInTheDocument(); + + act(() => { + useAppRoutesRegistry.getState().registerRoutes('ai', ModeRoutesMock); + }); + + expect(await screen.findByTestId('custom-mode-routes')).toBeInTheDocument(); + expect( + screen.queryByTestId('default-authenticated-routes') + ).not.toBeInTheDocument(); + }); + it('swaps to the registered mode component when the AppMode changes mid-session', async () => { setAuthState({ isAuthenticated: true }); act(() => { diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx index fcb9cc4c67b0..5e15eb5cefe9 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx @@ -15,6 +15,7 @@ import { isEmpty } from 'lodash'; import { lazy } from 'react'; import { Navigate, Route, Routes } from 'react-router-dom'; import { useShallow } from 'zustand/react/shallow'; +import { DEFAULT_APP_MODE } from '../../constants/appMode.constants'; import { APP_ROUTER_ROUTES } from '../../constants/router.constants'; import { useApplicationStore } from '../../hooks/useApplicationStore'; import { useAppMode } from '../../hooks/useAppMode'; @@ -82,7 +83,18 @@ const AppRouter = () => { const appMode = useAppMode(); const ModeRoutes = useAppRoutesRegistry((state) => state.routes[appMode]); - useResolvedAppMode(); + const isRegistrySettled = useResolvedAppMode(); + + // A non-default mode's routes are registered by the plugin that owns the + // mode, in an effect that waits for `applications` to load. Until then the + // registry looks empty, and falling through to `AuthenticatedRoutes` mounts + // its `path="*"` catch-all, which navigates to /404 and destroys the + // requested URL before the real routes ever mount — a deep link or a reload + // on any mode-specific route lands on Not Found, non-deterministically. + // `registrySettled` is what distinguishes that startup window from a mode + // whose plugin is genuinely uninstalled, where falling back is correct. + const isModeRoutesPending = + appMode !== DEFAULT_APP_MODE && !ModeRoutes && !isRegistrySettled; /** * isApplicationLoading is true when the application is loading in AuthProvider @@ -99,9 +111,17 @@ const AppRouter = () => { if (isAuthenticated) { const AuthenticatedRoutesComponent = ModeRoutes ?? AuthenticatedRoutes; + // The loader has to sit INSIDE AuthenticatedApp: `ApplicationsProvider` + // lives there, and it is what flips `applicationsLoaded` and so unblocks + // registration. Short-circuiting above this point would deadlock — the + // routes could never register, so the loader would never clear. return ( - + {isModeRoutesPending ? ( + + ) : ( + + )} ); } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManagerTable.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManagerTable.component.tsx index ddb9f40c263d..087c0d90b22f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManagerTable.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManagerTable.component.tsx @@ -162,9 +162,14 @@ const IncidentManagerTable = ({ return ( - + {/* The table lays out `auto`, so `wrap-break-word` would leave a long + test case name as one unbreakable min-content word and stretch the + column to fit it. `wrap-anywhere` shrinks that contribution; the + floor matches the declared width so auto-layout cannot then + collapse the column and wrap every name onto three lines. */} + )} - + {/* antd's `.ant-typography` sets `word-break: break-word`, which drops + the "No Assignee" placeholder's min-content contribution to a single + character. Owner names render nowrap+ellipsis already, so once every + row is unassigned nothing holds the column open and it collapses, + stacking the placeholder one letter per line. */} + {testCaseResolutionStatusDetailsRender( record.testCaseResolutionStatusDetails, record diff --git a/openmetadata-ui/src/main/resources/ui/src/hooks/useResolvedAppMode.ts b/openmetadata-ui/src/main/resources/ui/src/hooks/useResolvedAppMode.ts index feef771bf574..90d93f3e3e5a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/hooks/useResolvedAppMode.ts +++ b/openmetadata-ui/src/main/resources/ui/src/hooks/useResolvedAppMode.ts @@ -90,10 +90,13 @@ const resolvePersonaAppMode = ( * the resolver falls through to compute a fresh candidate. * * Consumers should invoke this hook exactly once, high in the tree - * (e.g. `AppRoot`). It has no return value — its effects are `writeAppMode` - * / `clearAppMode` calls. + * (e.g. `AppRoot`). Its work is done through effects — `writeAppMode` / + * `clearAppMode` calls. It returns the `registrySettled` flag described + * below so the router can tell "the owning plugin has not registered its + * routes yet" apart from "no plugin owns this mode", which are otherwise + * indistinguishable from an empty registry. */ -export const useResolvedAppMode = (): void => { +export const useResolvedAppMode = (): boolean => { const defaultPersonaId = useApplicationStore( (state) => state.currentUser?.defaultPersona?.id ); @@ -252,4 +255,6 @@ export const useResolvedAppMode = (): void => { registeredRoutes, registrySettled, ]); + + return registrySettled; }; From fad1969080f99b4484592fab4d0b7a3da224898d Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Mon, 10 Aug 2026 20:31:48 +0530 Subject: [PATCH 2/2] fix(ui): let a host freeze DataQualityProvider's URL tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider derives its filters from the query string, which is global. A host that keeps the page mounted while routing elsewhere — AI mode caches visited routes — leaves a backgrounded Data Quality page re-deriving its filters from whatever route now owns the URL. Its filter keys (`testPlatforms`, `tags`, `serviceName`, …) overlap with the Test Library's, so applying a Test Library filter made the hidden page refetch with another page's values, flip its loading flag, and remount the whole dashboard — 26 stray requests per filter click, plus an error toast from the generic catch when one of them failed. `isActive` defaults to true, so hosts that unmount the page on navigation are unaffected. Re-running the effect on re-activation is intentional: it revalidates against the page's real filters when it comes back. Co-Authored-By: Claude Opus 5 --- .../pages/DataQuality/DataQualityProvider.tsx | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/DataQuality/DataQualityProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/DataQuality/DataQualityProvider.tsx index 085bec4b74fb..e007aae17bd0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/DataQuality/DataQualityProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/DataQuality/DataQualityProvider.tsx @@ -41,9 +41,19 @@ export const DataQualityContext = createContext( const DataQualityProvider = ({ children, createActions, + isActive = true, }: { children: React.ReactNode; createActions?: DataQualityContextInterface['createActions']; + /** + * Whether this page currently owns the URL. Filters here are derived from the + * query string, which is global — so a host that keeps the page mounted while + * routing elsewhere (AI mode caches visited routes) must pass `false`, or the + * backgrounded page re-derives its filters from whatever route now owns the + * query string and refetches with another page's params. Defaults to `true` + * for hosts that unmount the page on navigation. + */ + isActive?: boolean; }) => { const { tab: activeTab = DataQualityPageTabs.TEST_CASES } = useRequiredParams<{ @@ -152,12 +162,19 @@ const DataQualityProvider = ({ }; useEffect(() => { + // Backgrounded: hold the last loaded summary rather than refetching against + // a query string that now belongs to another route. Re-running on + // re-activation is intentional — it revalidates against the real filters. + if (!isActive) { + return; + } + if (getPrioritizedViewPermission(testCasePermission, Operation.ViewBasic)) { fetchTestSummary(filterParams); } else { setIsTestCaseSummaryLoading(false); } - }, [filterKey]); + }, [filterKey, isActive]); return (