Merge pull request #15760 from logonoff/OCPBUGS-65930-release-4.19-preserve-search

[release-4.19] OCPBUGS-65930: Preserve query string in perspective switch + remove dev console folks from `reviewers`

Author: openshift-merge-bot[bot]

frontend/OWNERS

@@ -1,21 +1,18 @@
 reviewers:
+  - cajieh
   - jhadvig
-  - kyoto
+  - krishagarwal278
+  - Leo6Leo
   - rhamilto
+  - sg00dwin
   - spadgett
   - TheRealJon
-  - cajieh
-  - Mylanos
 approvers:
-  - christoph-jerolimov
   - cajieh
-  - invincibleJai
   - jhadvig
-  - kyoto
+  - logonoff
   - rawagner
   - rhamilto
   - spadgett
   - vikram-raj
   - vojtechszocs
-  - logonoff
-  - Mylanos

frontend/integration-tests/OWNERS

@@ -1,6 +1,4 @@
 reviewers:
-  - sanketpathak
-  - the-anton
   - yapei
 approvers:
   - jrichter1

frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx

@@ -1,5 +1,5 @@
-import * as React from 'react';
-import { useLocation } from 'react-router-dom-v5-compat';
+import { FC, useEffect } from 'react';
+import { createPath, useLocation } from 'react-router-dom-v5-compat';
 import { Perspective, PerspectiveContext } from '@console/dynamic-plugin-sdk';
 import { usePerspectives } from '@console/shared/src';
 import PerspectiveDetector from './PerspectiveDetector';
@@ -19,14 +19,14 @@ const getPerspectiveURLParam = (perspectives: Perspective[]) => {
   return perspectiveIDs.includes(perspectiveParam) ? perspectiveParam : null;
 };
 
