From de4109f206f0747497878bc3ecd21a1503e63d27 Mon Sep 17 00:00:00 2001 From: Jon Jackson Date: Thu, 7 May 2026 14:56:01 -0400 Subject: [PATCH 1/2] OCPBUGS-84047: Fix perspective stuck issue with transition guard Fixes perspective getting stuck when switching away from plugin-registered perspectives (e.g., Fleet Management). Plugins watch activePerspective changes and force back to their perspective, preventing user-initiated switches. Root cause: Plugin effects fire during perspective transitions and override the user's switch by calling setActivePerspective with the plugin's perspective. Solution: Add transition guard (isTransitioning ref) that blocks setPerspective calls during active transitions, preventing plugin interference. Additional fixes: - Extract getPathWithoutPerspectiveParam to utils.ts to eliminate duplication - Strip only ?perspective= param to prevent loops while preserving other query params and hash - Navigate before updating perspective state to prevent plugins from seeing mismatch - Remove location from effect deps to prevent firing on every navigation - Restore ACM default on initial load (when no previous perspective exists) Co-Authored-By: Claude Sonnet 4.5 --- .../detect-perspective/DetectPerspective.tsx | 11 +++++-- .../PerspectiveDetector.tsx | 13 ++++++--- .../__tests__/PerspectiveDetector.spec.tsx | 13 +++++---- .../useValuesForPerspectiveContext.ts | 29 +++++++++++++++++-- .../components/detect-perspective/utils.ts | 19 ++++++++++++ 5 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 frontend/packages/console-app/src/components/detect-perspective/utils.ts diff --git a/frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx b/frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx index 94c1738d443..67549eb606c 100644 --- a/frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx +++ b/frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx @@ -1,12 +1,13 @@ import type { FC } from 'react'; import { useEffect } from 'react'; -import { createPath, useLocation } from 'react-router'; +import { useLocation } from 'react-router'; import type { Perspective } from '@console/dynamic-plugin-sdk'; import { PerspectiveContext } from '@console/dynamic-plugin-sdk'; import { LoadingBox } from '@console/shared/src/components/loading/LoadingBox'; import { usePerspectives } from '@console/shared/src/hooks/usePerspectives'; import PerspectiveDetector from './PerspectiveDetector'; import { useValuesForPerspectiveContext } from './useValuesForPerspectiveContext'; +import { getPathWithoutPerspectiveParam } from './utils'; type DetectPerspectiveProps = { children: React.ReactNode; @@ -29,9 +30,13 @@ const DetectPerspective: FC = ({ children }) => { const location = useLocation(); useEffect(() => { if (perspectiveParam && perspectiveParam !== activePerspective) { - setActivePerspective(perspectiveParam, createPath(location)); + // Strip ?perspective= param to avoid loop, but preserve other query params and hash + setActivePerspective(perspectiveParam, getPathWithoutPerspectiveParam(location)); } - }, [perspectiveParam, activePerspective, setActivePerspective, location]); + // location is intentionally excluded from deps to prevent firing on every navigation + // The effect should only run when perspectiveParam changes (URL param added/changed) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [perspectiveParam, activePerspective, setActivePerspective]); return loaded ? ( activePerspective ? ( diff --git a/frontend/packages/console-app/src/components/detect-perspective/PerspectiveDetector.tsx b/frontend/packages/console-app/src/components/detect-perspective/PerspectiveDetector.tsx index 08df8a927b1..c6767a2abfb 100644 --- a/frontend/packages/console-app/src/components/detect-perspective/PerspectiveDetector.tsx +++ b/frontend/packages/console-app/src/components/detect-perspective/PerspectiveDetector.tsx @@ -1,8 +1,9 @@ import type { FC } from 'react'; import { useEffect, useState } from 'react'; -import { useLocation, createPath } from 'react-router'; +import { useLocation } from 'react-router'; import type { Perspective, ResolvedExtension } from '@console/dynamic-plugin-sdk'; import { usePerspectives } from '@console/shared/src/hooks/usePerspectives'; +import { getPathWithoutPerspectiveParam } from './utils'; type DetectorProps = { setActivePerspective: (perspective: string, next: string) => void; @@ -40,18 +41,22 @@ const Detector: FC = ({ }); useEffect(() => { + const pathWithoutPerspectiveParam = getPathWithoutPerspectiveParam(location); if (detectedPerspective) { - setActivePerspective(detectedPerspective, createPath(location)); + // Strip ?perspective= param to avoid loop, but preserve other query params and hash + setActivePerspective(detectedPerspective, pathWithoutPerspectiveParam); } else if (defaultPerspective && (detectors.length < 1 || detectionComplete)) { // set default perspective if there are no detectors or none of the detections were successful - setActivePerspective(defaultPerspective.properties.id, createPath(location)); + setActivePerspective(defaultPerspective.properties.id, pathWithoutPerspectiveParam); } + // location is intentionally excluded from deps to prevent firing on every navigation + // The effect should only run when detection completes or changes + // eslint-disable-next-line react-hooks/exhaustive-deps }, [ defaultPerspective, detectedPerspective, detectionComplete, detectors.length, - location, setActivePerspective, ]); diff --git a/frontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsx b/frontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsx index 3a786c08d69..bd9de78d168 100644 --- a/frontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsx +++ b/frontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsx @@ -53,7 +53,7 @@ describe('PerspectiveDetector', () => { render(); await waitFor(() => { - expect(setActivePerspective).toHaveBeenCalledWith('admin', ''); + expect(setActivePerspective).toHaveBeenCalledWith('admin', '/'); }); }); @@ -71,7 +71,7 @@ describe('PerspectiveDetector', () => { promiseResolver(() => [true, false]); await waitFor(() => { - expect(setActivePerspective).toHaveBeenCalledWith('dev', ''); + expect(setActivePerspective).toHaveBeenCalledWith('dev', '/'); }); }); @@ -89,7 +89,7 @@ describe('PerspectiveDetector', () => { promiseResolver(() => [false, false]); await waitFor(() => { - expect(setActivePerspective).toHaveBeenCalledWith('admin', ''); + expect(setActivePerspective).toHaveBeenCalledWith('admin', '/'); }); }); @@ -137,11 +137,11 @@ describe('PerspectiveDetector', () => { promiseResolver(() => [false, false]); await waitFor(() => { - expect(setActivePerspective).toHaveBeenCalledWith('admin', ''); + expect(setActivePerspective).toHaveBeenCalledWith('admin', '/'); }); }); - it('preserves query and hash when setting perspective', async () => { + it('preserves query params and hash but strips perspective param when setting perspective', async () => { let promiseResolver: (value: () => [boolean, boolean]) => void; const testPromise = new Promise<() => [boolean, boolean]>( (resolver) => (promiseResolver = resolver), @@ -151,7 +151,7 @@ describe('PerspectiveDetector', () => { (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives); useLocationMock.mockImplementation(() => ({ pathname: '/some/path', - search: '?query=param', + search: '?query=param&perspective=old', hash: '#some-hash', })); @@ -160,6 +160,7 @@ describe('PerspectiveDetector', () => { promiseResolver(() => [true, false]); await waitFor(() => { + // Preserves query params and hash, but strips ?perspective= to avoid loop expect(setActivePerspective).toHaveBeenCalledWith('dev', '/some/path?query=param#some-hash'); }); }); diff --git a/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts b/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts index 4be72bae345..63047ca7f0f 100644 --- a/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts +++ b/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts @@ -1,4 +1,4 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useState, useRef } from 'react'; import { useNavigate } from 'react-router'; import type { PerspectiveType, UseActivePerspective } from '@console/dynamic-plugin-sdk'; import { @@ -27,6 +27,8 @@ export const useValuesForPerspectiveContext = (): [ const latestPerspective = loaded && (preferredPerspective || lastPerspective); const acmPerspectiveExtension = usePerspectiveExtension(ACM_PERSPECTIVE_ID); const existingPerspective = activePerspective || latestPerspective; + // Default to ACM perspective on initial load if no previous perspective exists + // The transition guard prevents this from interfering during user-initiated switches const perspective = !!acmPerspectiveExtension && !existingPerspective ? ACM_PERSPECTIVE_ID @@ -34,13 +36,34 @@ export const useValuesForPerspectiveContext = (): [ const isValidPerspective = loaded && perspectiveExtensions.some((p) => p.properties.id === perspective); + // Guard to prevent plugins from forcing perspective during transitions + const isTransitioning = useRef(false); + const setPerspective = useCallback( (newPerspective, next) => { + // Ignore calls during transitions - plugins trying to force perspective back + if (isTransitioning.current) { + return; + } + + // Set guard to block plugin interference + isTransitioning.current = true; + + // Navigate FIRST, then update perspective state + // This prevents plugins from seeing activePerspective change while still on old route + // which triggers their perspective-forcing logic + navigate(next || '/'); + + // Update perspective state after navigation starts setLastPerspective(newPerspective); setActivePerspective(newPerspective); - // Navigate to next or root and let the default page determine where to go to next - navigate(next || '/'); fireTelemetryEvent('Perspective Changed', { perspective: newPerspective }); + + // Clear guard after navigation and state updates complete + // Use setTimeout to ensure this runs after all synchronous effects + setTimeout(() => { + isTransitioning.current = false; + }, 0); }, [setLastPerspective, setActivePerspective, navigate, fireTelemetryEvent], ); diff --git a/frontend/packages/console-app/src/components/detect-perspective/utils.ts b/frontend/packages/console-app/src/components/detect-perspective/utils.ts new file mode 100644 index 00000000000..c5def0d97d6 --- /dev/null +++ b/frontend/packages/console-app/src/components/detect-perspective/utils.ts @@ -0,0 +1,19 @@ +import type { Location } from 'react-router'; +import { createPath } from 'react-router'; + +/** + * Strip ?perspective= param to prevent loops, but preserve other query params and hash. + * + * The ?perspective= param can create a redirect loop when passed through navigation. + * This helper ensures we preserve all other query params and the hash fragment while + * removing only the problematic perspective param. + * + * @param location - The current location object from useLocation() + * @returns Path string with perspective param removed (e.g., "/path?other=value#hash") + */ +export const getPathWithoutPerspectiveParam = (location: Location): string => { + const path = createPath(location); + const url = new URL(path, window.location.origin); + url.searchParams.delete('perspective'); + return url.pathname + url.search + url.hash; +}; From 396f0cf2d6095d4673771eb5a7596dc4edfea33e Mon Sep 17 00:00:00 2001 From: Jon Jackson Date: Mon, 11 May 2026 10:16:22 -0400 Subject: [PATCH 2/2] OCPBUGS-84047: Add try/finally to transition guard for exception safety Wrap navigate, setLastPerspective, setActivePerspective, and fireTelemetryEvent in try/finally block to ensure isTransitioning guard is always released, even if any of those operations throw synchronously. Co-Authored-By: Claude Sonnet 4.5 --- .../useValuesForPerspectiveContext.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts b/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts index 63047ca7f0f..824eaf52db1 100644 --- a/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts +++ b/frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.ts @@ -49,21 +49,23 @@ export const useValuesForPerspectiveContext = (): [ // Set guard to block plugin interference isTransitioning.current = true; - // Navigate FIRST, then update perspective state - // This prevents plugins from seeing activePerspective change while still on old route - // which triggers their perspective-forcing logic - navigate(next || '/'); + try { + // Navigate FIRST, then update perspective state + // This prevents plugins from seeing activePerspective change while still on old route + // which triggers their perspective-forcing logic + navigate(next || '/'); - // Update perspective state after navigation starts - setLastPerspective(newPerspective); - setActivePerspective(newPerspective); - fireTelemetryEvent('Perspective Changed', { perspective: newPerspective }); - - // Clear guard after navigation and state updates complete - // Use setTimeout to ensure this runs after all synchronous effects - setTimeout(() => { - isTransitioning.current = false; - }, 0); + // Update perspective state after navigation starts + setLastPerspective(newPerspective); + setActivePerspective(newPerspective); + fireTelemetryEvent('Perspective Changed', { perspective: newPerspective }); + } finally { + // Clear guard after navigation and state updates complete + // Use setTimeout to ensure this runs after all synchronous effects + setTimeout(() => { + isTransitioning.current = false; + }, 0); + } }, [setLastPerspective, setActivePerspective, navigate, fireTelemetryEvent], );