fix(web): do not render Questions component at login path (#1828)

Stop mounting the `Questions` component when installer is at login path to avoid the login page
crashing because of _No QueryClient set, use QueryClientProvider to set
one_

web/src/components/core/LoginPage.test.tsx

@@ -22,15 +22,32 @@
 
 import React from "react";
 import { screen, within } from "@testing-library/react";
-import { plainRender } from "~/test-utils";
+import { installerRender, mockRoutes, plainRender } from "~/test-utils";
 import { LoginPage } from "~/components/core";
 import { AuthErrors } from "~/context/auth";
+import { PlainLayout } from "../layout";
+import { InstallationPhase } from "~/types/status";
 
 let consoleErrorSpy: jest.SpyInstance;
 let mockIsAuthenticated: boolean;
 let mockLoginError;
 const mockLoginFn = jest.fn();
 
+const phase: InstallationPhase = InstallationPhase.Startup;
+const isBusy: boolean = false;
+
+jest.mock("~/queries/status", () => ({
+  useInstallerStatus: () => ({
+    phase,
+    isBusy,
+  }),
+}));
+
+jest.mock("~/queries/issues", () => ({
+  ...jest.requireActual("~/queries/issues"),
+  useAllIssues: () => [],
+}));
+
 jest.mock("~/context/auth", () => ({
   ...jest.requireActual("~/context/auth"),
   useAuth: () => {
@@ -44,16 +61,29 @@ jest.mock("~/context/auth", () => ({
 
 describe("LoginPage", () => {
   beforeAll(() => {
+    mockRoutes("/login");
+    mockLoginFn.mockResolvedValue({ status: 200 });
     mockIsAuthenticated = false;
     mockLoginError = null;
-    mockLoginFn.mockResolvedValue({ status: 200 });
     consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
   });
 
   afterAll(() => {
     consoleErrorSpy.mockRestore();
   });
 
+  // Regresion test: when wrapped by Layout, it shouldn't fail with
+  // "No QueryClient set, use QueryClientProvider to set one"
+  // See commit ecc8d0865abbbebc7795e39bd85ec9462010d065
+  it("renders its content even when wrapped by Layout", async () => {
+    installerRender(
+      <PlainLayout>
+        <LoginPage />
+      </PlainLayout>,
+    );
+    await screen.findByRole("form", { name: "Login form" });
+  });
+
   describe("when user is not authenticated", () => {
     it("renders reference to root", () => {
       plainRender(<LoginPage />);

web/src/components/core/ServerError.test.tsx

@@ -22,7 +22,7 @@
 
 import React from "react";
 import { screen } from "@testing-library/react";
-import { plainRender } from "~/test-utils";
+import { installerRender } from "~/test-utils";
 
 import * as utils from "~/utils";
 import { ServerError } from "~/components/core";
@@ -46,7 +46,7 @@ jest.mock("~/components/layout/Layout", () => {
 
 describe("ServerError", () => {
   it("wraps a generic server problem message into a plain layout with neither, header nor sidebar", () => {
-    plainRender(<ServerError />);
+    installerRender(<ServerError />);
     expect(screen.queryByText("Header Mock")).toBeNull();
     expect(screen.queryByText("Sidebar Mock")).toBeNull();
     screen.getByText("PlainLayout Mock");
@@ -55,7 +55,7 @@ describe("ServerError", () => {
 
   it("calls location.reload when user clicks on 'Reload'", async () => {
     jest.spyOn(utils, "locationReload").mockImplementation(utils.noop);
-    const { user } = plainRender(<ServerError />);
+    const { user } = installerRender(<ServerError />);
     const reloadButton = await screen.findByRole("button", { name: /Reload/i });
     await user.click(reloadButton);
     expect(utils.locationReload).toHaveBeenCalled();

web/src/components/layout/Header.tsx

@@ -105,10 +105,12 @@ const OptionsDropdown = ({ showInstallerOptions }) => {
         </DropdownList>
       </Dropdown>
 
-      <InstallerOptions
-        isOpen={isInstallerOptionsOpen}
-        onClose={() => setIsInstallerOptionsOpen(false)}
-      />
+      {showInstallerOptions && (
+        <InstallerOptions
+          isOpen={isInstallerOptionsOpen}
+          onClose={() => setIsInstallerOptionsOpen(false)}
+        />
+      )}
     </>
   );
 };

web/src/components/layout/Layout.test.tsx

@@ -22,7 +22,7 @@
 
 import React from "react";
 import { screen } from "@testing-library/react";
-import { plainRender } from "~/test-utils";
+import { mockRoutes, installerRender } from "~/test-utils";
 import Layout from "./Layout";
 
 jest.mock("~/components/layout/Header", () => () => <div>Header Mock</div>);
@@ -32,37 +32,37 @@ jest.mock("~/components/questions/Questions", () => () => <div>Questions Mock</d
 
 describe("Layout", () => {
   it("renders a page with header and sidebar by default", () => {
-    plainRender(<Layout />);
+    installerRender(<Layout />);
     screen.getByText("Header Mock");
     screen.getByText("Sidebar Mock");
   });
 
   it("does not render the header when mountHeader=false", () => {
-    plainRender(<Layout mountHeader={false} />);
+    installerRender(<Layout mountHeader={false} />);
     expect(screen.queryByText("Header Mock")).toBeNull();
   });
 
   it("does not render the sidebar when mountSidebar=false", () => {
-    plainRender(<Layout mountSidebar={false} />);
+    installerRender(<Layout mountSidebar={false} />);
     expect(screen.queryByText("Sidebar Mock")).toBeNull();
   });
 
   describe("when children are not given", () => {
     it("renders router <Outlet />", () => {
-      plainRender(<Layout />);
+      installerRender(<Layout />);
       // NOTE: react-router-dom/Outlet is mock at src/test-utils
       screen.getByText("Outlet Content");
     });
 
     it("renders the questions component", () => {
-      plainRender(<Layout />);
+      installerRender(<Layout />);
       screen.getByText("Questions Mock");
     });
   });
 
   describe("when children are given", () => {
     it("renders children instead of router <Outlet />", () => {
-      plainRender(
+      installerRender(
         <Layout>
           <button>Dummy testing button</button>
         </Layout>,
@@ -73,8 +73,19 @@ describe("Layout", () => {
     });
 
     it("renders the questions component", () => {
-      plainRender(<Layout />);
+      installerRender(<Layout />);
       screen.getByText("Questions Mock");
     });
   });
+
+  describe("at /login path", () => {
+    beforeEach(() => {
+      mockRoutes("/login");
+    });
+
+    it("does not render the questions component", () => {
+      installerRender(<Layout />);
+      expect(screen.queryByText("Questions Mock")).toBeNull();
+    });
+  });
 });

web/src/components/layout/Layout.tsx

@@ -21,12 +21,13 @@
  */
 
 import React, { Suspense, useState } from "react";
-import { Outlet } from "react-router-dom";
-import { Page } from "@patternfly/react-core";
+import { Outlet, useLocation } from "react-router-dom";
+import { Page, PageProps } from "@patternfly/react-core";
 import { Questions } from "~/components/questions";
 import Header, { HeaderProps } from "~/components/layout/Header";
 import { Loading, Sidebar } from "~/components/layout";
 import { IssuesDrawer } from "~/components/core";
+import { ROOT } from "~/routes/paths";
 
 export type LayoutProps = React.PropsWithChildren<{
   mountHeader?: boolean;
@@ -46,30 +47,36 @@ const Layout = ({
   headerOptions = {},
   children,
 }: LayoutProps) => {
+  const location = useLocation();
   const [issuesDrawerVisible, setIssuesDrawerVisible] = useState<boolean>(false);
   const closeIssuesDrawer = () => setIssuesDrawerVisible(false);
   const toggleIssuesDrawer = () => setIssuesDrawerVisible(!issuesDrawerVisible);
 
+  const pageProps: Omit<PageProps, keyof React.HTMLProps<HTMLDivElement>> = {
+    isManagedSidebar: true,
+  };
+
+  if (mountSidebar) pageProps.sidebar = <Sidebar />;
+  if (mountHeader) {
+    pageProps.header = (
+      <Header
+        showSidebarToggle={mountSidebar}
+        toggleIssuesDrawer={toggleIssuesDrawer}
+        {...headerOptions}
+      />
+    );
+    // notificationDrawer is open/close from the header, it does not make sense
+    // to mount it if there is no header.
+    pageProps.notificationDrawer = <IssuesDrawer onClose={closeIssuesDrawer} />;
+    pageProps.isNotificationDrawerExpanded = issuesDrawerVisible;
+  }
+
   return (
     <>
-      <Page
-        isManagedSidebar
-        header={
-          mountHeader && (
-            <Header
-              showSidebarToggle={mountSidebar}
-              toggleIssuesDrawer={toggleIssuesDrawer}
-              {...headerOptions}
-            />
-          )
-        }
-        sidebar={mountSidebar && <Sidebar />}
-        notificationDrawer={<IssuesDrawer onClose={closeIssuesDrawer} />}
-        isNotificationDrawerExpanded={issuesDrawerVisible}
-      >
+      <Page {...pageProps}>
         <Suspense fallback={<Loading />}>{children || <Outlet />}</Suspense>
       </Page>
-      <Questions />
+      {location.pathname !== ROOT.login && <Questions />}
     </>
   );
 };

web/src/components/layout/Loading.test.tsx

@@ -23,7 +23,7 @@
 import React from "react";
 
 import { screen } from "@testing-library/react";
-import { plainRender } from "~/test-utils";
+import { installerRender, plainRender } from "~/test-utils";
 
 import Loading from "./Loading";
 
@@ -68,7 +68,7 @@ describe("Loading", () => {
 
   describe("when using the useLayout prop", () => {
     it("wraps the content within a PlainLayout with neither, header nor sidebar", () => {
-      plainRender(<Loading text="Making a test" useLayout />);
+      installerRender(<Loading text="Making a test" useLayout />);
       expect(screen.queryByText("Header Mock")).toBeNull();
       expect(screen.queryByText("Sidebar Mock")).toBeNull();
       screen.getByText("PlainLayout Mock");