Merge pull request #821 from openSUSE/fix-loading

Properly track the status
agama-project · Oct 26, 2023 · ff4fc26 · ff4fc26
2 parents b5920ac + 494bb5a
commit ff4fc26
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 58 deletions.
diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes
@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu Oct 26 16:07:02 UTC 2023 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- Properly track the status changes. It prevents of getting stuck
+  in the first page when there are multiple products
+  (gh#openSUSE/agama#821).
+
 -------------------------------------------------------------------
 Thu Oct 26 05:58:15 UTC 2023 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
 

diff --git a/web/src/App.jsx b/web/src/App.jsx
@@ -34,7 +34,6 @@ import {
   Disclosure,
   Installation,
   IssuesLink,
-  LoadingEnvironment,
   LogsButton,
   ShowLogButton,
   ShowTerminalButton,
@@ -63,6 +62,18 @@ function App() {
   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();
@@ -71,22 +82,18 @@ function App() {
       setStatus(status);
     };
 
-    if (client) loadPhase().catch(console.error);
-  }, [client, setPhase, setStatus]);
-
-  useEffect(() => {
     if (client) {
-      return client.manager.onPhaseChange(setPhase);
+      loadPhase().catch(console.error);
     }
-  }, [client, setPhase]);
+  }, [client, setPhase, setStatus]);
 
   const Content = () => {
     if (!client || !products) {
       return (attempt > ATTEMPTS) ? <DBusError /> : <Loading />;
     }
 
     if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) {
-      return <LoadingEnvironment onStatusChange={setStatus} />;
+      return <Loading />;
     }
 
     if (phase === INSTALL) {

diff --git a/web/src/App.test.jsx b/web/src/App.test.jsx
@@ -44,7 +44,6 @@ jest.mock("~/context/software", () => ({
 // Mock some components,
 // See https://www.chakshunyu.com/blog/how-to-mock-a-react-component-in-jest/#default-export
 jest.mock("~/components/core/DBusError", () => <div>D-BusError Mock</div>);
-jest.mock("~/components/core/LoadingEnvironment", () => () => <div>LoadingEnvironment Mock</div>);
 jest.mock("~/components/questions/Questions", () => () => <div>Questions Mock</div>);
 jest.mock("~/components/core/Installation", () => () => <div>Installation Mock</div>);
 jest.mock("~/components/core/Sidebar", () => () => <div>Sidebar Mock</div>);
@@ -56,6 +55,7 @@ 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", () => {
@@ -68,6 +68,7 @@ describe("App", () => {
           getStatus: getStatusFn,
           getPhase: getPhaseFn,
           onPhaseChange: onPhaseChangeFn,
+          onStatusChange: onStatusChangeFn,
         },
         language: {
           getUILanguage: jest.fn().mockResolvedValue("en-us"),
@@ -92,7 +93,7 @@ describe("App", () => {
       mockProducts = undefined;
     });
 
-    it("renders the LoadingEnvironment screen", async () => {
+    it("renders the Loading screen", async () => {
       installerRender(<App />, { withL10n: true });
       await screen.findByText(/Loading installation environment/);
     });
@@ -104,9 +105,9 @@ describe("App", () => {
       getStatusFn.mockResolvedValue(BUSY);
     });
 
-    it("renders the LoadingEnvironment screen", async () => {
+    it("renders the Loading screen", async () => {
       installerRender(<App />, { withL10n: true });
-      await screen.findByText("LoadingEnvironment Mock");
+      await screen.findByText(/Loading installation environment/);
     });
   });
 
@@ -116,10 +117,10 @@ describe("App", () => {
       getStatusFn.mockResolvedValue(BUSY);
     });
 
-    it("renders the LoadingEnvironment component", async () => {
+    it("renders the Loading screen", async () => {
       installerRender(<App />, { withL10n: true });
 
-      await screen.findByText("LoadingEnvironment Mock");
+      await screen.findByText(/Loading installation environment/);
     });
   });
 

diff --git a/web/src/components/core/LoadingEnvironment.jsx b/web/src/components/core/LoadingEnvironment.jsx
diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js
@@ -38,7 +38,6 @@ export { default as InstallButton } from "./InstallButton";
 export { default as IssuesLink } from "./IssuesLink";
 export { default as IssuesPage } from "./IssuesPage";
 export { default as SectionSkeleton } from "./SectionSkeleton";
-export { default as LoadingEnvironment } from "./LoadingEnvironment";
 export { default as LogsButton } from "./LogsButton";
 export { default as FileViewer } from "./FileViewer";
 export { default as RowActions } from "./RowActions";