-const DetectPerspective: React.FC<DetectPerspectiveProps> = ({ children }) => {
+const DetectPerspective: FC<DetectPerspectiveProps> = ({ children }) => {
   const [activePerspective, setActivePerspective, loaded] = useValuesForPerspectiveContext();
   const perspectiveExtensions = usePerspectives();
   const perspectiveParam = getPerspectiveURLParam(perspectiveExtensions);
   const location = useLocation();
-  React.useEffect(() => {
+  useEffect(() => {
     if (perspectiveParam && perspectiveParam !== activePerspective) {
-      setActivePerspective(perspectiveParam, location.pathname);
+      setActivePerspective(perspectiveParam, createPath(location));
     }
   }, [perspectiveParam, activePerspective, setActivePerspective, location]);
   return loaded ? (

frontend/packages/console-app/src/components/detect-perspective/PerspectiveDetector.tsx

@@ -1,5 +1,5 @@
-import * as React from 'react';
-import { useLocation } from 'react-router';
+import { FC, useEffect, useState } from 'react';
+import { useLocation, createPath } from 'react-router-dom-v5-compat';
 import { Perspective, ResolvedExtension } from '@console/dynamic-plugin-sdk';
 import { usePerspectives } from '@console/shared/src';
 
@@ -16,13 +16,13 @@ type PerspectiveDetectorProps = {
   setActivePerspective: (perspective: string, next: string) => void;
 };
 
-const Detector: React.FC<DetectorProps> = ({
+const Detector: FC<DetectorProps> = ({
   setActivePerspective,
   perspectiveExtensions,
   detectors,
 }) => {
-  const { pathname } = useLocation() ?? {};
-  let detectedPerspective: string;
+  const location = useLocation();
+  let detectedPerspective: string | undefined;
   const defaultPerspective =
     perspectiveExtensions.find((p) => p.properties.default) || perspectiveExtensions[0];
   const detectionResults = detectors.map((detector) => detector?.());
@@ -38,31 +38,31 @@ const Detector: React.FC<DetectorProps> = ({
     return true;
   });
 
-  React.useEffect(() => {
+  useEffect(() => {
     if (detectedPerspective) {
-      setActivePerspective(detectedPerspective, pathname);
+      setActivePerspective(detectedPerspective, createPath(location));
     } else if (defaultPerspective && (detectors.length < 1 || detectionComplete)) {
-      // set default perspective if there are no detectors or none of the detections were successfull
-      setActivePerspective(defaultPerspective.properties.id, pathname);
+      // set default perspective if there are no detectors or none of the detections were successful
+      setActivePerspective(defaultPerspective.properties.id, createPath(location));
     }
   }, [
     defaultPerspective,
     detectedPerspective,
     detectionComplete,
     detectors.length,
-    pathname,
+    location,
     setActivePerspective,
   ]);
 
   return null;
 };
 
-const PerspectiveDetector: React.FC<PerspectiveDetectorProps> = ({ setActivePerspective }) => {
+const PerspectiveDetector: FC<PerspectiveDetectorProps> = ({ setActivePerspective }) => {
   const perspectiveExtensions = usePerspectives();
-  const [detectors, setDetectors] = React.useState<
+  const [detectors, setDetectors] = useState<
     (undefined | ResolvedExtension<Perspective>['properties']['usePerspectiveDetection'])[]
   >();
-  React.useEffect(() => {
+  useEffect(() => {
     let resolveCount = 0;
     const resolvedDetectors: ResolvedExtension<
       Perspective

frontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsx

@@ -1,5 +1,5 @@
-import { mount } from 'enzyme';
-import { act } from 'react-dom/test-utils';
+import { render, waitFor } from '@testing-library/react';
+import { useLocation } from 'react-router-dom-v5-compat';
 import { Perspective } from '@console/dynamic-plugin-sdk';
 import { LoadedExtension } from '@console/plugin-sdk';
 import { usePerspectives } from '@console/shared/src';
@@ -13,12 +13,10 @@ jest.mock('@console/shared/src', () => ({
   usePerspectives: jest.fn(),
 }));
 
-jest.mock('react-router', () => {
-  return {
-    ...jest.requireActual('react-router'),
-    useLocation: jest.fn(() => ({ pathname: '' })),
-  };
-});
+jest.mock('react-router-dom-v5-compat', () => ({
+  ...jest.requireActual('react-router-dom-v5-compat'),
+  useLocation: jest.fn(() => ({ pathname: '' })),
+}));
 
 const mockPerspectives = [
   {
@@ -41,18 +39,24 @@ const mockPerspectives = [
 
 const setActivePerspective = jest.fn();
 
+const useLocationMock = useLocation as jest.Mock;
+
 describe('PerspectiveDetector', () => {
-  it('should set default perspective if there are no perspective detectors available', async () => {
+  beforeEach(() => {
+    setActivePerspective.mockClear();
+  });
+
+  it('should set default perspective when there are no perspective detectors available', async () => {
     (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives);
 
-    const wrapper = mount(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
-    expect(wrapper.isEmptyRender()).toBe(true);
-    expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
+    render(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
+
+    await waitFor(() => {
+      expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
+    });
   });
 
-  it('should set detected perspective if detection is successful', async () => {
-    // create a promise and capture the resolver such that we can use act later on to ensure
-    // the test waits for this promise to resolve before continuing
+  it('should set detected perspective when detection is successful', async () => {
     let promiseResolver: (value: () => [boolean, boolean]) => void;
     const testPromise = new Promise<() => [boolean, boolean]>(
       (resolver) => (promiseResolver = resolver),
@@ -61,17 +65,16 @@ describe('PerspectiveDetector', () => {
 
     (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives);
 
-    const wrapper = mount(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
-    await act(async () => {
-      promiseResolver(() => [true, false]);
+    render(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
+
+    promiseResolver(() => [true, false]);
+
+    await waitFor(() => {
+      expect(setActivePerspective).toHaveBeenCalledWith('dev', '');
     });
-    expect(wrapper.isEmptyRender()).toBe(true);
-    expect(setActivePerspective).toHaveBeenCalledWith('dev', '');
   });
 
-  it('should set default perspective if detection fails', async () => {
-    // create a promise and capture the resolver such that we can use act later on to ensure
-    // the test waits for this promise to resolve before continuing
+  it('should set default perspective when detection fails', async () => {
     let promiseResolver: (value: () => [boolean, boolean]) => void;
     const testPromise = new Promise<() => [boolean, boolean]>(
       (resolver) => (promiseResolver = resolver),
@@ -80,15 +83,16 @@ describe('PerspectiveDetector', () => {
 
     (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives);
 
-    const wrapper = mount(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
-    await act(async () => {
-      promiseResolver(() => [false, false]);
+    render(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
+
+    promiseResolver(() => [false, false]);
+
+    await waitFor(() => {
+      expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
     });
-    expect(wrapper.isEmptyRender()).toBe(true);
-    expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
   });
 
-  it('should set admin as default perspective if all perspectives are disabled', async () => {
+  it('should set admin as default perspective when all perspectives are disabled', async () => {
     const perspectives: PerspectiveType[] = [
       {
         id: 'dev',
@@ -118,8 +122,7 @@ describe('PerspectiveDetector', () => {
       },
     ];
     window.SERVER_FLAGS.perspectives = JSON.stringify(perspectives);
-    // create a promise and capture the resolver such that we can use act later on to ensure
-    // the test waits for this promise to resolve before continuing
+
     let promiseResolver: (value: () => [boolean, boolean]) => void;
     const testPromise = new Promise<() => [boolean, boolean]>(
       (resolver) => (promiseResolver = resolver),
@@ -128,11 +131,35 @@ describe('PerspectiveDetector', () => {
 
     (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives);
 
-    const wrapper = mount(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
-    await act(async () => {
-      promiseResolver(() => [false, false]);
+    render(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
+
+    promiseResolver(() => [false, false]);
+
+    await waitFor(() => {
+      expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
+    });
+  });
+
+  it('preserves query and hash when setting perspective', async () => {
+    let promiseResolver: (value: () => [boolean, boolean]) => void;
+    const testPromise = new Promise<() => [boolean, boolean]>(
+      (resolver) => (promiseResolver = resolver),
+    );
+    mockPerspectives[1].properties.usePerspectiveDetection = () => testPromise;
+
+    (usePerspectives as jest.Mock).mockImplementation(() => mockPerspectives);
+    useLocationMock.mockImplementation(() => ({
+      pathname: '/some/path',
+      search: '?query=param',
+      hash: '#some-hash',
+    }));
+
+    render(<PerspectiveDetector setActivePerspective={setActivePerspective} />);
+
+    promiseResolver(() => [true, false]);
+
+    await waitFor(() => {
+      expect(setActivePerspective).toHaveBeenCalledWith('dev', '/some/path?query=param#some-hash');
     });
-    expect(wrapper.isEmptyRender()).toBe(true);
-    expect(setActivePerspective).toHaveBeenCalledWith('admin', '');
   });
 });

frontend/packages/console-telemetry-plugin/OWNERS

@@ -1,8 +1,3 @@
-reviewers:
-  - christoph-jerolimov
-  - lokanandaprabhu
-  - lucifergene
-  - vikram-raj
 approvers:
   - christoph-jerolimov
   - debsmita1

frontend/packages/console-telemetry-plugin/integration-tests/OWNERS

@@ -1,6 +1,3 @@
-reviewers:
-  - sanketpathak
-  - the-anton
 approvers:
   - jrichter1
   - psrna

frontend/packages/dev-console/OWNERS

@@ -1,8 +1,3 @@
-reviewers:
-  - jerolimov
-  - lokanandaprabhu
-  - lucifergene
-  - vikram-raj
 approvers:
   - debsmita1
   - divyanshiGupta

frontend/packages/dev-console/integration-tests/OWNERS

@@ -1,6 +1,3 @@
-reviewers:
-  - sanketpathak
-  - the-anton
 approvers:
   - jrichter1
   - psrna

frontend/packages/git-service/OWNERS

@@ -1,8 +1,3 @@
-reviewers:
-  - jerolimov
-  - lokanandaprabhu
-  - lucifergene
-  - vikram-raj
 approvers:
   - debsmita1
   - divyanshiGupta