From 58b47cdfc65b89268b6801e808cf67add0a9f947 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Thu, 15 Jun 2023 15:20:36 +0200 Subject: [PATCH 1/6] webui: start using useReducer for managing complicated state objects centrally useReducer is a hook that allows to extract state logic outside of the components into a state store. [1] [2] Anaconda is consuming quite complicated objects from the anaconda dbus APIs. The fetched API data will soon need to be reused across components while the app grows and get's more complex. Extracting part of the state outside of the components makes API data manipulation and fetching a bit cleaner and easier to reuse. [1] https://react.dev/reference/react/useReducer [2] https://react.dev/learn/extracting-state-logic-into-a-reducer --- ui/webui/src/actions.js | 83 +++++++++++++++++++ ui/webui/src/components/AnacondaWizard.jsx | 9 +- ui/webui/src/components/app.jsx | 5 ++ .../components/review/ReviewConfiguration.jsx | 10 +-- .../storage/InstallationDestination.jsx | 78 +++++------------ ui/webui/src/reducer.js | 53 ++++++++++++ 6 files changed, 170 insertions(+), 68 deletions(-) create mode 100644 ui/webui/src/actions.js create mode 100644 ui/webui/src/reducer.js diff --git a/ui/webui/src/actions.js b/ui/webui/src/actions.js new file mode 100644 index 00000000000..73384d2e509 --- /dev/null +++ b/ui/webui/src/actions.js @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with This program; If not, see . + */ +import cockpit from "cockpit"; + +import { + getAllDiskSelection, + getDeviceData, + getDevices, + getDiskFreeSpace, + getDiskTotalSpace, + getFormatData, + getUsableDisks, +} from "./apis/storage.js"; + +export const getDevicesAction = () => { + return async function fetchUserThunk (dispatch) { + const devices = await getDevices(); + return devices[0].map(device => dispatch(getDeviceDataAction({ device }))); + }; +}; + +export const getDeviceDataAction = ({ device }) => { + return async function fetchUserThunk (dispatch) { + let devData = {}; + const deviceData = await getDeviceData({ disk: device }) + .then(res => { + devData = res[0]; + return getDiskFreeSpace({ diskNames: [device] }); + }) + .then(free => { + // Since the getDeviceData returns an object with variants as values, + // extend it with variants to keep the format consistent + devData.free = cockpit.variant(String, free[0]); + return getDiskTotalSpace({ diskNames: [device] }); + }) + .then(total => { + devData.total = cockpit.variant(String, total[0]); + return getFormatData({ diskName: device }); + }) + .then(formatData => { + devData.formatData = formatData[0]; + return ({ [device]: devData }); + }) + .catch(console.error); + + return dispatch({ + type: "GET_DEVICE_DATA", + payload: { deviceData } + }); + }; +}; + +export const getDiskSelectionAction = () => { + return async function fetchUserThunk (dispatch) { + const usableDisks = await getUsableDisks(); + const diskSelection = await getAllDiskSelection(); + + return dispatch({ + type: "GET_DISK_SELECTION", + payload: { + diskSelection: { + ignoredDisks: diskSelection[0].IgnoredDisks.v, + selectedDisks: diskSelection[0].SelectedDisks.v, + usableDisks: usableDisks[0], + } + }, + }); + }; +}; diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index 288e2abf714..d877a5fcedf 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -40,11 +40,13 @@ import { InstallationProgress } from "./installation/InstallationProgress.jsx"; import { ReviewConfiguration, ReviewConfigurationConfirmModal } from "./review/ReviewConfiguration.jsx"; import { exitGui } from "../helpers/exit.js"; import { usePageLocation } from "hooks"; -import { resetPartitioning } from "../apis/storage.js"; +import { + resetPartitioning +} from "../apis/storage.js"; const _ = cockpit.gettext; -export const AnacondaWizard = ({ onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { +export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { const [isFormValid, setIsFormValid] = useState(true); const [stepNotification, setStepNotification] = useState(); const [isInProgress, setIsInProgress] = useState(false); @@ -64,6 +66,7 @@ export const AnacondaWizard = ({ onAddErrorNotification, toggleContextHelp, hide label: _("Installation destination"), steps: [{ component: InstallationDestination, + data: { deviceData, diskSelection, dispatch }, id: "storage-devices", label: _("Storage devices") }, { @@ -78,6 +81,7 @@ export const AnacondaWizard = ({ onAddErrorNotification, toggleContextHelp, hide }, { component: ReviewConfiguration, + data: { deviceData }, id: "installation-review", label: _("Review and install"), }, @@ -144,6 +148,7 @@ export const AnacondaWizard = ({ onAddErrorNotification, toggleContextHelp, hide window.sessionStorage.setItem("storage-scenario-id", scenarioId); setStorageScenarioId(scenarioId); }} + {...s.data} /> ), }); diff --git a/ui/webui/src/components/app.jsx b/ui/webui/src/components/app.jsx index 2f5f841511a..997518b038f 100644 --- a/ui/webui/src/components/app.jsx +++ b/ui/webui/src/components/app.jsx @@ -37,6 +37,7 @@ import { PayloadsClient } from "../apis/payloads"; import { readBuildstamp, getIsFinal } from "../helpers/betanag.js"; import { readConf } from "../helpers/conf.js"; +import { useReducerWithThunk, reducer, initialState } from "../reducer.js"; export const Application = () => { const [address, setAddress] = useState(); @@ -47,6 +48,7 @@ export const Application = () => { const [isHelpExpanded, setIsHelpExpanded] = useState(false); const [helpContent, setHelpContent] = useState(""); const [prettyName, setPrettyName] = useState(""); + const [state, dispatch] = useReducerWithThunk(reducer, initialState); useEffect(() => { cockpit.file("/run/anaconda/bus.address").watch(address => { @@ -139,6 +141,9 @@ export const Application = () => { toggleContextHelp={toggleContextHelp} hideContextHelp={() => setIsHelpExpanded(false)} title={title} + deviceData={state.devices} + diskSelection={state.diskSelection} + dispatch={dispatch} conf={conf} /> diff --git a/ui/webui/src/components/review/ReviewConfiguration.jsx b/ui/webui/src/components/review/ReviewConfiguration.jsx index 012da15209b..a33fd635aca 100644 --- a/ui/webui/src/components/review/ReviewConfiguration.jsx +++ b/ui/webui/src/components/review/ReviewConfiguration.jsx @@ -36,7 +36,6 @@ import { import { getSelectedDisks, - getDeviceData, getAppliedPartitioning, getPartitioningRequest, } from "../../apis/storage.js"; @@ -100,8 +99,7 @@ const DeviceRow = ({ name, data }) => { ); }; -export const ReviewConfiguration = ({ idPrefix, storageScenarioId }) => { - const [deviceData, setDeviceData] = useState({}); +export const ReviewConfiguration = ({ deviceData, idPrefix, storageScenarioId }) => { const [selectedDisks, setSelectedDisks] = useState(); const [systemLanguage, setSystemLanguage] = useState(); const [encrypt, setEncrypt] = useState(); @@ -117,10 +115,6 @@ export const ReviewConfiguration = ({ idPrefix, storageScenarioId }) => { const initializeDisks = async () => { const selDisks = await getSelectedDisks().catch(console.error); setSelectedDisks(selDisks); - for (const disk of selDisks) { - const devData = await getDeviceData({ disk }).catch(console.error); - setDeviceData(d => ({ ...d, [disk]: devData[0] })); - } }; const initializeEncrypt = async () => { const partitioning = await getAppliedPartitioning().catch(console.error); @@ -191,7 +185,7 @@ export const ReviewConfiguration = ({ idPrefix, storageScenarioId }) => { {_("Storage devices and configurations")} - {Object.keys(deviceData).map(deviceName => + {selectedDisks.map(deviceName => )} diff --git a/ui/webui/src/components/storage/InstallationDestination.jsx b/ui/webui/src/components/storage/InstallationDestination.jsx index d1c8dda0679..094a9432e12 100644 --- a/ui/webui/src/components/storage/InstallationDestination.jsx +++ b/ui/webui/src/components/storage/InstallationDestination.jsx @@ -53,14 +53,7 @@ import { helpStorageOptions } from "./HelpStorageOptions.jsx"; import { applyPartitioning, createPartitioning, - getAllDiskSelection, - getDevices, - getDeviceData, - getDiskFreeSpace, - getDiskTotalSpace, - getFormatData, getRequiredDeviceSize, - getUsableDisks, partitioningConfigureWithTask, resetPartitioning, runStorageTask, @@ -75,6 +68,7 @@ import { import { getRequiredSpace, } from "../../apis/payloads"; +import { getDevicesAction, getDiskSelectionAction } from "../../actions.js"; import { sleep, @@ -195,8 +189,7 @@ const DropdownBulkSelect = ({ ); }; -const LocalStandardDisks = ({ idPrefix, setIsFormValid, onAddErrorNotification }) => { - const [deviceData, setDeviceData] = useState({}); +const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, setIsFormValid, onAddErrorNotification }) => { const [disks, setDisks] = useState({}); const [refreshCnt, setRefreshCnt] = useState(0); const [isRescanningDisks, setIsRescanningDisks] = useState(false); @@ -204,53 +197,19 @@ const LocalStandardDisks = ({ idPrefix, setIsFormValid, onAddErrorNotification } const [equalDisksNotify, setEqualDisksNotify] = useState(false); useEffect(() => { - let usableDisks; - let devices; - getDevices() - .then(ret => { - devices = ret[0]; - return getUsableDisks(); - }) - .then(res => { - usableDisks = res[0]; - return getAllDiskSelection(); - }) - .then(props => { - // Select default disks for the partitioning - const defaultDisks = selectDefaultDisks({ - ignoredDisks: props[0].IgnoredDisks.v, - selectedDisks: props[0].SelectedDisks.v, - usableDisks, - }); - setDisks(usableDisks.reduce((acc, cur) => ({ ...acc, [cur]: defaultDisks.includes(cur) }), {})); - - // Show disks data - devices.forEach(disk => { - let deviceData = {}; - const diskNames = [disk]; - - getDeviceData({ disk }) - .then(res => { - deviceData = res[0]; - return getDiskFreeSpace({ diskNames }); - }, console.error) - .then(free => { - // Since the getDeviceData returns an object with variants as values, - // extend it with variants to keep the format consistent - deviceData.free = cockpit.variant(String, free[0]); - return getDiskTotalSpace({ diskNames }); - }, console.error) - .then(total => { - deviceData.total = cockpit.variant(String, total[0]); - return getFormatData({ diskName: disk }); - }, console.error) - .then(formatData => { - deviceData.formatData = formatData[0]; - setDeviceData(d => ({ ...d, [disk]: deviceData })); - }, console.error); - }); - }, console.error); - }, [refreshCnt]); + // Select default disks for the partitioning + const defaultDisks = selectDefaultDisks({ + ignoredDisks: diskSelection.ignoredDisks, + selectedDisks: diskSelection.selectedDisks, + usableDisks: diskSelection.usableDisks, + }); + setDisks(diskSelection.usableDisks.reduce((acc, cur) => ({ ...acc, [cur]: defaultDisks.includes(cur) }), {})); + }, [refreshCnt, dispatch, diskSelection]); + + useEffect(() => { + dispatch(getDevicesAction()); + dispatch(getDiskSelectionAction()); + }, [dispatch, refreshCnt]); const totalDisksCnt = Object.keys(disks).length; const selectedDisksCnt = Object.keys(disks).filter(disk => !!disks[disk]).length; @@ -269,7 +228,7 @@ const LocalStandardDisks = ({ idPrefix, setIsFormValid, onAddErrorNotification } setSelectedDisks({ drives: selected }).catch(onAddErrorNotification); }, [disks, onAddErrorNotification, selectedDisksCnt, setIsFormValid]); - const loading = Object.keys(disks).some(disk => !deviceData[disk]); + const loading = !deviceData || Object.keys(disks).some(disk => !deviceData[disk]); if (loading) { return ; } @@ -478,7 +437,7 @@ const LocalStandardDisks = ({ idPrefix, setIsFormValid, onAddErrorNotification } ); }; -export const InstallationDestination = ({ idPrefix, setIsFormValid, onAddErrorNotification, toggleContextHelp, stepNotification, isInProgress }) => { +export const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix, setIsFormValid, onAddErrorNotification, toggleContextHelp, stepNotification, isInProgress }) => { const [requiredSize, setRequiredSize] = useState(0); const toggleHelpStorageOptions = () => { @@ -516,6 +475,9 @@ export const InstallationDestination = ({ idPrefix, setIsFormValid, onAddErrorNo variant="danger" />} . + */ + +import { useReducer, useCallback } from "react"; + +export const useReducerWithThunk = (reducer, initialState) => { + const [state, dispatch] = useReducer(reducer, initialState); + + function customDispatch (action) { + if (typeof action === "function") { + return action(customDispatch); + } else { + dispatch(action); + } + } + + // Memoize so you can include it in the dependency array without causing infinite loops + // eslint-disable-next-line react-hooks/exhaustive-deps + const stableDispatch = useCallback(customDispatch, [dispatch]); + + return [state, stableDispatch]; +}; + +export const reducer = (state, action) => { + if (action.type === "GET_DEVICE_DATA") { + return { ...state, devices: { ...action.payload.deviceData, ...state.devices } }; + } else if (action.type === "GET_DISK_SELECTION") { + return { ...state, diskSelection: action.payload.diskSelection }; + } +}; + +export const initialState = { + devices: {}, + diskSelection: { + usableDisks: [], + selectedDisks: [], + ignoredDisks: [] + } +}; From 394cccf2f010eb1c28e8589b40a733a7f1cd60c7 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Wed, 21 Jun 2023 12:26:04 +0200 Subject: [PATCH 2/6] webui: use useReducer also for language state collection --- ui/webui/src/actions.js | 40 ++++++++++ ui/webui/src/components/AnacondaWizard.jsx | 3 +- ui/webui/src/components/app.jsx | 2 + .../localization/InstallationLanguage.jsx | 74 ++++++++----------- ui/webui/src/reducer.js | 13 +++- 5 files changed, 85 insertions(+), 47 deletions(-) diff --git a/ui/webui/src/actions.js b/ui/webui/src/actions.js index 73384d2e509..38559087037 100644 --- a/ui/webui/src/actions.js +++ b/ui/webui/src/actions.js @@ -25,6 +25,13 @@ import { getFormatData, getUsableDisks, } from "./apis/storage.js"; +import { + getCommonLocales, + getLanguages, + getLanguageData, + getLocales, + getLocaleData, +} from "./apis/localization.js"; export const getDevicesAction = () => { return async function fetchUserThunk (dispatch) { @@ -81,3 +88,36 @@ export const getDiskSelectionAction = () => { }); }; }; + +export const getLanguagesAction = () => { + return async function fetchUserThunk (dispatch) { + const languageIds = await getLanguages(); + + dispatch(getCommonLocalesAction()); + return languageIds.map(language => dispatch(getLanguageDataAction({ language }))); + }; +}; + +export const getLanguageDataAction = ({ language }) => { + return async function fetchUserThunk (dispatch) { + const localeIds = await getLocales({ lang: language }); + const languageData = await getLanguageData({ lang: language }); + const locales = await Promise.all(localeIds.map(async locale => await getLocaleData({ locale }))); + + return dispatch({ + type: "GET_LANGUAGE_DATA", + payload: { languageData: { [language]: { languageData, locales } } } + }); + }; +}; + +export const getCommonLocalesAction = () => { + return async function fetchUserThunk (dispatch) { + const commonLocales = await getCommonLocales(); + + return dispatch({ + type: "GET_COMMON_LOCALES", + payload: { commonLocales } + }); + }; +}; diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index d877a5fcedf..3bd3dc5a515 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -46,7 +46,7 @@ import { const _ = cockpit.gettext; -export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { +export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, languages, commonLocales, onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { const [isFormValid, setIsFormValid] = useState(true); const [stepNotification, setStepNotification] = useState(); const [isInProgress, setIsInProgress] = useState(false); @@ -57,6 +57,7 @@ export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, onAddError const stepsOrder = [ { component: InstallationLanguage, + data: { dispatch, languages, commonLocales }, id: "installation-language", label: _("Welcome"), }, diff --git a/ui/webui/src/components/app.jsx b/ui/webui/src/components/app.jsx index 997518b038f..5822900689b 100644 --- a/ui/webui/src/components/app.jsx +++ b/ui/webui/src/components/app.jsx @@ -142,6 +142,8 @@ export const Application = () => { hideContextHelp={() => setIsHelpExpanded(false)} title={title} deviceData={state.devices} + languages={state.languages} + commonLocales={state.commonLocales} diskSelection={state.diskSelection} dispatch={dispatch} conf={conf} diff --git a/ui/webui/src/components/localization/InstallationLanguage.jsx b/ui/webui/src/components/localization/InstallationLanguage.jsx index 149ea5456cf..002622980e4 100644 --- a/ui/webui/src/components/localization/InstallationLanguage.jsx +++ b/ui/webui/src/components/localization/InstallationLanguage.jsx @@ -40,12 +40,7 @@ import { setLocale } from "../../apis/boss.js"; import { getLanguage, - getLanguages, - getLanguageData, - getLocales, - getLocaleData, setLanguage, - getCommonLocales, } from "../../apis/localization.js"; import { @@ -54,6 +49,7 @@ import { setLangCookie } from "../../helpers/language.js"; import { AnacondaPage } from "../AnacondaPage.jsx"; +import { getLanguagesAction } from "../../actions.js"; import "./InstallationLanguage.scss"; @@ -69,9 +65,6 @@ class LanguageSelector extends React.Component { constructor (props) { super(props); this.state = { - languages: [], - locales: [], - commonLocales: [], search: "", lang: "", }; @@ -93,28 +86,14 @@ class LanguageSelector extends React.Component { } catch (e) { this.props.onAddErrorNotification(e); } - - const languageIds = await getLanguages(); - // Create the languages state object - this.setState({ languages: await Promise.all(languageIds.map(async lang => await getLanguageData({ lang }))) }); - // Create the locales state object - const localeIds = await Promise.all(languageIds.map(async lang => await getLocales({ lang }))); - const locales = await Promise.all(localeIds.map(async localeId => { - return await Promise.all(localeId.map(async locale => await getLocaleData({ locale }))); - })); - this.setState({ locales }, this.updateNativeName); - - // Create a list of common locales. - this.setState({ commonLocales: await getCommonLocales() }); } - async updateNativeName (localeData) { - localeData = localeData || await getLocaleData({ locale: this.state.lang }); - this.props.getNativeName(getLocaleNativeName(localeData)); + async updateNativeName (localeItem) { + this.props.setNativeName(getLocaleNativeName(localeItem)); } renderOptions (filter) { - const { languages, locales } = this.state; + const { languages } = this.props; const idPrefix = this.props.idPrefix; const filterLow = filter.toLowerCase(); @@ -124,10 +103,11 @@ class LanguageSelector extends React.Component { let foundSelected = false; // Returns a locale with a given code. const findLocaleWithId = (localeCode) => { - for (const locale of this.state.locales) { - for (const subLocale of locale) { - if (getLocaleId(subLocale) === localeCode) { - return subLocale; + for (const languageId in languages) { + const languageItem = languages[languageId]; + for (const locale of languageItem.locales) { + if (getLocaleId(locale) === localeCode) { + return locale; } } } @@ -180,7 +160,7 @@ class LanguageSelector extends React.Component { key="group-common-languages" > { - this.state.commonLocales + this.props.commonLocales .map(findLocaleWithId) .filter(locale => locale) .map(locale => createMenuItem(locale, "option-common-")) @@ -192,20 +172,20 @@ class LanguageSelector extends React.Component { } // List alphabetically. - for (const langLocales of locales) { - const currentLang = languages.find(lang => getLanguageId(lang) === getLanguageId(langLocales[0])); - - const label = cockpit.format("$0 ($1)", getLanguageNativeName(currentLang), getLanguageEnglishName(currentLang)); + const languagesIds = Object.keys(languages).sort(); + for (const languageId of languagesIds) { + const languageItem = languages[languageId]; + const label = cockpit.format("$0 ($1)", getLanguageNativeName(languageItem.languageData), getLanguageEnglishName(languageItem.languageData)); if (!filter || label.toLowerCase().indexOf(filterLow) !== -1) { filtered.push( - {langLocales.map(locale => createMenuItem(locale, "option-alpha-"))} + {languageItem.locales.map(locale => createMenuItem(locale, "option-alpha-"))} ); } @@ -227,11 +207,12 @@ class LanguageSelector extends React.Component { } render () { - const { languages, locales, lang, commonLocales } = this.state; + const { lang } = this.state; + const { languages, commonLocales } = this.props; + const handleOnSelect = (_event, item) => { - const { locales } = this.state; - for (const locale of locales) { - for (const localeItem of locale) { + for (const languageItem in languages) { + for (const localeItem of languages[languageItem].locales) { if (getLocaleId(localeItem) === item) { setLangCookie({ cockpitLang: convertToCockpitLang({ lang: getLocaleId(localeItem) }) }); setLanguage({ lang: getLocaleId(localeItem) }) @@ -261,7 +242,7 @@ class LanguageSelector extends React.Component { } }; - const isLoading = languages.length === 0 || languages.length !== locales.length || commonLocales.length === 0; + const isLoading = languages.length === 0 || commonLocales.length === 0; if (isLoading) { return ; @@ -315,14 +296,15 @@ class LanguageSelector extends React.Component { } LanguageSelector.contextType = AddressContext; -export const InstallationLanguage = ({ idPrefix, setIsFormValid, onAddErrorNotification }) => { +export const InstallationLanguage = ({ idPrefix, languages, commonLocales, dispatch, setIsFormValid, onAddErrorNotification }) => { const [nativeName, setNativeName] = React.useState(false); const { setLanguage } = React.useContext(LanguageContext); const [distributionName, setDistributionName] = useState(""); useEffect(() => { readOsRelease().then(osRelease => setDistributionName(osRelease.NAME)); - }, []); + dispatch(getLanguagesAction()); + }, [dispatch]); return ( @@ -347,9 +329,11 @@ export const InstallationLanguage = ({ idPrefix, setIsFormValid, onAddErrorNotif diff --git a/ui/webui/src/reducer.js b/ui/webui/src/reducer.js index d54b9d8ba81..23007a49da6 100644 --- a/ui/webui/src/reducer.js +++ b/ui/webui/src/reducer.js @@ -41,6 +41,14 @@ export const reducer = (state, action) => { } else if (action.type === "GET_DISK_SELECTION") { return { ...state, diskSelection: action.payload.diskSelection }; } + + if (action.type === "GET_LANGUAGE_DATA") { + return { ...state, languages: { ...action.payload.languageData, ...state.languages } }; + } + + if (action.type === "GET_COMMON_LOCALES") { + return { ...state, commonLocales: action.payload.commonLocales }; + } }; export const initialState = { @@ -49,5 +57,8 @@ export const initialState = { usableDisks: [], selectedDisks: [], ignoredDisks: [] - } + }, + + languages: {}, + commonLocales: [] }; From a1ae1868be8413c5b135e43d0c39bc5e1a3a25e9 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Wed, 21 Jun 2023 14:07:28 +0200 Subject: [PATCH 3/6] webui: split reducers acording to the data type --- ui/webui/src/components/AnacondaWizard.jsx | 8 ++-- ui/webui/src/components/app.jsx | 6 +-- ui/webui/src/reducer.js | 52 +++++++++++++++------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index 3bd3dc5a515..e4f5e3d92df 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -46,7 +46,7 @@ import { const _ = cockpit.gettext; -export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, languages, commonLocales, onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { +export const AnacondaWizard = ({ dispatch, storageData, localizationData, onAddErrorNotification, toggleContextHelp, hideContextHelp, title, conf }) => { const [isFormValid, setIsFormValid] = useState(true); const [stepNotification, setStepNotification] = useState(); const [isInProgress, setIsInProgress] = useState(false); @@ -57,7 +57,7 @@ export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, languages, const stepsOrder = [ { component: InstallationLanguage, - data: { dispatch, languages, commonLocales }, + data: { dispatch, languages: localizationData.languages, commonLocales: localizationData.commonLocales }, id: "installation-language", label: _("Welcome"), }, @@ -67,7 +67,7 @@ export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, languages, label: _("Installation destination"), steps: [{ component: InstallationDestination, - data: { deviceData, diskSelection, dispatch }, + data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection, dispatch }, id: "storage-devices", label: _("Storage devices") }, { @@ -82,7 +82,7 @@ export const AnacondaWizard = ({ dispatch, deviceData, diskSelection, languages, }, { component: ReviewConfiguration, - data: { deviceData }, + data: { deviceData: storageData.devices }, id: "installation-review", label: _("Review and install"), }, diff --git a/ui/webui/src/components/app.jsx b/ui/webui/src/components/app.jsx index 5822900689b..6febb01280b 100644 --- a/ui/webui/src/components/app.jsx +++ b/ui/webui/src/components/app.jsx @@ -141,10 +141,8 @@ export const Application = () => { toggleContextHelp={toggleContextHelp} hideContextHelp={() => setIsHelpExpanded(false)} title={title} - deviceData={state.devices} - languages={state.languages} - commonLocales={state.commonLocales} - diskSelection={state.diskSelection} + storageData={state.storage} + localizationData={state.localization} dispatch={dispatch} conf={conf} /> diff --git a/ui/webui/src/reducer.js b/ui/webui/src/reducer.js index 23007a49da6..f7cb9672030 100644 --- a/ui/webui/src/reducer.js +++ b/ui/webui/src/reducer.js @@ -17,6 +17,29 @@ import { useReducer, useCallback } from "react"; +/* Initial state for the storeage store substate */ +export const storageInitialState = { + devices: {}, + diskSelection: { + usableDisks: [], + selectedDisks: [], + ignoredDisks: [] + }, +}; + +/* Initial state for the localization store substate */ +export const localizationInitialState = { + languages: {}, + commonLocales: [] +}; + +/* Initial state for the global store */ +export const initialState = { + localization: localizationInitialState, + storage: storageInitialState, +}; + +/* Custom hook to use the reducer with async actions */ export const useReducerWithThunk = (reducer, initialState) => { const [state, dispatch] = useReducer(reducer, initialState); @@ -36,29 +59,28 @@ export const useReducerWithThunk = (reducer, initialState) => { }; export const reducer = (state, action) => { + return ({ + localization: localizationReducer(state.localization, action), + storage: storageReducer(state.storage, action), + }); +}; + +export const storageReducer = (state = storageInitialState, action) => { if (action.type === "GET_DEVICE_DATA") { return { ...state, devices: { ...action.payload.deviceData, ...state.devices } }; } else if (action.type === "GET_DISK_SELECTION") { return { ...state, diskSelection: action.payload.diskSelection }; + } else { + return state; } +}; +export const localizationReducer = (state = localizationInitialState, action) => { if (action.type === "GET_LANGUAGE_DATA") { return { ...state, languages: { ...action.payload.languageData, ...state.languages } }; - } - - if (action.type === "GET_COMMON_LOCALES") { + } else if (action.type === "GET_COMMON_LOCALES") { return { ...state, commonLocales: action.payload.commonLocales }; + } else { + return state; } }; - -export const initialState = { - devices: {}, - diskSelection: { - usableDisks: [], - selectedDisks: [], - ignoredDisks: [] - }, - - languages: {}, - commonLocales: [] -}; From 115dbc53189834299ec842abdd24876417f1ccfd Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Wed, 21 Jun 2023 14:20:40 +0200 Subject: [PATCH 4/6] webui: split actions per data type --- ui/webui/src/actions/localization-actions.js | 57 +++++++++++++++++++ .../storage-actions.js} | 43 +------------- .../localization/InstallationLanguage.jsx | 2 +- .../storage/InstallationDestination.jsx | 2 +- 4 files changed, 61 insertions(+), 43 deletions(-) create mode 100644 ui/webui/src/actions/localization-actions.js rename ui/webui/src/{actions.js => actions/storage-actions.js} (69%) diff --git a/ui/webui/src/actions/localization-actions.js b/ui/webui/src/actions/localization-actions.js new file mode 100644 index 00000000000..a7ec46e36ba --- /dev/null +++ b/ui/webui/src/actions/localization-actions.js @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with This program; If not, see . + */ + +import { + getCommonLocales, + getLanguages, + getLanguageData, + getLocales, + getLocaleData, +} from "../apis/localization.js"; + +export const getLanguagesAction = () => { + return async function fetchUserThunk (dispatch) { + const languageIds = await getLanguages(); + + dispatch(getCommonLocalesAction()); + return languageIds.map(language => dispatch(getLanguageDataAction({ language }))); + }; +}; + +export const getLanguageDataAction = ({ language }) => { + return async function fetchUserThunk (dispatch) { + const localeIds = await getLocales({ lang: language }); + const languageData = await getLanguageData({ lang: language }); + const locales = await Promise.all(localeIds.map(async locale => await getLocaleData({ locale }))); + + return dispatch({ + type: "GET_LANGUAGE_DATA", + payload: { languageData: { [language]: { languageData, locales } } } + }); + }; +}; + +export const getCommonLocalesAction = () => { + return async function fetchUserThunk (dispatch) { + const commonLocales = await getCommonLocales(); + + return dispatch({ + type: "GET_COMMON_LOCALES", + payload: { commonLocales } + }); + }; +}; diff --git a/ui/webui/src/actions.js b/ui/webui/src/actions/storage-actions.js similarity index 69% rename from ui/webui/src/actions.js rename to ui/webui/src/actions/storage-actions.js index 38559087037..df5b9cd7b21 100644 --- a/ui/webui/src/actions.js +++ b/ui/webui/src/actions/storage-actions.js @@ -14,6 +14,7 @@ * You should have received a copy of the GNU Lesser General Public License * along with This program; If not, see . */ + import cockpit from "cockpit"; import { @@ -24,14 +25,7 @@ import { getDiskTotalSpace, getFormatData, getUsableDisks, -} from "./apis/storage.js"; -import { - getCommonLocales, - getLanguages, - getLanguageData, - getLocales, - getLocaleData, -} from "./apis/localization.js"; +} from "../apis/storage.js"; export const getDevicesAction = () => { return async function fetchUserThunk (dispatch) { @@ -88,36 +82,3 @@ export const getDiskSelectionAction = () => { }); }; }; - -export const getLanguagesAction = () => { - return async function fetchUserThunk (dispatch) { - const languageIds = await getLanguages(); - - dispatch(getCommonLocalesAction()); - return languageIds.map(language => dispatch(getLanguageDataAction({ language }))); - }; -}; - -export const getLanguageDataAction = ({ language }) => { - return async function fetchUserThunk (dispatch) { - const localeIds = await getLocales({ lang: language }); - const languageData = await getLanguageData({ lang: language }); - const locales = await Promise.all(localeIds.map(async locale => await getLocaleData({ locale }))); - - return dispatch({ - type: "GET_LANGUAGE_DATA", - payload: { languageData: { [language]: { languageData, locales } } } - }); - }; -}; - -export const getCommonLocalesAction = () => { - return async function fetchUserThunk (dispatch) { - const commonLocales = await getCommonLocales(); - - return dispatch({ - type: "GET_COMMON_LOCALES", - payload: { commonLocales } - }); - }; -}; diff --git a/ui/webui/src/components/localization/InstallationLanguage.jsx b/ui/webui/src/components/localization/InstallationLanguage.jsx index 002622980e4..cee2d4518d3 100644 --- a/ui/webui/src/components/localization/InstallationLanguage.jsx +++ b/ui/webui/src/components/localization/InstallationLanguage.jsx @@ -49,7 +49,7 @@ import { setLangCookie } from "../../helpers/language.js"; import { AnacondaPage } from "../AnacondaPage.jsx"; -import { getLanguagesAction } from "../../actions.js"; +import { getLanguagesAction } from "../../actions/localization-actions.js"; import "./InstallationLanguage.scss"; diff --git a/ui/webui/src/components/storage/InstallationDestination.jsx b/ui/webui/src/components/storage/InstallationDestination.jsx index 094a9432e12..540e83020bc 100644 --- a/ui/webui/src/components/storage/InstallationDestination.jsx +++ b/ui/webui/src/components/storage/InstallationDestination.jsx @@ -68,7 +68,7 @@ import { import { getRequiredSpace, } from "../../apis/payloads"; -import { getDevicesAction, getDiskSelectionAction } from "../../actions.js"; +import { getDevicesAction, getDiskSelectionAction } from "../../actions/storage-actions.js"; import { sleep, From bbea776d0140c54fb67fde5be0f4849bfa070268 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Thu, 22 Jun 2023 22:58:44 +0200 Subject: [PATCH 5/6] webui: use the store as single source of truth for disk selection Listen to signals for when a property changes in the backend for DiskSelection, and only use the backend values for updating the UI. Additionally when re-scanning disks also reset disk selection to 'None selected', at least for the time being as pre-selecting disks after re-scan is not trivial. --- ui/webui/src/apis/storage.js | 28 +++++- ui/webui/src/components/AnacondaWizard.jsx | 2 +- ui/webui/src/components/app.jsx | 6 +- .../components/review/ReviewConfiguration.jsx | 13 +-- .../storage/InstallationDestination.jsx | 96 ++++++++++++------- ui/webui/test/check-storage | 2 +- ui/webui/test/reference | 2 +- 7 files changed, 96 insertions(+), 53 deletions(-) diff --git a/ui/webui/src/apis/storage.js b/ui/webui/src/apis/storage.js index e5d4a86bcdd..29adafd04c9 100644 --- a/ui/webui/src/apis/storage.js +++ b/ui/webui/src/apis/storage.js @@ -16,6 +16,8 @@ */ import cockpit from "cockpit"; +import { getDiskSelectionAction } from "../actions/storage-actions.js"; + export class StorageClient { constructor (address) { if (StorageClient.instance) { @@ -284,13 +286,21 @@ export const resetPartitioning = () => { * @returns {Promise} Resolves a DBus path to a task */ export const runStorageTask = ({ task, onSuccess, onFail }) => { + // FIXME: This is a workaround for 'Succeeded' signal being emited twice + let succeededEmitted = false; const taskProxy = new StorageClient().client.proxy( "org.fedoraproject.Anaconda.Task", task ); const addEventListeners = () => { taskProxy.addEventListener("Stopped", () => taskProxy.Finish().catch(onFail)); - taskProxy.addEventListener("Succeeded", onSuccess); + taskProxy.addEventListener("Succeeded", () => { + if (succeededEmitted) { + return; + } + succeededEmitted = true; + onSuccess(); + }); }; taskProxy.wait(() => { addEventListeners(); @@ -390,3 +400,19 @@ export const setSelectedDisks = ({ drives }) => { ] ); }; + +export const startEventMonitorStorage = ({ dispatch }) => { + return new StorageClient().client.subscribe( + { }, + (path, iface, signal, args) => { + switch (signal) { + case "PropertiesChanged": + if (args[0] === "org.fedoraproject.Anaconda.Modules.Storage.DiskSelection") { + dispatch(getDiskSelectionAction()); + } + break; + default: + console.debug(`Unhandled signal on ${path}: ${iface}.${signal}`); + } + }); +}; diff --git a/ui/webui/src/components/AnacondaWizard.jsx b/ui/webui/src/components/AnacondaWizard.jsx index e4f5e3d92df..32c78228ae3 100644 --- a/ui/webui/src/components/AnacondaWizard.jsx +++ b/ui/webui/src/components/AnacondaWizard.jsx @@ -82,7 +82,7 @@ export const AnacondaWizard = ({ dispatch, storageData, localizationData, onAddE }, { component: ReviewConfiguration, - data: { deviceData: storageData.devices }, + data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection }, id: "installation-review", label: _("Review and install"), }, diff --git a/ui/webui/src/components/app.jsx b/ui/webui/src/components/app.jsx index 6febb01280b..b97ba6d1314 100644 --- a/ui/webui/src/components/app.jsx +++ b/ui/webui/src/components/app.jsx @@ -32,7 +32,7 @@ import { HelpDrawer } from "./HelpDrawer.jsx"; import { BossClient } from "../apis/boss.js"; import { LocalizationClient } from "../apis/localization.js"; -import { StorageClient } from "../apis/storage.js"; +import { StorageClient, startEventMonitorStorage } from "../apis/storage.js"; import { PayloadsClient } from "../apis/payloads"; import { readBuildstamp, getIsFinal } from "../helpers/betanag.js"; @@ -61,6 +61,8 @@ export const Application = () => { clients.forEach(c => c.init()); setAddress(address); + + startEventMonitorStorage({ dispatch }); }); readConf().then( @@ -74,7 +76,7 @@ export const Application = () => { ); readOsRelease().then(osRelease => setPrettyName(osRelease.PRETTY_NAME)); - }, []); + }, [dispatch]); const onAddNotification = (notificationProps) => { setNotifications({ diff --git a/ui/webui/src/components/review/ReviewConfiguration.jsx b/ui/webui/src/components/review/ReviewConfiguration.jsx index a33fd635aca..02b1a4e5883 100644 --- a/ui/webui/src/components/review/ReviewConfiguration.jsx +++ b/ui/webui/src/components/review/ReviewConfiguration.jsx @@ -35,7 +35,6 @@ import { } from "@patternfly/react-core"; import { - getSelectedDisks, getAppliedPartitioning, getPartitioningRequest, } from "../../apis/storage.js"; @@ -99,8 +98,7 @@ const DeviceRow = ({ name, data }) => { ); }; -export const ReviewConfiguration = ({ deviceData, idPrefix, storageScenarioId }) => { - const [selectedDisks, setSelectedDisks] = useState(); +export const ReviewConfiguration = ({ deviceData, diskSelection, idPrefix, storageScenarioId }) => { const [systemLanguage, setSystemLanguage] = useState(); const [encrypt, setEncrypt] = useState(); const [showLanguageSection, setShowLanguageSection] = useState(true); @@ -112,22 +110,17 @@ export const ReviewConfiguration = ({ deviceData, idPrefix, storageScenarioId }) const langData = await getLanguageData({ lang }).catch(console.error); setSystemLanguage(langData["native-name"].v); }; - const initializeDisks = async () => { - const selDisks = await getSelectedDisks().catch(console.error); - setSelectedDisks(selDisks); - }; const initializeEncrypt = async () => { const partitioning = await getAppliedPartitioning().catch(console.error); const request = await getPartitioningRequest({ partitioning }).catch(console.error); setEncrypt(request.encrypted.v); }; initializeLanguage(); - initializeDisks(); initializeEncrypt(); }, []); // handle case of disks not (yet) loaded - if (!selectedDisks || !systemLanguage) { + if (!systemLanguage) { return null; } @@ -185,7 +178,7 @@ export const ReviewConfiguration = ({ deviceData, idPrefix, storageScenarioId }) {_("Storage devices and configurations")} - {selectedDisks.map(deviceName => + {diskSelection.selectedDisks.map(deviceName => )} diff --git a/ui/webui/src/components/storage/InstallationDestination.jsx b/ui/webui/src/components/storage/InstallationDestination.jsx index 540e83020bc..096381a9ec5 100644 --- a/ui/webui/src/components/storage/InstallationDestination.jsx +++ b/ui/webui/src/components/storage/InstallationDestination.jsx @@ -15,7 +15,7 @@ * along with This program; If not, see . */ import cockpit from "cockpit"; -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { Alert, @@ -103,14 +103,10 @@ const selectDefaultDisks = ({ ignoredDisks, selectedDisks, usableDisks }) => { } }; -const setSelectionForAllDisks = ({ disks, value }) => { - return (Object.keys(disks).reduce((acc, cur) => ({ ...acc, [cur]: value }), {})); -}; - const containEqualDisks = (disks1, disks2) => { - const disks1Str = Object.keys(disks1).sort() + const disks1Str = disks1.sort() .join(); - const disks2Str = Object.keys(disks2).sort() + const disks2Str = disks2.sort() .join(); return disks1Str === disks2Str; }; @@ -190,45 +186,53 @@ const DropdownBulkSelect = ({ }; const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, setIsFormValid, onAddErrorNotification }) => { - const [disks, setDisks] = useState({}); - const [refreshCnt, setRefreshCnt] = useState(0); const [isRescanningDisks, setIsRescanningDisks] = useState(false); - const [lastRescanDisks, setLastRescanDisks] = useState({}); const [equalDisksNotify, setEqualDisksNotify] = useState(false); + const refUsableDisks = useRef(); + + useEffect(() => { + if (isRescanningDisks) { + refUsableDisks.current = diskSelection.usableDisks; + setEqualDisksNotify(true); + } + }, [isRescanningDisks, diskSelection.usableDisks]); useEffect(() => { - // Select default disks for the partitioning + // Select default disks for the partitioning on component mount + if (refUsableDisks.current !== undefined) { + return; + } + const defaultDisks = selectDefaultDisks({ ignoredDisks: diskSelection.ignoredDisks, selectedDisks: diskSelection.selectedDisks, usableDisks: diskSelection.usableDisks, }); - setDisks(diskSelection.usableDisks.reduce((acc, cur) => ({ ...acc, [cur]: defaultDisks.includes(cur) }), {})); - }, [refreshCnt, dispatch, diskSelection]); + + if (!containEqualDisks(diskSelection.selectedDisks, defaultDisks)) { + refUsableDisks.current = diskSelection.usableDisks; + setSelectedDisks({ drives: defaultDisks }); + } + }, [diskSelection]); useEffect(() => { dispatch(getDevicesAction()); dispatch(getDiskSelectionAction()); - }, [dispatch, refreshCnt]); + }, [dispatch]); - const totalDisksCnt = Object.keys(disks).length; - const selectedDisksCnt = Object.keys(disks).filter(disk => !!disks[disk]).length; + const totalDisksCnt = diskSelection.usableDisks.length; + const selectedDisksCnt = diskSelection.selectedDisks.length; // When the selected disks change in the UI, update in the backend as well useEffect(() => { // Do not update on the inital value, wait for initialization by the other effect - if (Object.keys(disks).length === 0) { + if (refUsableDisks.current === undefined) { return; } setIsFormValid(selectedDisksCnt > 0); + }, [selectedDisksCnt, setIsFormValid]); - const selected = Object.keys(disks).filter(disk => disks[disk]); - console.log("Updating storage backend with selected disks:", selected.join(",")); - - setSelectedDisks({ drives: selected }).catch(onAddErrorNotification); - }, [disks, onAddErrorNotification, selectedDisksCnt, setIsFormValid]); - - const loading = !deviceData || Object.keys(disks).some(disk => !deviceData[disk]); + const loading = !deviceData || diskSelection.usableDisks.some(disk => !deviceData[disk]); if (loading) { return ; } @@ -256,17 +260,19 @@ const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, set variant="secondary" onClick={() => { setIsRescanningDisks(true); - setLastRescanDisks({ ...disks }); - setDisks(setSelectionForAllDisks({ disks, value: false })); + setSelectedDisks({ drives: [] }); scanDevicesWithTask() .then(res => { runStorageTask({ task: res[0], - onSuccess: () => resetPartitioning().then(() => setRefreshCnt(refreshCnt + 1), onAddErrorNotification), + onSuccess: () => resetPartitioning().then(() => { + dispatch(getDevicesAction()); + dispatch(getDiskSelectionAction()); + }, onAddErrorNotification), onFail: onAddErrorNotification }); }) - .finally(() => { setIsRescanningDisks(false); setEqualDisksNotify(true) }); + .finally(() => setIsRescanningDisks(false)); }} > ); - const localDisksRows = Object.keys(disks).map(disk => { + const localDisksRows = diskSelection.usableDisks.map(disk => { const hasPartitions = deviceData[disk]?.children?.v.length && deviceData[disk]?.children?.v.every(partition => deviceData[partition]); return ({ - selected: !!disks[disk], + selected: !!diskSelection.selectedDisks.includes(disk), hasPadding: true, props: { key: disk, id: disk }, columns: [ @@ -354,7 +360,7 @@ const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, set ] ); - const rescanningDisksRows = Object.keys(disks).map(disk => ( + const rescanningDisksRows = diskSelection.usableDisks.map(disk => ( { columns: rescanningDisksRow } @@ -362,9 +368,15 @@ const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, set const dropdownBulkSelect = ( setDisks(setSelectionForAllDisks({ disks, value: true }))} - onSelectNone={() => setDisks(setSelectionForAllDisks({ disks, value: false }))} - onChange={(checked) => setDisks(setSelectionForAllDisks({ disks, value: checked }))} + onSelectAll={() => setSelectedDisks({ drives: diskSelection.usableDisks })} + onSelectNone={() => setSelectedDisks({ drives: [] })} + onChange={(checked) => { + if (checked) { + setSelectedDisks({ drives: diskSelection.usableDisks }); + } else { + setSelectedDisks({ drives: [] }); + } + }} selectedCnt={selectedDisksCnt} totalCnt={totalDisksCnt} isDisabled={isRescanningDisks} @@ -399,7 +411,15 @@ const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, set } onSelect={ !isRescanningDisks - ? (_, isSelected, diskId) => setDisks({ ...disks, [Object.keys(disks)[diskId]]: isSelected }) + ? (_, isSelected, diskId) => { + const newDisk = diskSelection.usableDisks[diskId]; + + if (isSelected) { + setSelectedDisks({ drives: [...diskSelection.selectedDisks, newDisk] }); + } else { + setSelectedDisks({ drives: diskSelection.selectedDisks.filter(disk => disk !== newDisk) }); + } + } : () => {} } rows={ @@ -410,6 +430,8 @@ const LocalStandardDisks = ({ deviceData, diskSelection, dispatch, idPrefix, set /> ); + const equalDisks = refUsableDisks.current && containEqualDisks(refUsableDisks.current, diskSelection.usableDisks); + return (
- {equalDisksNotify && containEqualDisks(disks, lastRescanDisks) && + {equalDisksNotify && equalDisks && setEqualDisksNotify(false)} />} + actionClose={ { setEqualDisksNotify(false) }} />} />} {localDisksToolbar} {localDisksTable} diff --git a/ui/webui/test/check-storage b/ui/webui/test/check-storage index 3f0f73bb90e..b6ead85e88a 100755 --- a/ui/webui/test/check-storage +++ b/ui/webui/test/check-storage @@ -311,7 +311,7 @@ class TestStorageExtraDisks(anacondalib.VirtInstallMachineCase): s.check_disk_partition("vdb", "/dev/vdb1", "Encrypted (LUKS)", "429 MB") s.check_disk_partition("vdb", "/dev/vdb2", "ext4", "859 MB") - s.check_disk_selected("vda", True) + s.check_disk_selected("vda", False) s.check_disk_selected("vdb", False) b.assert_pixels( diff --git a/ui/webui/test/reference b/ui/webui/test/reference index 27104458d69..f8a5a93c5e5 160000 --- a/ui/webui/test/reference +++ b/ui/webui/test/reference @@ -1 +1 @@ -Subproject commit 27104458d69da4fb346d47f0a0bdb58c11fffefc +Subproject commit f8a5a93c5e560e79f49b02f88246d3f80c848508 From 1b30b1117cfa99fd21d833613e994c4e8d081765 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Fri, 23 Jun 2023 13:33:00 +0200 Subject: [PATCH 6/6] webui: tests: wait for the webui to update disk selection instead of asserting the state The latter approach is rather flaky. --- ui/webui/test/helpers/storage.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/webui/test/helpers/storage.py b/ui/webui/test/helpers/storage.py index b3eb0edc7c1..0211115404a 100644 --- a/ui/webui/test/helpers/storage.py +++ b/ui/webui/test/helpers/storage.py @@ -71,7 +71,11 @@ def click_checkbox_and_check_all_disks(self, disks, selected): @log_step(snapshot_before=True) def check_disk_selected(self, disk, selected=True): - assert self.browser.get_checked(f"#{disk} input") == selected + if selected: + self.browser.wait_visible(f"#{disk} input:checked") + else: + self.browser.wait_visible(f"#{disk} input:not(:checked)") + @log_step() def wait_no_disks(self):