From 32aaa54c75961882e97f64e8e043e21fa0853e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 5 May 2023 11:42:48 +0100 Subject: [PATCH 1/2] [web] Load proposal if needed --- web/src/client/storage.js | 4 +- web/src/components/storage/ProposalPage.jsx | 15 +++++- .../components/storage/ProposalPage.test.jsx | 53 +++++++++++++++++-- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 3907cd6fed..1cfc396ef9 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -114,7 +114,7 @@ class ProposalManager { * @typedef {object} ProposalData * @property {AvailableDevice[]} availableDevices * @property {Volume[]} volumeTemplates - * @property {Result} result + * @property {Result|undefined} result */ async getData() { const availableDevices = await this.getAvailableDevices(); @@ -154,7 +154,7 @@ class ProposalManager { /** * Gets the values of the current proposal * - * @return {Promise} + * @return {Promise} */ async getResult() { const proxy = await this.proposalProxy(); diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index a274fa0209..a0ae75c08a 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -27,6 +27,7 @@ import { toValidationError, useCancellablePromise } from "~/utils"; import { Icon } from "~/components/layout"; import { Page } from "~/components/core"; import { ProposalActionsSection, ProposalPageOptions, ProposalSettingsSection } from "~/components/storage"; +import { IDLE } from "~/client/status"; const initialState = { loading: true, @@ -49,7 +50,7 @@ const reducer = (state, action) => { case "UPDATE_PROPOSAL": { const { proposal, errors } = action.payload; - const { availableDevices, volumeTemplates, result } = proposal; + const { availableDevices, volumeTemplates, result = {} } = proposal; const { candidateDevices, lvm, encryptionPassword, volumes, actions } = result; return { ...state, @@ -92,7 +93,7 @@ export default function ProposalPage() { const { proposal, errors } = await loadProposal(); dispatch({ type: "UPDATE_PROPOSAL", payload: { proposal, errors } }); - dispatch({ type: "STOP_LOADING" }); + if (proposal.result !== undefined) dispatch({ type: "STOP_LOADING" }); }, [cancellablePromise, client, loadProposal]); const calculate = useCallback(async (settings) => { @@ -111,6 +112,16 @@ export default function ProposalPage() { return client.onDeprecate(() => load()); }, [client, load]); + useEffect(() => { + const statusHandler = (serviceStatus) => { + // Load the proposal if no proposal has been loaded yet. This can happen if the proposal + // page is visited before probing has finished. + if (serviceStatus === IDLE && state.settings.candidateDevices === undefined) load(); + }; + + return client.onStatusChange(statusHandler); + }); + const changeSettings = async (settings) => { const newSettings = { ...state.settings, ...settings }; diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index e8b1ea2100..7bccd931ef 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -23,6 +23,7 @@ import React from "react"; import { act, screen, waitFor } from "@testing-library/react"; import { createCallbackMock, installerRender, mockComponent } from "~/test-utils"; import { createClient } from "~/client"; +import { IDLE } from "~/client/status"; import { ProposalPage } from "~/components/storage"; jest.mock("~/client"); @@ -56,10 +57,12 @@ const isDeprecatedFn = jest.fn(); let onDeprecateFn = jest.fn(); +let onStatusChangeFn = jest.fn(); + beforeEach(() => { isDeprecatedFn.mockResolvedValue(false); - proposalData = defaultProposalData; + proposalData = { ...defaultProposalData }; createClient.mockImplementation(() => { return { @@ -71,7 +74,8 @@ beforeEach(() => { }, getErrors: jest.fn().mockResolvedValue([]), isDeprecated: isDeprecatedFn, - onDeprecate: onDeprecateFn + onDeprecate: onDeprecateFn, + onStatusChange: onStatusChangeFn } }; }); @@ -130,7 +134,7 @@ describe("when the storage devices become deprecated", () => { await screen.findByText("/dev/vda"); - proposalData.result.candidateDevices = ["/dev/vdb"]; + proposalData.result = { ...defaultProposalData.result, candidateDevices: ["/dev/vdb"] }; const [onDeprecateCb] = callbacks; await act(() => onDeprecateCb()); @@ -138,3 +142,46 @@ describe("when the storage devices become deprecated", () => { await screen.findByText("/dev/vdb"); }); }); + +describe("when there is no proposal yet", () => { + beforeEach(() => { + proposalData.result = undefined; + }); + + it("shows the page as loading", async () => { + installerRender(); + + screen.getAllByText(/PFSkeleton/); + await waitFor(() => expect(screen.queryByText(/Installation device/)).toBeNull()); + }); + + it("loads the proposal when the service finishes to calculate", async () => { + const [mockFunction, callbacks] = createCallbackMock(); + onStatusChangeFn = mockFunction; + installerRender(); + + screen.getAllByText(/PFSkeleton/); + + proposalData.result = { ...defaultProposalData.result }; + + const [onStatusChangeCb] = callbacks; + await act(() => onStatusChangeCb(IDLE)); + await screen.findByText("/dev/vda"); + }); +}); + +describe("when there is a proposal", () => { + it("does not load the proposal when the service finishes to calculate", async () => { + const [mockFunction, callbacks] = createCallbackMock(); + onStatusChangeFn = mockFunction; + installerRender(); + + await screen.findByText("/dev/vda"); + + proposalData.result.candidateDevices = ["/dev/vdb"]; + + const [onStatusChangeCb] = callbacks; + await act(() => onStatusChangeCb(IDLE)); + await screen.findByText("/dev/vda"); + }); +}); From 0e8d1de3407817b930a7ff4e4b8f19462d0334e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 5 May 2023 14:16:31 +0100 Subject: [PATCH 2/2] [web] Only subscribe when needed --- web/src/components/storage/ProposalPage.jsx | 10 +++++++--- web/src/components/storage/ProposalPage.test.jsx | 5 +---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index a0ae75c08a..44ddf89368 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -113,14 +113,18 @@ export default function ProposalPage() { }, [client, load]); useEffect(() => { + const proposalLoaded = () => state.settings.candidateDevices !== undefined; + const statusHandler = (serviceStatus) => { // Load the proposal if no proposal has been loaded yet. This can happen if the proposal // page is visited before probing has finished. - if (serviceStatus === IDLE && state.settings.candidateDevices === undefined) load(); + if (serviceStatus === IDLE && !proposalLoaded()) load(); }; - return client.onStatusChange(statusHandler); - }); + if (!proposalLoaded()) { + return client.onStatusChange(statusHandler); + } + }, [client, load, state.settings]); const changeSettings = async (settings) => { const newSettings = { ...state.settings, ...settings }; diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 7bccd931ef..b106cd7e07 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -178,10 +178,7 @@ describe("when there is a proposal", () => { await screen.findByText("/dev/vda"); - proposalData.result.candidateDevices = ["/dev/vdb"]; - const [onStatusChangeCb] = callbacks; - await act(() => onStatusChangeCb(IDLE)); - await screen.findByText("/dev/vda"); + expect(onStatusChangeCb).toBeUndefined(); }); });