Wizard: validate wizard with Redux Hook

Use redux hook to validate the form.
This gives us single point of contact for "is the data valid?"
while not requiring every redux action touching form data to perform validation.

It's not perfect and might be improved when using external library solving the problems we're having.
This commit is contained in:
Ondrej Ezr 2024-04-24 15:48:58 +02:00 committed by Klara Simickova
parent fad7648d38
commit 4692eae454
10 changed files with 150 additions and 233 deletions

View file

@ -1,18 +1,16 @@
import React, { useEffect, useMemo } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import {
Button,
Wizard,
WizardFooterWrapper,
WizardStep,
WizardStepType,
useWizardContext,
} from '@patternfly/react-core';
import { useNavigate, useSearchParams } from 'react-router-dom';
import DetailsStep from './steps/Details';
import FileSystemStep from './steps/FileSystem';
import { FileSystemStepFooter } from './steps/FileSystem/FileSystemConfiguration';
import FileSystemStep, { FileSystemContext } from './steps/FileSystem';
import FirstBootStep from './steps/FirstBoot';
import ImageOutputStep from './steps/ImageOutput';
import OscapStep from './steps/Oscap';
@ -25,6 +23,10 @@ import SnapshotStep from './steps/Snapshot';
import Aws from './steps/TargetEnvironment/Aws';
import Azure from './steps/TargetEnvironment/Azure';
import Gcp from './steps/TargetEnvironment/Gcp';
import {
useFilesystemValidation,
useDetailsValidation,
} from './utilities/useValidation';
import {
isAwsAccountIdValid,
isAzureTenantGUIDValid,
@ -54,7 +56,6 @@ import {
selectGcpShareMethod,
selectImageTypes,
selectRegistrationType,
selectStepValidation,
addImageType,
selectSnapshotDate,
selectUseLatest,
@ -67,11 +68,13 @@ import { ImageBuilderHeader } from '../sharedComponents/ImageBuilderHeader';
type CustomWizardFooterPropType = {
disableBack?: boolean;
disableNext: boolean;
beforeNext?: () => boolean;
};
export const CustomWizardFooter = ({
disableBack: disableBack,
disableNext: disableNext,
beforeNext,
}: CustomWizardFooterPropType) => {
const { goToNextStep, goToPrevStep, close } = useWizardContext();
return (
@ -79,7 +82,9 @@ export const CustomWizardFooter = ({
<Button
ouiaId="wizard-next-btn"
variant="primary"
onClick={goToNextStep}
onClick={() => {
if (!beforeNext || beforeNext()) goToNextStep();
}}
isDisabled={disableNext}
>
Next
@ -172,15 +177,11 @@ const CreateImageWizard = ({ isEdit }: CreateImageWizardProps) => {
const snapshotStepRequiresChoice = !useLatest && !snapshotDate;
const [currentStep, setCurrentStep] = React.useState<WizardStepType>();
const onStepChange = (
_event: React.MouseEvent<HTMLButtonElement>,
currentStep: WizardStepType
) => setCurrentStep(currentStep);
const detailsValidation = useDetailsValidation();
const fileSystemValidation = useFilesystemValidation();
const [filesystemPristine, setFilesystemPristine] = useState(true);
const detailsValidation = useAppSelector(selectStepValidation('details'));
let startIndex = 1; // default index
if (isEdit) {
if (snapshottingEnabled) {
startIndex = isFirstBootEnabled ? 15 : 14;
@ -196,7 +197,6 @@ const CreateImageWizard = ({ isEdit }: CreateImageWizardProps) => {
<Wizard
startIndex={startIndex}
onClose={() => navigate(resolveRelPath(''))}
onStepChange={onStepChange}
isVisitRequired
>
<WizardStep
@ -302,9 +302,24 @@ const CreateImageWizard = ({ isEdit }: CreateImageWizardProps) => {
<WizardStep
name="File system configuration"
id="step-file-system"
footer={<FileSystemStepFooter />}
footer={
<CustomWizardFooter
beforeNext={() => {
if (fileSystemValidation.disabledNext) {
setFilesystemPristine(false);
return false;
}
return true;
}}
disableNext={
!filesystemPristine && fileSystemValidation.disabledNext
}
/>
}
>
<FileSystemStep />
<FileSystemContext.Provider value={filesystemPristine}>
<FileSystemStep />
</FileSystemContext.Provider>
</WizardStep>
<WizardStep
name="Content"
@ -358,17 +373,11 @@ const CreateImageWizard = ({ isEdit }: CreateImageWizardProps) => {
)}
<WizardStep
name="Details"
id="step-details"
status={
currentStep?.id !== 'step-details' &&
detailsValidation === 'error'
? 'error'
: 'default'
}
id={'step-details'}
isDisabled={snapshotStepRequiresChoice}
footer={
<CustomWizardFooter
disableNext={detailsValidation !== 'success'}
disableNext={detailsValidation.disabledNext}
/>
}
>

View file

@ -7,11 +7,7 @@ import {
TextInputProps,
} from '@patternfly/react-core';
import { useAppDispatch, useAppSelector } from '../../store/hooks';
import {
setStepInputValidation,
selectInputValidation,
} from '../../store/wizardSlice';
import { StepValidation } from './utilities/useValidation';
interface ValidatedTextInputPropTypes extends TextInputProps {
dataTestId?: string | undefined;
@ -23,63 +19,38 @@ interface ValidatedTextInputPropTypes extends TextInputProps {
placeholder?: string;
}
interface StateValidatedTextInputPropTypes extends TextInputProps {
interface HookValidatedTextInputPropTypes extends TextInputProps {
dataTestId?: string | undefined;
ouiaId?: string;
stepId: string;
inputId: string;
ariaLabel: string | undefined;
helperText: string | undefined;
validator: (value: string | undefined) => boolean;
value: string;
placeholder?: string;
stepValidation: StepValidation;
fieldName: string;
}
export const StateValidatedInput = ({
export const HookValidatedInput = ({
dataTestId,
ouiaId,
stepId,
inputId,
ariaLabel,
helperText,
validator,
value,
placeholder,
onChange,
}: StateValidatedTextInputPropTypes) => {
const dispatch = useAppDispatch();
const validatedState = useAppSelector(selectInputValidation(stepId, inputId));
stepValidation,
fieldName,
}: HookValidatedTextInputPropTypes) => {
const [isPristine, setIsPristine] = useState(!value ? true : false);
// Do not surface validation on pristine state components
const validated = isPristine ? 'default' : validatedState;
const validated = isPristine
? 'default'
: stepValidation.errors[fieldName]
? 'error'
: 'success';
const handleBlur = () => {
setIsPristine(false);
const isValid = validator(value);
dispatch(
setStepInputValidation({
stepId,
inputId,
isValid,
errorText: isValid ? helperText : undefined,
})
);
};
const wrappedOnChange = (
evt: React.FormEvent<HTMLInputElement>,
newVal: string
) => {
if (onChange) onChange(evt, newVal);
const isValid = validator(newVal);
dispatch(
setStepInputValidation({
stepId,
inputId,
isValid,
errorText: isValid ? helperText : undefined,
})
);
};
return (
@ -89,7 +60,7 @@ export const StateValidatedInput = ({
data-testid={dataTestId}
ouiaId={ouiaId}
type="text"
onChange={wrappedOnChange}
onChange={onChange}
validated={validated}
aria-label={ariaLabel}
onBlur={handleBlur}

View file

@ -17,11 +17,8 @@ import {
selectBlueprintDescription,
selectBlueprintName,
} from '../../../../store/wizardSlice';
import { StateValidatedInput } from '../../ValidatedTextInput';
import {
isBlueprintDescriptionValid,
isBlueprintNameValid,
} from '../../validators';
import { useDetailsValidation } from '../../utilities/useValidation';
import { HookValidatedInput } from '../../ValidatedTextInput';
const DetailsStep = () => {
const dispatch = useAppDispatch();
@ -41,6 +38,8 @@ const DetailsStep = () => {
dispatch(changeBlueprintDescription(description));
};
const stepValidation = useDetailsValidation();
return (
<Form>
<Title headingLevel="h1" size="xl">
@ -52,16 +51,15 @@ const DetailsStep = () => {
blueprint.
</Text>
<FormGroup isRequired label="Blueprint name" fieldId="blueprint-name">
<StateValidatedInput
<HookValidatedInput
ariaLabel="blueprint name"
dataTestId="blueprint"
stepId="details"
inputId="blueprint-name"
value={blueprintName}
validator={isBlueprintNameValid}
onChange={handleNameChange}
helperText="Please enter a valid name"
placeholder="Add blueprint name"
stepValidation={stepValidation}
fieldName="name"
/>
<FormHelperText>
<HelperText>
@ -76,16 +74,15 @@ const DetailsStep = () => {
label="Blueprint description"
fieldId="blueprint-description-name"
>
<StateValidatedInput
<HookValidatedInput
ariaLabel="blueprint description"
dataTestId="blueprint description"
stepId="details"
inputId="blueprint-description"
value={blueprintDescription || ''}
validator={isBlueprintDescriptionValid}
onChange={handleDescriptionChange}
helperText="Please enter a valid description"
placeholder="Add description"
stepValidation={stepValidation}
fieldName="description"
/>
</FormGroup>
</Form>

View file

@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react';
import React, { useState } from 'react';
import {
Alert,
@ -7,8 +7,6 @@ import {
TextContent,
TextInput,
TextVariants,
useWizardContext,
WizardFooterWrapper,
} from '@patternfly/react-core';
import { Select, SelectOption } from '@patternfly/react-core/deprecated';
import { MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons';
@ -33,17 +31,12 @@ import {
removePartition,
selectPartitions,
changePartitionUnit,
setIsNextButtonTouched,
selectIsNextButtonTouched,
selectFileSystemPartitionMode,
} from '../../../../store/wizardSlice';
import UsrSubDirectoriesDisabled from '../../UsrSubDirectoriesDisabled';
import { ValidatedTextInput } from '../../ValidatedTextInput';
import {
getDuplicateMountPoints,
isFileSystemConfigValid,
isMountpointMinSizeValid,
} from '../../validators';
import { useFilesystemValidation } from '../../utilities/useValidation';
import { HookValidatedInput } from '../../ValidatedTextInput';
import { FileSystemContext } from './index';
export type Partition = {
id: string;
@ -52,47 +45,6 @@ export type Partition = {
unit: Units;
};
export const FileSystemStepFooter = () => {
const { goToNextStep, goToPrevStep, close } = useWizardContext();
const [isValid, setIsValid] = useState(false);
const dispatch = useAppDispatch();
const [isNextDisabled, setNextDisabled] = useState(false);
const fileSystemPartitionMode = useAppSelector(selectFileSystemPartitionMode);
const partitions = useAppSelector(selectPartitions);
const onValidate = () => {
dispatch(setIsNextButtonTouched(false));
if (!isValid) {
setNextDisabled(true);
} else {
goToNextStep();
}
};
useEffect(() => {
if (
fileSystemPartitionMode === 'automatic' ||
isFileSystemConfigValid(partitions)
) {
setIsValid(true);
} else setIsValid(false);
setNextDisabled(false);
dispatch(setIsNextButtonTouched(true));
}, [partitions, fileSystemPartitionMode, dispatch]);
return (
<WizardFooterWrapper>
<Button onClick={onValidate} isDisabled={isNextDisabled}>
Next
</Button>
<Button variant="secondary" onClick={goToPrevStep}>
Back
</Button>
<Button ouiaId="wizard-cancel-btn" variant="link" onClick={close}>
Cancel
</Button>
</WizardFooterWrapper>
);
};
const FileSystemConfiguration = () => {
const partitions = useAppSelector(selectPartitions);
const environments = useAppSelector(selectImageTypes);
@ -193,12 +145,11 @@ export const Row = ({
onDrop,
}: RowPropTypes) => {
const dispatch = useAppDispatch();
const partitions = useAppSelector(selectPartitions);
const handleRemovePartition = (id: string) => {
dispatch(removePartition(id));
};
const isNextButtonPristine = useAppSelector(selectIsNextButtonTouched);
const duplicates = getDuplicateMountPoints(partitions);
const stepValidation = useFilesystemValidation();
const isPristine = React.useContext(FileSystemContext);
return (
<Tr
@ -215,15 +166,14 @@ export const Row = ({
/>
<Td className="pf-m-width-20">
<MountpointPrefix partition={partition} />
{!isNextButtonPristine &&
duplicates.indexOf(partition.mountpoint) !== -1 && (
<Alert
variant="danger"
isInline
isPlain
title="Duplicate mount point."
/>
)}
{!isPristine && stepValidation.errors[`mountpoint-${partition.id}`] && (
<Alert
variant="danger"
isInline
isPlain
title={stepValidation.errors[`mountpoint-${partition.id}`]}
/>
)}
</Td>
{partition.mountpoint !== '/' &&
!partition.mountpoint.startsWith('/boot') &&
@ -352,15 +302,17 @@ export const getConversionFactor = (units: Units) => {
const MinimumSize = ({ partition }: MinimumSizePropTypes) => {
const dispatch = useAppDispatch();
const stepValidation = useFilesystemValidation();
return (
<ValidatedTextInput
<HookValidatedInput
ariaLabel="minimum partition size"
helperText="Must be larger than 0"
validator={isMountpointMinSizeValid}
value={partition.min_size}
type="text"
ouiaId="size"
stepValidation={stepValidation}
fieldName={`min-size-${partition.id}`}
onChange={(event, minSize) => {
if (minSize === '' || /^\d+$/.test(minSize)) {
dispatch(

View file

@ -11,6 +11,8 @@ import { selectFileSystemPartitionMode } from '../../../../store/wizardSlice';
import { useHasSpecificTargetOnly } from '../../utilities/hasSpecificTargetOnly';
export type FileSystemPartitionMode = 'automatic' | 'manual';
export const FileSystemContext = React.createContext<boolean>(true);
const FileSystemStep = () => {
const fileSystemPartitionMode = useAppSelector(selectFileSystemPartitionMode);
const hasIsoTargetOnly = useHasSpecificTargetOnly('image-installer');

View file

@ -15,14 +15,14 @@ import { useNavigate, useParams } from 'react-router-dom';
import { CreateSaveAndBuildBtn, CreateSaveButton } from './CreateDropdown';
import { EditSaveAndBuildBtn, EditSaveButton } from './EditDropdown';
import { useServerStore, useAppSelector } from '../../../../../store/hooks';
import { useServerStore } from '../../../../../store/hooks';
import {
useCreateBlueprintMutation,
useUpdateBlueprintMutation,
} from '../../../../../store/imageBuilderApi';
import { selectIsValid } from '../../../../../store/wizardSlice';
import { resolveRelPath } from '../../../../../Utilities/path';
import { mapRequestFromState } from '../../../utilities/requestMapper';
import { useIsBlueprintValid } from '../../../utilities/useValidation';
const ReviewWizardFooter = () => {
const { goToPrevStep, close } = useWizardContext();
@ -41,7 +41,7 @@ const ReviewWizardFooter = () => {
setIsOpen(!isOpen);
};
const navigate = useNavigate();
const isValid = useAppSelector(selectIsValid);
const isValid = useIsBlueprintValid();
useEffect(() => {
if (isUpdateSuccess || isCreateSuccess) {

View file

@ -170,12 +170,10 @@ export const mapRequestToState = (request: BlueprintResponse): wizardState => {
partitions: request.customizations.filesystem.map((fs) =>
convertFilesystemToPartition(fs)
),
isNextButtonTouched: true,
}
: {
mode: 'automatic' as FileSystemPartitionMode,
partitions: [],
isNextButtonTouched: true,
};
const arch = request.image_requests[0].architecture;
@ -259,7 +257,6 @@ export const mapRequestToState = (request: BlueprintResponse): wizardState => {
repository: '',
package_list: [],
})) || [],
stepValidations: {},
};
};

View file

@ -0,0 +1,65 @@
import { useAppSelector } from '../../../store/hooks';
import {
selectBlueprintName,
selectBlueprintDescription,
selectFileSystemPartitionMode,
selectPartitions,
} from '../../../store/wizardSlice';
import {
getDuplicateMountPoints,
isBlueprintNameValid,
isBlueprintDescriptionValid,
isMountpointMinSizeValid,
} from '../validators';
export type StepValidation = {
errors: {
[key: string]: string;
};
disabledNext: boolean;
};
export function useIsBlueprintValid(): boolean {
const filesystem = useFilesystemValidation();
const details = useDetailsValidation();
return !filesystem.disabledNext && !details.disabledNext;
}
export function useFilesystemValidation(): StepValidation {
const mode = useAppSelector(selectFileSystemPartitionMode);
const partitions = useAppSelector(selectPartitions);
let disabledNext = false;
const errors: { [key: string]: string } = {};
if (mode === 'automatic') {
return { errors, disabledNext: false };
}
const duplicates = getDuplicateMountPoints(partitions);
for (const partition of partitions) {
if (!isMountpointMinSizeValid(partition.min_size)) {
errors[`min-size-${partition.id}`] = 'Invalid size';
disabledNext = true;
}
if (duplicates.includes(partition.mountpoint)) {
errors[`mountpoint-${partition.id}`] = 'Duplicate mount points';
disabledNext = true;
}
}
return { errors, disabledNext };
}
export function useDetailsValidation(): StepValidation {
const name = useAppSelector(selectBlueprintName);
const description = useAppSelector(selectBlueprintDescription);
const nameValid = isBlueprintNameValid(name);
const descriptionValid = isBlueprintDescriptionValid(description);
return {
errors: {
name: nameValid ? '' : 'Invalid name',
description: descriptionValid ? '' : 'Invalid description',
},
disabledNext: !nameValid || !descriptionValid,
};
}

View file

@ -28,7 +28,6 @@ import {
GcpShareMethod,
} from '../Components/CreateImageWizardV2/steps/TargetEnvironment/Gcp';
import { V1ListSourceResponseItem } from '../Components/CreateImageWizardV2/types';
import { isBlueprintNameValid } from '../Components/CreateImageWizardV2/validators';
import { RHEL_9, X86_64 } from '../constants';
import { RootState } from '.';
@ -78,7 +77,6 @@ export type wizardState = {
fileSystem: {
mode: FileSystemPartitionMode;
partitions: Partition[];
isNextButtonTouched: boolean;
};
snapshotting: {
useLatest: boolean;
@ -98,15 +96,6 @@ export type wizardState = {
blueprintName: string;
blueprintDescription: string;
};
stepValidations: {
[key: string]: {
validated: 'default' | 'success' | 'error';
errorText: string | null;
inputs: {
[key: string]: boolean;
};
};
};
};
const initialState: wizardState = {
@ -145,7 +134,6 @@ const initialState: wizardState = {
fileSystem: {
mode: 'automatic',
partitions: [],
isNextButtonTouched: true,
},
snapshotting: {
useLatest: true,
@ -162,7 +150,6 @@ const initialState: wizardState = {
blueprintName: '',
blueprintDescription: '',
},
stepValidations: {},
firstBoot: { script: '' },
};
@ -250,10 +237,6 @@ export const selectFileSystemPartitionMode = (state: RootState) => {
return state.wizard.fileSystem.mode;
};
export const selectIsNextButtonTouched = (state: RootState) => {
return state.wizard.fileSystem.isNextButtonTouched;
};
export const selectPartitions = (state: RootState) => {
return state.wizard.fileSystem.partitions;
};
@ -293,23 +276,6 @@ export const selectBlueprintDescription = (state: RootState) => {
return state.wizard.details.blueprintDescription;
};
export const selectIsValid = (state: RootState) => {
return Object.values(state.wizard.stepValidations).every(
(step) => step.validated === 'success'
);
};
export const selectStepValidation = (stepId: string) => (state: RootState) => {
return state.wizard.stepValidations[stepId]?.validated || 'default';
};
export const selectInputValidation =
(stepId: string, inputId: string) => (state: RootState) => {
const isValid = state.wizard.stepValidations[stepId]?.inputs?.[inputId];
if (isValid === undefined) return 'default';
return isValid ? 'success' : 'error';
};
export const selectFirstBootScript = (state: RootState) => {
return state.wizard.firstBoot?.script;
};
@ -319,19 +285,8 @@ export const wizardSlice = createSlice({
initialState,
reducers: {
initializeWizard: () => initialState,
loadWizardState: (state, action: PayloadAction<wizardState>) => {
const isNameValid = isBlueprintNameValid(
action.payload.details.blueprintName
);
action.payload.stepValidations = {
details: {
validated: isNameValid ? 'success' : 'error',
errorText: null,
inputs: { name: isNameValid },
},
};
return action.payload;
},
loadWizardState: (state, action: PayloadAction<wizardState>) =>
action.payload,
changeServerUrl: (state, action: PayloadAction<string>) => {
state.env.serverUrl = action.payload;
},
@ -448,9 +403,6 @@ export const wizardSlice = createSlice({
) => {
state.fileSystem.partitions = action.payload;
},
setIsNextButtonTouched: (state, action: PayloadAction<boolean>) => {
state.fileSystem.isNextButtonTouched = action.payload;
},
changeFileSystemPartitionMode: (
state,
action: PayloadAction<FileSystemPartitionMode>
@ -639,32 +591,6 @@ export const wizardSlice = createSlice({
setFirstBootScript: (state, action: PayloadAction<string>) => {
state.firstBoot.script = action.payload;
},
setStepInputValidation: (
state,
action: PayloadAction<{
stepId: string;
inputId: string;
isValid: boolean;
errorText: string | undefined;
}>
) => {
const inputs = {
...state.stepValidations[action.payload.stepId]?.inputs,
[action.payload.inputId]: action.payload.isValid,
};
const validated = Object.values(inputs).every((input) => input === true)
? 'success'
: 'error';
state.stepValidations[action.payload.stepId] = {
...state.stepValidations[action.payload.stepId],
validated,
inputs,
};
if (!action.payload.isValid && action.payload.errorText) {
state.stepValidations[action.payload.stepId].errorText =
action.payload.errorText;
}
},
},
});
@ -695,7 +621,6 @@ export const {
changeActivationKey,
changeOscapProfile,
changeFileSystemConfiguration,
setIsNextButtonTouched,
changeFileSystemPartitionMode,
clearPartitions,
addPartition,
@ -718,7 +643,6 @@ export const {
changeBlueprintName,
changeBlueprintDescription,
loadWizardState,
setStepInputValidation,
setFirstBootScript,
} = wizardSlice.actions;
export default wizardSlice.reducer;

View file

@ -817,7 +817,7 @@ describe('Step File system configuration', () => {
await clickNext();
expect(await getNextButton()).toBeDisabled();
const mountPointAlerts = screen.getAllByRole('heading', {
name: /danger alert: duplicate mount point\./i,
name: /danger alert: duplicate mount point/i,
});
const tbody = await screen.findByTestId('file-system-configuration-tbody');
const rows = within(tbody).getAllByRole('row');
@ -910,7 +910,7 @@ describe('Step Details', () => {
await clickNext();
};
test('image name invalid for more than 63 chars', async () => {
test('image name invalid for more than 100 chars and description for 250', async () => {
await setUp();
// Enter image name