fix(web): do not render App on protected routes

web/package/agama-web-ui.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Fri Jun 21 14:32:11 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- Avoid connection attempts when the user is not logged in
+  (gh#openSUSE/agama#1366).
+
 -------------------------------------------------------------------
 Thu Jun 20 05:33:29 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
 

web/src/App.jsx

@@ -19,13 +19,13 @@
  * find current contact information at www.suse.com.
  */
 
-import React, { useEffect, useState } from "react";
+import React from "react";
 import { Navigate, Outlet, useLocation } from "react-router-dom";
 import { Loading } from "./components/layout";
 import { Questions } from "~/components/questions";
 import { ServerError, Installation } from "~/components/core";
 import { useInstallerL10n } from "./context/installerL10n";
-import { useInstallerClient, useInstallerClientStatus } from "~/context/installer";
+import { useInstallerClientStatus } from "~/context/installer";
 import { useProduct } from "./context/product";
 import { CONFIG, INSTALL, STARTUP } from "~/client/phase";
 import { BUSY } from "~/client/status";
@@ -38,38 +38,10 @@ import { BUSY } from "~/client/status";
  *   error (3 by default). The component will keep trying to connect.
  */
 function App() {
-  const client = useInstallerClient();
   const location = useLocation();
-  const { connected, error } = useInstallerClientStatus();
+  const { connected, error, phase, status } = useInstallerClientStatus();
   const { selectedProduct, products } = useProduct();
   const { language } = useInstallerL10n();
-  const [status, setStatus] = useState(undefined);
-  const [phase, setPhase] = useState(undefined);
-
-  useEffect(() => {
-    if (client) {
-      return client.manager.onPhaseChange(setPhase);
-    }
-  }, [client, setPhase]);
-
-  useEffect(() => {
-    if (client) {
-      return client.manager.onStatusChange(setStatus);
-    }
-  }, [client, setStatus]);
-
-  useEffect(() => {
-    const loadPhase = async () => {
-      const phase = await client.manager.getPhase();
-      const status = await client.manager.getStatus();
-      setPhase(phase);
-      setStatus(status);
-    };
-
-    if (client) {
-      loadPhase().catch(console.error);
-    }
-  }, [client, setPhase, setStatus]);
 
   const Content = () => {
     if (error) return <ServerError />;

web/src/App.test.jsx

@@ -44,14 +44,16 @@ jest.mock("~/context/product", () => ({
   }
 }));
 
+const mockClientStatus = {
+  connected: true,
+  error: false,
+  phase: STARTUP,
+  status: BUSY
+};
+
 jest.mock("~/context/installer", () => ({
   ...jest.requireActual("~/context/installer"),
-  useInstallerClientStatus: () => {
-    return {
-      connected: true,
-      error: false
-    };
-  }
+  useInstallerClientStatus: () => mockClientStatus
 }));
 
 // Mock some components,
@@ -61,32 +63,12 @@ jest.mock("~/components/core/Installation", () => () => <div>Installation Mock</
 jest.mock("~/components/layout/Loading", () => () => <div>Loading Mock</div>);
 jest.mock("~/components/product/ProductSelectionProgress", () => () => <div>Product progress</div>);
 
-// this object holds the mocked callbacks
-const callbacks = {};
-const getStatusFn = jest.fn();
-const getPhaseFn = jest.fn();
-
-// capture the latest subscription to the manager#onPhaseChange for triggering it manually
-const onPhaseChangeFn = cb => {
-  callbacks.onPhaseChange = cb;
-};
-const onStatusChangeFn = cb => {
-  callbacks.onStatusChange = cb;
-};
-const changePhaseTo = phase => act(() => callbacks.onPhaseChange(phase));
-
 describe("App", () => {
   beforeEach(() => {
     // setting the language through a cookie
     document.cookie = "agamaLang=en-us; path=/;";
     createClient.mockImplementation(() => {
       return {
-        manager: {
-          getStatus: getStatusFn,
-          getPhase: getPhaseFn,
-          onPhaseChange: onPhaseChangeFn,
-          onStatusChange: onStatusChangeFn
-        },
         l10n: {
           locales: jest.fn().mockResolvedValue([["en_us", "English", "United States"]]),
           getLocales: jest.fn().mockResolvedValue(["en_us"]),
@@ -126,22 +108,10 @@ describe("App", () => {
     });
   });
 
-  describe("on the startup phase", () => {
-    beforeEach(() => {
-      getPhaseFn.mockResolvedValue(STARTUP);
-      getStatusFn.mockResolvedValue(BUSY);
-    });
-
-    it("renders the Loading screen", async () => {
-      installerRender(<App />, { withL10n: true });
-      await screen.findByText("Loading Mock");
-    });
-  });
-
   describe("when the service is busy during startup", () => {
     beforeEach(() => {
-      getPhaseFn.mockResolvedValue(STARTUP);
-      getStatusFn.mockResolvedValue(BUSY);
+      mockClientStatus.phase = STARTUP;
+      mockClientStatus.status = BUSY;
     });
 
     it("renders the Loading screen", async () => {
@@ -152,12 +122,12 @@ describe("App", () => {
 
   describe("on the CONFIG phase", () => {
     beforeEach(() => {
-      getPhaseFn.mockResolvedValue(CONFIG);
+      mockClientStatus.phase = CONFIG;
     });
 
     describe("if the service is busy", () => {
       beforeEach(() => {
-        getStatusFn.mockResolvedValue(BUSY);
+        mockClientStatus.status = BUSY;
       });
 
       it("redirects to product selection progress", async () => {
@@ -168,7 +138,7 @@ describe("App", () => {
 
     describe("if the service is not busy", () => {
       beforeEach(() => {
-        getStatusFn.mockResolvedValue(IDLE);
+        mockClientStatus.status = IDLE;
       });
 
       it("renders the application content", async () => {
@@ -180,7 +150,7 @@ describe("App", () => {
 
   describe("on the INSTALL phase", () => {
     beforeEach(() => {
-      getPhaseFn.mockResolvedValue(INSTALL);
+      mockClientStatus.phase = INSTALL;
       mockSelectedProduct = { id: "Fake product" };
     });
 
@@ -189,17 +159,4 @@ describe("App", () => {
       await screen.findByText("Installation Mock");
     });
   });
-
-  describe("when service phase changes", () => {
-    beforeEach(() => {
-      getPhaseFn.mockResolvedValue(CONFIG);
-    });
-
-    it("renders the Installation component on the INSTALL phase", async () => {
-      installerRender(<App />, { withL10n: true });
-      await screen.findByText(/Outlet Content/);
-      changePhaseTo(INSTALL);
-      await screen.findByText("Installation Mock");
-    });
-  });
 });

web/src/Protected.jsx

@@ -27,6 +27,8 @@ import { AppProviders } from "./context/app";
 export default function Protected() {
   const { isLoggedIn } = useAuth();
 
+  if (isLoggedIn === undefined) return;
+
   if (isLoggedIn === false) {
     return <Navigate to="/login" />;
   }

web/src/components/overview/SoftwareSection.test.jsx

@@ -56,7 +56,7 @@ beforeEach(() => {
   });
 });
 
-it("renders the required space and the selected patterns", async () => {
+it.only("renders the required space and the selected patterns", async () => {
   installerRender(<SoftwareSection />);
   await screen.findByText("500 MiB");
   await screen.findByText(kdePattern.summary);

web/src/context/installer.jsx

@@ -27,7 +27,9 @@ import { createDefaultClient } from "~/client";
 const InstallerClientContext = React.createContext(null);
 // TODO: we use a separate context to avoid changing all the codes to
 // `useInstallerClient`. We should merge them in the future.
-const InstallerClientStatusContext = React.createContext({ connected: false, error: false });
+const InstallerClientStatusContext = React.createContext({
+  connected: false, error: false, phase: undefined, status: undefined
+});
 
 /**
  * Returns the D-Bus installer client
@@ -77,6 +79,8 @@ function InstallerClientProvider({
   const [value, setValue] = useState(client);
   const [connected, setConnected] = useState(false);
   const [error, setError] = useState(false);
+  const [status, setStatus] = useState(undefined);
+  const [phase, setPhase] = useState(undefined);
 
   useEffect(() => {
     const connectClient = async () => {
@@ -99,6 +103,31 @@ function InstallerClientProvider({
     if (!value) connectClient();
   }, [setValue, value]);
 
+  useEffect(() => {
+    if (value) {
+      return value.manager.onPhaseChange(setPhase);
+    }
+  }, [value, setPhase]);
+
+  useEffect(() => {
+    if (value) {
+      return value.manager.onStatusChange(setStatus);
+    }
+  }, [value, setStatus]);
+
+  useEffect(() => {
+    const loadPhase = async () => {
+      const initialPhase = await value.manager.getPhase();
+      const initialStatus = await value.manager.getStatus();
+      setPhase(initialPhase);
+      setStatus(initialStatus);
+    };
+
+    if (value) {
+      loadPhase().catch(console.error);
+    }
+  }, [value, setPhase, setStatus]);
+
   useEffect(() => {
     if (!value) return;
 
@@ -115,7 +144,7 @@ function InstallerClientProvider({
 
   return (
     <InstallerClientContext.Provider value={value}>
-      <InstallerClientStatusContext.Provider value={{ connected, error }}>
+      <InstallerClientStatusContext.Provider value={{ connected, error, phase, status }}>
         {children}
       </InstallerClientStatusContext.Provider>
     </InstallerClientContext.Provider>

web/src/context/installer.test.jsx

@@ -24,16 +24,20 @@ import { act, screen } from "@testing-library/react";
 import { createDefaultClient } from "~/client";
 import { plainRender, createCallbackMock } from "~/test-utils";
 import { InstallerClientProvider, useInstallerClientStatus } from "./installer";
+import { STARTUP } from "~/client/phase";
+import { BUSY } from "~/client/status";
 
 jest.mock("~/client");
 
 // Helper component to check the client status.
 const ClientStatus = () => {
-  const { connected } = useInstallerClientStatus();
+  const { connected, phase, status } = useInstallerClientStatus();
 
   return (
     <ul>
       <li>{`connected: ${connected}`}</li>
+      <li>{`phase: ${phase}`}</li>
+      <li>{`status: ${status}`}</li>
     </ul>
   );
 };
@@ -43,7 +47,13 @@ describe("installer context", () => {
     createDefaultClient.mockImplementation(() => {
       return {
         onConnect: jest.fn(),
-        onDisconnect: jest.fn()
+        onDisconnect: jest.fn(),
+        manager: {
+          getPhase: jest.fn().mockResolvedValue(STARTUP),
+          getStatus: jest.fn().mockResolvedValue(BUSY),
+          onPhaseChange: jest.fn(),
+          onStatusChange: jest.fn()
+        }
       };
     });
   });
@@ -55,5 +65,7 @@ describe("installer context", () => {
       </InstallerClientProvider>
     );
     await screen.findByText("connected: false");
+    await screen.findByText("phase: 0");
+    await screen.findByText("status: 1");
   });
 });

web/src/context/installerL10n.test.jsx

@@ -35,6 +35,12 @@ const setUILocaleFn = jest.fn().mockResolvedValue();
 const client = {
   onConnect: jest.fn(),
   onDisconnect: jest.fn(),
+  manager: {
+    getPhase: jest.fn(),
+    getStatus: jest.fn(),
+    onPhaseChange: jest.fn(),
+    onStatusChange: jest.fn()
+  },
   l10n: {
     getUILocale: getUILocaleFn,
     setUILocale: setUILocaleFn,

web/src/test-utils.js

@@ -84,6 +84,18 @@ const Providers = ({ children, withL10n }) => {
     client.onDisconnect = noop;
   }
 
+  if (!client.manager) {
+    client.manager = {};
+  }
+
+  client.manager = {
+    getPhase: noop,
+    getStatus: noop,
+    onPhaseChange: noop,
+    onStatusChange: noop,
+    ...client.manager
+  };
+
   if (withL10n) {
     return (
       <InstallerClientProvider client={client}>