diff --git a/frontend/src/api/k8s/pvcs.ts b/frontend/src/api/k8s/pvcs.ts index 05a6be46e5..4cb8b3e838 100644 --- a/frontend/src/api/k8s/pvcs.ts +++ b/frontend/src/api/k8s/pvcs.ts @@ -21,7 +21,7 @@ export const assemblePvc = ( hideFromUI?: boolean, ): PersistentVolumeClaimKind => { const { name: pvcName, description, size, storageClassName } = data; - const name = editName || translateDisplayNameForK8s(pvcName); + const name = editName || data.k8sName || translateDisplayNameForK8s(pvcName); return { apiVersion: 'v1', @@ -85,8 +85,13 @@ export const updatePvc = ( ): Promise => { const pvc = assemblePvc(data, namespace, existingData.metadata.name); + const pvcResource = _.merge({}, existingData, pvc); + if (!data.description && pvcResource.metadata.annotations?.['openshift.io/description']) { + pvcResource.metadata.annotations['openshift.io/description'] = undefined; + } + return k8sUpdateResource( - applyK8sAPIOptions({ model: PVCModel, resource: _.merge({}, existingData, pvc) }, opts), + applyK8sAPIOptions({ model: PVCModel, resource: pvcResource }, opts), ); }; diff --git a/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx b/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx index 80f5b3883f..c82c6fe4d5 100644 --- a/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx +++ b/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx @@ -2,7 +2,6 @@ import * as React from 'react'; import { Button, FormGroup, - FormHelperText, HelperText, HelperTextItem, TextArea, @@ -75,29 +74,31 @@ const K8sNameDescriptionField: React.FC = ({ value={name} onChange={(event, value) => onDataChange?.('name', value)} /> - {!showK8sField && !k8sName.state.immutable && ( - - - {nameHelperText} - {k8sName.value && ( + {nameHelperText || (!showK8sField && !k8sName.state.immutable) ? ( + + {nameHelperText && {nameHelperText}} + {!showK8sField && !k8sName.state.immutable && ( + <> + {k8sName.value && ( + + The resource name will be {k8sName.value}. + + )} - The resource name will be {k8sName.value}. + {' '} + - )} - - {' '} - - - - - )} + + )} + + ) : null} = ( diff --git a/frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts b/frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts index c8f1cc0b25..f13e97a81c 100644 --- a/frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts +++ b/frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts @@ -46,6 +46,7 @@ export const setupDefaults = ({ staticPrefix, regexp, invalidCharsMessage, + editableK8sName, }: UseK8sNameDescriptionDataConfiguration): K8sNameDescriptionFieldData => { let initialName = ''; let initialDescription = ''; @@ -72,7 +73,7 @@ export const setupDefaults = ({ k8sName: { value: initialK8sNameValue, state: { - immutable: initialK8sNameValue !== '', + immutable: !editableK8sName && initialK8sNameValue !== '', invalidCharacters: false, invalidLength: false, maxLength: configuredMaxLength, @@ -80,7 +81,10 @@ export const setupDefaults = ({ staticPrefix, regexp, invalidCharsMessage, - touched: false, + touched: + !!editableK8sName && + initialK8sNameValue !== '' && + initialK8sNameValue !== translateDisplayNameForK8s(initialName), }, }, })('name', initialName) satisfies K8sNameDescriptionFieldData; diff --git a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx index 120ffd1c27..fd23e5cf23 100644 --- a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx @@ -41,7 +41,8 @@ const BaseStorageModal: React.FC = ({ onClose, onNameChange, }) => { - const [createData, setCreateData, resetData] = useCreateStorageObject(existingPvc, existingData); + const [createData, setCreateData] = useCreateStorageObject(existingPvc, existingData); + const [nameDescValid, setNameDescValid] = React.useState(); const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const preferredStorageClass = usePreferredStorageClass(); const [defaultStorageClass] = useDefaultStorageClass(); @@ -64,21 +65,14 @@ const BaseStorageModal: React.FC = ({ setCreateData, ]); - const onBeforeClose = (submitted: boolean) => { - onClose(submitted); - setError(undefined); - setActionInProgress(false); - resetData(); - }; - - const canCreate = !actionInProgress && createData.name.trim().length > 0 && isValid; + const canCreate = !actionInProgress && nameDescValid && isValid; const submit = () => { setError(undefined); setActionInProgress(true); onSubmit(createData) - .then(() => onBeforeClose(true)) + .then(() => onClose(true)) .catch((err) => { setError(err); setActionInProgress(false); @@ -91,13 +85,13 @@ const BaseStorageModal: React.FC = ({ description={description} variant="medium" isOpen - onClose={() => onBeforeClose(false)} + onClose={() => onClose(false)} showClose footer={ onBeforeClose(false)} + onCancel={() => onClose(false)} isSubmitDisabled={!canCreate} error={error} alertTitle="Error creating storage" @@ -118,8 +112,10 @@ const BaseStorageModal: React.FC = ({ currentStatus={existingPvc?.status} autoFocusName onNameChange={onNameChange} + setValid={setNameDescValid} hasDuplicateName={hasDuplicateName} disableStorageClassSelect={!!existingPvc} + editableK8sName={!existingPvc} /> {children} diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx index cce0496cfc..59b67f3c19 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { StackItem } from '@patternfly/react-core'; +import { FormGroup } from '@patternfly/react-core'; import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; import { ForNotebookSelection, StorageData } from '~/pages/projects/types'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; @@ -133,30 +133,26 @@ const ClusterStorageModal: React.FC = ({ existingPvc, {workbenchEnabled && ( <> {hasExistingNotebookConnections && ( - - - setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) - } - loaded={removableNotebookLoaded} - error={removableNotebookError} - /> - - )} - - { - setNotebookData(forNotebookData); - }} - forNotebookData={notebookData} - connectedNotebooks={connectedNotebooks} + + setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) + } + loaded={removableNotebookLoaded} + error={removableNotebookError} /> - + )} + { + setNotebookData(forNotebookData); + }} + forNotebookData={notebookData} + connectedNotebooks={connectedNotebooks} + /> {restartNotebooks.length !== 0 && ( - + - + )} )} diff --git a/frontend/src/pages/projects/screens/detail/storage/utils.ts b/frontend/src/pages/projects/screens/detail/storage/utils.ts index 3af3ceaaf2..8198d0c57f 100644 --- a/frontend/src/pages/projects/screens/detail/storage/utils.ts +++ b/frontend/src/pages/projects/screens/detail/storage/utils.ts @@ -1,4 +1,4 @@ -import { getDisplayNameFromK8sResource, getDescriptionFromK8sResource } from '~/concepts/k8s/utils'; +import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import { PersistentVolumeClaimKind } from '~/k8sTypes'; import { StorageData } from '~/pages/projects/types'; diff --git a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx index 79a461354d..c3e431a66d 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx @@ -1,11 +1,13 @@ import * as React from 'react'; -import { Stack, StackItem } from '@patternfly/react-core'; +import { FormSection, HelperTextItem } from '@patternfly/react-core'; import { StorageData, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import PVSizeField from '~/pages/projects/components/PVSizeField'; -import NameDescriptionField from '~/concepts/k8s/NameDescriptionField'; import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; import { PersistentVolumeClaimKind } from '~/k8sTypes'; -import { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText'; +import K8sNameDescriptionField, { + useK8sNameDescriptionFieldData, +} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField'; +import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils'; import StorageClassSelect from './StorageClassSelect'; type CreateNewStorageSectionProps = { @@ -16,7 +18,9 @@ type CreateNewStorageSectionProps = { menuAppendTo?: HTMLElement; disableStorageClassSelect?: boolean; onNameChange?: (value: string) => void; + setValid?: (isValid: boolean) => void; hasDuplicateName?: boolean; + editableK8sName?: boolean; }; const CreateNewStorageSection = ({ @@ -27,49 +31,62 @@ const CreateNewStorageSection = ({ autoFocusName, disableStorageClassSelect, onNameChange, + setValid, hasDuplicateName, + editableK8sName, }: CreateNewStorageSectionProps): React.ReactNode => { const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; + const { data: clusterStorageNameDesc, onDataChange: setClusterNameDesc } = + useK8sNameDescriptionFieldData({ + initialData: { + name: data.name, + k8sName: data.k8sName, + description: data.description, + }, + editableK8sName, + }); + + React.useEffect(() => { + setData('name', clusterStorageNameDesc.name); + setData('k8sName', clusterStorageNameDesc.k8sName.value); + setData('description', clusterStorageNameDesc.description); + onNameChange?.(clusterStorageNameDesc.name); + setValid?.(isK8sNameDescriptionDataValid(clusterStorageNameDesc)); + // only update if the name description changes + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [clusterStorageNameDesc]); return ( - - - { - setData('name', newData.name); - setData('description', newData.description); - }} - onNameChange={onNameChange} - hasNameError={hasDuplicateName} - nameHelperText={ - hasDuplicateName ? : undefined - } - autoFocusName={autoFocusName} - /> - - - {isStorageClassesAvailable && ( - setData('storageClassName', name)} - disableStorageClassSelect={disableStorageClassSelect} - menuAppendTo={menuAppendTo} - /> - )} - - - + + {data.name} already exists. Try a different name. + + ) : undefined + } + /> + {isStorageClassesAvailable && ( + setData('storageClassName', name)} + disableStorageClassSelect={disableStorageClassSelect} menuAppendTo={menuAppendTo} - fieldID="create-new-storage-size" - currentStatus={currentStatus} - size={String(data.size)} - setSize={(size) => setData('size', size)} /> - - + )} + setData('size', size)} + /> + ); }; diff --git a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx index 41fae36a55..73df098ca6 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx @@ -1,5 +1,4 @@ import * as React from 'react'; -import { StackItem } from '@patternfly/react-core'; import { MountPath, StorageData } from '~/pages/projects/types'; import BaseStorageModal from '~/pages/projects/screens/detail/storage/BaseStorageModal'; import SpawnerMountPathField from './SpawnerMountPathField'; @@ -60,13 +59,11 @@ const WorkbenchStorageModal: React.FC = ({ isValid={!actionInProgress && !mountPath.error && !hasDuplicateName} onClose={onClose} > - - - + ); }; diff --git a/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts index 5b86f3b132..3f5bf5e1cc 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts @@ -1,4 +1,4 @@ -import { renderHook, act } from '@testing-library/react'; +import { renderHook } from '@testing-library/react'; import { useCreateStorageObject, useMountPathFormat, @@ -49,8 +49,10 @@ describe('useCreateStorageObject', () => { const [data] = result.current; expect(data).toEqual({ name: '', + k8sName: '', description: '', size: '1Gi', + storageClassName: undefined, }); }); @@ -63,21 +65,6 @@ describe('useCreateStorageObject', () => { expect(data.size).toBe('2Gi'); expect(data.storageClassName).toBe('test-storage-class'); }); - - it('should reset to default values when resetDefaults is called', () => { - const { result } = renderHook(() => useCreateStorageObject(existingData)); - const [, , resetDefaults] = result.current; - - act(() => { - resetDefaults(); - }); - - const [data] = result.current; - expect(data.name).toBe(''); - expect(data.description).toBe(''); - expect(data.size).toBe('1Gi'); // Default size from mock - expect(data.storageClassName).toBeUndefined(); - }); }); describe('validateMountPath', () => { diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 75a5380397..a0cabdcb6d 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -19,38 +19,21 @@ import { MOUNT_PATH_PREFIX } from './const'; export const useCreateStorageObject = ( existingData?: PersistentVolumeClaimKind, formData?: StorageData, -): [ - data: StorageData, - setData: UpdateObjectAtPropAndValue, - resetDefaults: () => void, -] => { +): [data: StorageData, setData: UpdateObjectAtPropAndValue] => { const size = useDefaultPvcSize(); - const createDataState = useGenericObjectState({ - name: '', - description: '', - size, - }); - const [, setCreateData] = createDataState; - const existingName = - formData?.name || (existingData ? getDisplayNameFromK8sResource(existingData) : ''); - const existingDescription = - formData?.description || (existingData ? getDescriptionFromK8sResource(existingData) : ''); - const existingSize = - formData?.size || (existingData ? existingData.spec.resources.requests.storage : size); - const existingStorageClassName = - formData?.storageClassName || existingData?.spec.storageClassName; + const createStorageData = { + name: formData?.name || (existingData ? getDisplayNameFromK8sResource(existingData) : ''), + k8sName: formData?.k8sName || (existingData ? existingData.metadata.name : ''), + description: + formData?.description || (existingData ? getDescriptionFromK8sResource(existingData) : ''), + size: formData?.size || (existingData ? existingData.spec.resources.requests.storage : size), + storageClassName: formData?.storageClassName || existingData?.spec.storageClassName, + }; - React.useEffect(() => { - if (existingName) { - setCreateData('name', existingName); - setCreateData('description', existingDescription); - setCreateData('size', existingSize); - setCreateData('storageClassName', existingStorageClassName); - } - }, [existingName, existingDescription, setCreateData, existingSize, existingStorageClassName]); + const [data, setData] = useGenericObjectState(createStorageData); - return createDataState; + return [data, setData]; }; export const useCreateStorageObjectForNotebook = ( diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index 3021cc8b92..45bc9a2815 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -60,6 +60,7 @@ export enum StorageType { export type StorageData = { name: string; + k8sName?: string; size?: string; storageType?: StorageType; description?: string; diff --git a/frontend/src/utilities/useGenericObjectState.ts b/frontend/src/utilities/useGenericObjectState.ts index b5b985fd83..0b0ab9f57d 100644 --- a/frontend/src/utilities/useGenericObjectState.ts +++ b/frontend/src/utilities/useGenericObjectState.ts @@ -11,7 +11,12 @@ const useGenericObjectState = (defaultData: T | (() => T)): GenericObjectStat const [value, setValue] = React.useState(defaultData); const setPropValue = React.useCallback>((propKey, propValue) => { - setValue((oldValue) => ({ ...oldValue, [propKey]: propValue })); + setValue((oldValue) => { + if (oldValue[propKey] !== propValue) { + return { ...oldValue, [propKey]: propValue }; + } + return oldValue; + }); }, []); const defaultDataRef = React.useRef(value);