From facb71ceaedc96cc5b8a6e5283374756fcba4c00 Mon Sep 17 00:00:00 2001 From: Ondrej Ezr Date: Mon, 24 Jun 2024 22:39:38 +0200 Subject: [PATCH] Wizard: validate uniqueness of Blueprint name --- .../ValidatedTextInput.tsx | 7 +-- .../steps/Details/index.tsx | 2 - .../FileSystem/FileSystemConfiguration.tsx | 1 - .../utilities/useValidation.tsx | 46 +++++++++++++++++-- .../CreateImageWizard.test.tsx | 21 ++++----- .../steps/Details/Details.test.tsx | 11 +++++ .../steps/Oscap/Oscap.test.tsx | 11 +++-- src/test/fixtures/blueprints.ts | 2 +- src/test/mocks/handlers.js | 7 ++- 9 files changed, 80 insertions(+), 28 deletions(-) diff --git a/src/Components/CreateImageWizardV2/ValidatedTextInput.tsx b/src/Components/CreateImageWizardV2/ValidatedTextInput.tsx index a8a7f023..18a7bc42 100644 --- a/src/Components/CreateImageWizardV2/ValidatedTextInput.tsx +++ b/src/Components/CreateImageWizardV2/ValidatedTextInput.tsx @@ -23,7 +23,6 @@ interface HookValidatedTextInputPropTypes extends TextInputProps { dataTestId?: string | undefined; ouiaId?: string; ariaLabel: string | undefined; - helperText: string | undefined; value: string; placeholder?: string; stepValidation: StepValidation; @@ -34,7 +33,6 @@ export const HookValidatedInput = ({ dataTestId, ouiaId, ariaLabel, - helperText, value, placeholder, onChange, @@ -43,7 +41,10 @@ export const HookValidatedInput = ({ }: HookValidatedTextInputPropTypes) => { const [isPristine, setIsPristine] = useState(!value ? true : false); // Do not surface validation on pristine state components + // Allow step validation to be set on pristine state, when needed const validated = isPristine + ? 'default' + : stepValidation.errors[fieldName] === 'default' ? 'default' : stepValidation.errors[fieldName] ? 'error' @@ -69,7 +70,7 @@ export const HookValidatedInput = ({ {validated === 'error' && ( - {helperText} + {stepValidation.errors[fieldName]} )} diff --git a/src/Components/CreateImageWizardV2/steps/Details/index.tsx b/src/Components/CreateImageWizardV2/steps/Details/index.tsx index 1031d691..487c0adb 100644 --- a/src/Components/CreateImageWizardV2/steps/Details/index.tsx +++ b/src/Components/CreateImageWizardV2/steps/Details/index.tsx @@ -56,7 +56,6 @@ const DetailsStep = () => { dataTestId="blueprint" value={blueprintName} onChange={handleNameChange} - helperText="Please enter a valid name" placeholder="Add blueprint name" stepValidation={stepValidation} fieldName="name" @@ -79,7 +78,6 @@ const DetailsStep = () => { dataTestId="blueprint description" value={blueprintDescription || ''} onChange={handleDescriptionChange} - helperText="Please enter a valid description" placeholder="Add description" stepValidation={stepValidation} fieldName="description" diff --git a/src/Components/CreateImageWizardV2/steps/FileSystem/FileSystemConfiguration.tsx b/src/Components/CreateImageWizardV2/steps/FileSystem/FileSystemConfiguration.tsx index e7e7a5e7..38906133 100644 --- a/src/Components/CreateImageWizardV2/steps/FileSystem/FileSystemConfiguration.tsx +++ b/src/Components/CreateImageWizardV2/steps/FileSystem/FileSystemConfiguration.tsx @@ -307,7 +307,6 @@ const MinimumSize = ({ partition }: MinimumSizePropTypes) => { return ( (null); + + const [trigger] = useLazyGetBlueprintsQuery(); + + useEffect(() => { + if (wizardMode === 'create' && name !== '' && nameValid) { + trigger({ name }) + .unwrap() + .then((response: BlueprintsResponse) => { + if (response?.meta?.count > 0) { + setUniqueName(false); + } else { + setUniqueName(true); + } + }) + .catch(() => { + // If the request fails, we assume the name is unique + setUniqueName(true); + }); + } + }, [wizardMode, name, setUniqueName, trigger]); + + let nameError = ''; + if (!nameValid) { + nameError = 'Invalid blueprint name'; + } else if (uniqueName === false) { + nameError = 'Blueprint with this name already exists'; + } else if (wizardMode === 'create' && uniqueName === null) { + // Hack to keep the error message from flickering + nameError = 'default'; + } + return { errors: { - name: nameValid ? '' : 'Invalid name', + name: nameError, description: descriptionValid ? '' : 'Invalid description', }, - disabledNext: !nameValid || !descriptionValid, + disabledNext: !!nameError || !descriptionValid, }; } diff --git a/src/test/Components/CreateImageWizardV2/CreateImageWizard.test.tsx b/src/test/Components/CreateImageWizardV2/CreateImageWizard.test.tsx index bf68231d..1bb02a44 100644 --- a/src/test/Components/CreateImageWizardV2/CreateImageWizard.test.tsx +++ b/src/test/Components/CreateImageWizardV2/CreateImageWizard.test.tsx @@ -914,16 +914,17 @@ describe('Step Details', () => { await setUp(); // Enter image name + + const invalidName = 'a'.repeat(101); + await enterBlueprintName(invalidName); + expect(await getNextButton()).toHaveClass('pf-m-disabled'); + expect(await getNextButton()).toBeDisabled(); const nameInput = await screen.findByRole('textbox', { name: /blueprint name/i, }); - const invalidName = 'a'.repeat(101); - await user.type(nameInput, invalidName); - expect(await getNextButton()).toHaveClass('pf-m-disabled'); - expect(await getNextButton()).toBeDisabled(); await user.clear(nameInput); - await user.type(nameInput, 'valid-name'); + await enterBlueprintName(); expect(await getNextButton()).not.toHaveClass('pf-m-disabled'); expect(await getNextButton()).toBeEnabled(); @@ -992,10 +993,7 @@ describe('Step Review', () => { // skip firstboot await clickNext(); // skip Details - const blueprintName = await screen.findByRole('textbox', { - name: /blueprint name/i, - }); - await user.type(blueprintName, 'valid-name'); + await enterBlueprintName(); await clickNext(); }; @@ -1056,10 +1054,7 @@ describe('Step Review', () => { await clickNext(); // skip First boot await clickNext(); - const blueprintName = await screen.findByRole('textbox', { - name: /blueprint name/i, - }); - await user.type(blueprintName, 'valid-name'); + await enterBlueprintName(); await clickNext(); }; diff --git a/src/test/Components/CreateImageWizardV2/steps/Details/Details.test.tsx b/src/test/Components/CreateImageWizardV2/steps/Details/Details.test.tsx index d25d13e3..ff037508 100644 --- a/src/test/Components/CreateImageWizardV2/steps/Details/Details.test.tsx +++ b/src/test/Components/CreateImageWizardV2/steps/Details/Details.test.tsx @@ -82,6 +82,17 @@ describe('validates name', () => { const nextButton = await getNextButton(); expect(nextButton).toBeEnabled(); }); + + test('with non-unique name', async () => { + await renderCreateMode(); + await goToRegistrationStep(); + await clickRegisterLater(); + await goToDetailsStep(); + await enterBlueprintName('Lemon Pie'); + + const nextButton = await getNextButton(); + expect(nextButton).toBeDisabled(); + }); }); describe('registration request generated correctly', () => { diff --git a/src/test/Components/CreateImageWizardV2/steps/Oscap/Oscap.test.tsx b/src/test/Components/CreateImageWizardV2/steps/Oscap/Oscap.test.tsx index e2e0d483..d82dd2ad 100644 --- a/src/test/Components/CreateImageWizardV2/steps/Oscap/Oscap.test.tsx +++ b/src/test/Components/CreateImageWizardV2/steps/Oscap/Oscap.test.tsx @@ -93,7 +93,7 @@ const goToReviewStep = async () => { await clickNext(); // Additional packages await clickNext(); // FirstBoot await clickNext(); // Details - await enterBlueprintName('oscap'); + await enterBlueprintName('Oscap test'); await clickNext(); // Review }; @@ -130,7 +130,10 @@ describe('oscap', () => { const receivedRequest = await interceptBlueprintRequest(CREATE_BLUEPRINT); - const expectedRequest = oscapCreateBlueprintRequest; + const expectedRequest: CreateBlueprintRequest = { + ...oscapCreateBlueprintRequest, + name: 'Oscap test', + }; expect(receivedRequest).toEqual(expectedRequest); }); @@ -146,7 +149,7 @@ describe('oscap', () => { const expectedRequest: CreateBlueprintRequest = { ...baseCreateBlueprintRequest, - name: 'oscap', + name: 'Oscap test', }; expect(receivedRequest).toEqual(expectedRequest); @@ -170,7 +173,7 @@ describe('oscap', () => { kernel: expectedKernelCisL2, filesystem: expectedFilesystemCisL2, }, - name: 'oscap', + name: 'Oscap test', }; await waitFor(() => { diff --git a/src/test/fixtures/blueprints.ts b/src/test/fixtures/blueprints.ts index fe3a34b1..f652f83d 100644 --- a/src/test/fixtures/blueprints.ts +++ b/src/test/fixtures/blueprints.ts @@ -157,7 +157,7 @@ export const mockGetBlueprints: GetBlueprintsApiResponse = { }, { id: '147032db-8697-4638-8fdd-6f428100d8fc', - name: 'Red Velvet', + name: 'Pink Velvet', description: 'Layered cake with icing', version: 1, last_modified_at: '2021-09-08T21:00:00.000Z', diff --git a/src/test/mocks/handlers.js b/src/test/mocks/handlers.js index 6b8ff1e2..414b44bd 100644 --- a/src/test/mocks/handlers.js +++ b/src/test/mocks/handlers.js @@ -155,11 +155,16 @@ export const handlers = [ } ), rest.get(`${IMAGE_BUILDER_API}/blueprints`, (req, res, ctx) => { + const nameParam = req.url.searchParams.get('name'); const search = req.url.searchParams.get('search'); const limit = req.url.searchParams.get('limit') || '10'; const offset = req.url.searchParams.get('offset') || '0'; const resp = Object.assign({}, mockGetBlueprints); - if (search) { + if (nameParam) { + resp.data = resp.data.filter(({ name }) => { + return nameParam === name; + }); + } else if (search) { let regexp; try { regexp = new RegExp(search);