From 355fbf7322aae4a2d98db4b85d5bb8fd2529566a Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 10 Oct 2024 08:56:54 -0500 Subject: [PATCH] Refactor useDefaultStorageClass hook to use FetchState (#3319) --- .../screens/server/SpawnerPage.tsx | 5 +- .../detail/storage/ManageStorageModal.tsx | 2 +- .../projects/screens/spawner/SpawnerPage.tsx | 2 +- .../spawner/storage/StorageClassSelect.tsx | 2 +- .../spawner/storage/useDefaultStorageClass.ts | 59 ++++++++++++------- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx b/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx index dac574eee6..aeaf3c1a58 100644 --- a/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx +++ b/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx @@ -84,7 +84,7 @@ const SpawnerPage: React.FC = () => { const [variableRows, setVariableRows] = React.useState([]); const [submitError, setSubmitError] = React.useState(null); - const defaultStorageClass = useDefaultStorageClass(); + const [defaultStorageClass, defaultStorageClassLoaded] = useDefaultStorageClass(); const [selectedAcceleratorProfile, setSelectedAcceleratorProfile] = useGenericObjectState({ @@ -99,7 +99,8 @@ const SpawnerPage: React.FC = () => { ({ errors, variables }) => Object.keys(errors).length > 0 || variables.find((variable) => !variable.name || variable.name === EMPTY_KEY), - ); + ) || + !defaultStorageClassLoaded; React.useEffect(() => { const setFirstValidImage = () => { diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx index 807fb54607..2c4a457b17 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx @@ -28,7 +28,7 @@ type AddStorageModalProps = { const ManageStorageModal: React.FC = ({ existingData, isOpen, onClose }) => { const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const preferredStorageClass = usePreferredStorageClass(); - const defaultStorageClass = useDefaultStorageClass(); + const [defaultStorageClass] = useDefaultStorageClass(); const [createData, setCreateData, resetData] = useCreateStorageObjectForNotebook(existingData); const [actionInProgress, setActionInProgress] = React.useState(false); diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx index ec74a5b62f..9fc14141c0 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx @@ -70,7 +70,7 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { >(); const [storageDataWithoutDefault, setStorageData] = useStorageDataObject(existingNotebook); - const defaultStorageClass = useDefaultStorageClass(); + const [defaultStorageClass] = useDefaultStorageClass(); const preferredStorageClass = usePreferredStorageClass(); const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const defaultStorageClassName = isStorageClassesAvailable diff --git a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx index 4d5b9c569d..4efb4030c7 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx @@ -30,7 +30,7 @@ const StorageClassSelect: React.FC = ({ }) => { const [storageClasses, storageClassesLoaded] = useStorageClasses(); const hasStorageClassConfigs = storageClasses.some((sc) => !!getStorageClassConfig(sc)); - const defaultSc = useDefaultStorageClass(); + const [defaultSc] = useDefaultStorageClass(); const enabledStorageClasses = storageClasses .filter((sc) => getStorageClassConfig(sc)?.isEnabled === true) diff --git a/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts b/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts index 8d69ec2270..b020197e37 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts @@ -3,35 +3,50 @@ import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; import useStorageClasses from '~/concepts/k8s/useStorageClasses'; import { StorageClassKind } from '~/k8sTypes'; import { getStorageClassConfig } from '~/pages/storageClasses/utils'; +import useFetchState, { + FetchState, + FetchStateCallbackPromise, + NotReadyError, +} from '~/utilities/useFetchState'; -const useDefaultStorageClass = (): StorageClassKind | undefined => { +const useDefaultStorageClass = (): FetchState => { const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; - const [storageClasses, storageClassesLoaded] = useStorageClasses(); - const [defaultStorageClass, setDefaultStorageClass] = React.useState< - StorageClassKind | undefined - >(); + const [storageClasses, storageClassesLoaded, storageClassesError] = useStorageClasses(); - React.useEffect(() => { - if (!storageClassesLoaded || !isStorageClassesAvailable) { - return; - } + const fetchDefaultStorageClass: FetchStateCallbackPromise = + React.useCallback( + () => + new Promise((resolve, reject) => { + if (!isStorageClassesAvailable) { + resolve(null); + } + if (!storageClassesLoaded) { + reject(new NotReadyError('Storage classes are not loaded')); + } + if (storageClassesError) { + resolve(null); + } - const enabledStorageClasses = storageClasses.filter( - (sc) => getStorageClassConfig(sc)?.isEnabled === true, - ); + const enabledStorageClasses = storageClasses.filter( + (sc) => getStorageClassConfig(sc)?.isEnabled === true, + ); - const defaultSc = enabledStorageClasses.find( - (sc) => getStorageClassConfig(sc)?.isDefault === true, - ); + const defaultSc = enabledStorageClasses.find( + (sc) => getStorageClassConfig(sc)?.isDefault === true, + ); - if (!defaultSc && enabledStorageClasses.length > 0) { - setDefaultStorageClass(enabledStorageClasses[0]); - } else { - setDefaultStorageClass(defaultSc); - } - }, [storageClasses, storageClassesLoaded, isStorageClassesAvailable]); + if (!defaultSc && enabledStorageClasses.length > 0) { + resolve(enabledStorageClasses[0]); + } else if (defaultSc) { + resolve(defaultSc); + } else { + resolve(null); + } + }), + [storageClasses, storageClassesLoaded, storageClassesError, isStorageClassesAvailable], + ); - return defaultStorageClass; + return useFetchState(fetchDefaultStorageClass, null); }; export default useDefaultStorageClass;