From b71f58d7d8fb36cd84e6fa71b3894475e9fd605a Mon Sep 17 00:00:00 2001 From: Martin Morales Date: Thu, 23 Mar 2023 18:47:05 -0500 Subject: [PATCH 1/3] Fix EntityPicker allowing clear value when required #16993 Signed-off-by: Martin Morales --- .changeset/long-ravens-listen.md | 5 +++++ .../src/components/fields/EntityPicker/EntityPicker.tsx | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .changeset/long-ravens-listen.md diff --git a/.changeset/long-ravens-listen.md b/.changeset/long-ravens-listen.md new file mode 100644 index 0000000000..8300c12f77 --- /dev/null +++ b/.changeset/long-ravens-listen.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': minor +--- + +Fixed bug in EntityPicker component that allowed for empty values when field is required. This bug occurs only after a user fills the EntityPicker field, clears it, and then continues to the next form step. diff --git a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx index aeb4a52f77..e19e27641c 100644 --- a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx @@ -96,7 +96,8 @@ export const EntityPicker = (props: EntityPickerProps) => { (_: any, ref: string | Entity | null, reason: AutocompleteChangeReason) => { // ref can either be a string from free solo entry or if (typeof ref !== 'string') { - onChange(ref ? stringifyEntityRef(ref as Entity) : ''); + // if ref does not exist: pass 'undefined' to trigger validation for non-free solo entries + onChange(ref ? stringifyEntityRef(ref as Entity) : undefined); } else { if (reason === 'blur' || reason === 'create-option') { // Add in default namespace, etc. From 7551840bc6eea9a574660622f18c65e7aeccc4ff Mon Sep 17 00:00:00 2001 From: Martin Morales Date: Tue, 28 Mar 2023 15:55:18 -0500 Subject: [PATCH 2/3] Added EntityPicker tests for required/non-required free solo/non-free solo instances Signed-off-by: Martin Morales --- .../fields/EntityPicker/EntityPicker.test.tsx | 394 +++++++++++++++++- .../fields/EntityPicker/EntityPicker.tsx | 2 +- 2 files changed, 393 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx index 937ff7d07a..d71e6cb776 100644 --- a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx @@ -19,13 +19,13 @@ import { Entity } from '@backstage/catalog-model'; import { CatalogApi, catalogApiRef } from '@backstage/plugin-catalog-react'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { FieldProps } from '@rjsf/core'; -import { fireEvent } from '@testing-library/react'; +import { fireEvent, screen } from '@testing-library/react'; import React from 'react'; import { EntityPicker } from './EntityPicker'; import { EntityPickerProps } from './schema'; const makeEntity = (kind: string, namespace: string, name: string): Entity => ({ - apiVersion: 'backstage.io/v1beta1', + apiVersion: 'scaffolder.backstage.io/v1beta3', kind, metadata: { namespace, name }, }); @@ -332,4 +332,394 @@ describe('', () => { expect(onChange).toHaveBeenCalledWith('group:default/team-b'); }); }); + + describe('Required EntityPicker', () => { + beforeEach(() => { + uiSchema = { + 'ui:options': { + catalogFilter: [ + { + kind: ['Group'], + 'metadata.name': 'test-entity', + }, + { + kind: ['User'], + 'metadata.name': 'test-entity', + }, + ], + }, + }; + props = { + onChange, + schema, + required: true, + uiSchema, + rawErrors, + formData, + } as unknown as FieldProps; + + catalogApi.getEntities.mockResolvedValue({ items: entities }); + }); + + it('User enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: '' } }); + fireEvent.blur(input); + + expect(input).toHaveValue(''); + }); + + it('User selects item', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: 'team-a' } }); + fireEvent.blur(input); + + expect(input).toHaveValue('team-a'); + expect(onChange).toHaveBeenCalledWith('team-a'); + }); + + it('User selects item and enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + // Open the Autocomplete dropdown + const input = screen.getByRole('textbox'); + fireEvent.click(input); + + // Select an option from the dropdown + fireEvent.change(input, { target: { value: 'team-a' } }); + + // Close the dropdown by clicking outside the Autocomplete component + const outside = screen.getByTestId('outside'); + fireEvent.mouseDown(outside); + + // Click back into the Autocomplete component + fireEvent.click(input); + + // Verify that the selected option is displayed in the input + expect(input).toHaveValue('team-a'); + + // Click the Clear button to clear the input + const clearButton = screen.getByLabelText('Clear'); + fireEvent.click(clearButton); + + // Verify that the input is empty + expect(input).toHaveValue(''); + + // Verify that the handleChange function was called with null + expect(onChange).toHaveBeenCalledWith(undefined); + }); + }); + + describe('Optional EntityPicker', () => { + beforeEach(() => { + uiSchema = { + 'ui:options': { + catalogFilter: [ + { + kind: ['Group'], + 'metadata.name': 'test-entity', + }, + { + kind: ['User'], + 'metadata.name': 'test-entity', + }, + ], + }, + }; + props = { + onChange, + schema, + required: false, + uiSchema, + rawErrors, + formData, + } as unknown as FieldProps; + + catalogApi.getEntities.mockResolvedValue({ items: entities }); + }); + + it('User enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: '' } }); + fireEvent.blur(input); + + expect(input).toHaveValue(''); + }); + + it('User selects item', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: 'team-a' } }); + fireEvent.blur(input); + + expect(input).toHaveValue('team-a'); + expect(onChange).toHaveBeenCalledWith('team-a'); + }); + + it('User selects item and enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + // Open the Autocomplete dropdown + const input = screen.getByRole('textbox'); + fireEvent.click(input); + + // Select an option from the dropdown + fireEvent.change(input, { target: { value: 'team-a' } }); + + // Close the dropdown by clicking outside the Autocomplete component + const outside = screen.getByTestId('outside'); + fireEvent.mouseDown(outside); + + // Click back into the Autocomplete component + fireEvent.click(input); + + // Verify that the selected option is displayed in the input + expect(input).toHaveValue('team-a'); + + // Click the Clear button to clear the input + const clearButton = screen.getByLabelText('Clear'); + fireEvent.click(clearButton); + + // Verify that the input is empty + expect(input).toHaveValue(''); + + // Verify that the handleChange function was called with null + expect(onChange).toHaveBeenCalledWith(undefined); + }); + }); + + describe('Required Free Solo', () => { + beforeEach(() => { + uiSchema = { + 'ui:options': { + catalogFilter: [ + { + kind: ['Group'], + 'metadata.name': 'test-entity', + }, + { + kind: ['User'], + 'metadata.name': 'test-entity', + }, + ], + }, + allowArbitraryValues: true, + }; + props = { + onChange, + schema, + required: true, + uiSchema, + rawErrors, + formData, + } as unknown as FieldProps; + + catalogApi.getEntities.mockResolvedValue({ items: entities }); + }); + + it('User enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: '' } }); + fireEvent.blur(input); + + expect(input).toHaveValue(''); + }); + + it('User selects item', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: 'team-a' } }); + fireEvent.blur(input); + + expect(input).toHaveValue('team-a'); + expect(onChange).toHaveBeenCalledWith('team-a'); + }); + + it('User selects item and enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + // Open the Autocomplete dropdown + const input = screen.getByRole('textbox'); + fireEvent.click(input); + + // Select an option from the dropdown + fireEvent.change(input, { target: { value: 'team-a' } }); + + // Close the dropdown by clicking outside the Autocomplete component + const outside = screen.getByTestId('outside'); + fireEvent.mouseDown(outside); + + // Click back into the Autocomplete component + fireEvent.click(input); + + // Verify that the selected option is displayed in the input + expect(input).toHaveValue('team-a'); + + // Click the Clear button to clear the input + const clearButton = screen.getByLabelText('Clear'); + fireEvent.click(clearButton); + + // Verify that the input is empty + expect(input).toHaveValue(''); + + // Verify that the handleChange function was called with null + expect(onChange).toHaveBeenCalledWith(undefined); + }); + }); + + describe('Optional Free Solo', () => { + beforeEach(() => { + uiSchema = { + 'ui:options': { + catalogFilter: [ + { + kind: ['Group'], + 'metadata.name': 'test-entity', + }, + { + kind: ['User'], + 'metadata.name': 'test-entity', + }, + ], + }, + allowArbitraryValues: true, + }; + props = { + onChange, + schema, + required: false, + uiSchema, + rawErrors, + formData, + } as unknown as FieldProps; + + catalogApi.getEntities.mockResolvedValue({ items: entities }); + }); + + it('User enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: '' } }); + fireEvent.blur(input); + + expect(input).toHaveValue(''); + }); + + it('User selects item', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.change(input, { target: { value: 'team-a' } }); + fireEvent.blur(input); + + expect(input).toHaveValue('team-a'); + expect(onChange).toHaveBeenCalledWith('team-a'); + }); + + it('User selects item and enters clear input', async () => { + await renderInTestApp( + + +
Outside
+ , + ); + + // Open the Autocomplete dropdown + const input = screen.getByRole('textbox'); + fireEvent.click(input); + + // Select an option from the dropdown + fireEvent.change(input, { target: { value: 'team-a' } }); + + // Close the dropdown by clicking outside the Autocomplete component + const outside = screen.getByTestId('outside'); + fireEvent.mouseDown(outside); + + // Click back into the Autocomplete component + fireEvent.click(input); + + // Verify that the selected option is displayed in the input + expect(input).toHaveValue('team-a'); + + // Click the Clear button to clear the input + const clearButton = screen.getByLabelText('Clear'); + fireEvent.click(clearButton); + + // Verify that the input is empty + expect(input).toHaveValue(''); + + // Verify that the handleChange function was called with null + expect(onChange).toHaveBeenCalledWith(undefined); + }); + }); }); diff --git a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx index e19e27641c..0f698919d5 100644 --- a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.tsx @@ -96,7 +96,7 @@ export const EntityPicker = (props: EntityPickerProps) => { (_: any, ref: string | Entity | null, reason: AutocompleteChangeReason) => { // ref can either be a string from free solo entry or if (typeof ref !== 'string') { - // if ref does not exist: pass 'undefined' to trigger validation for non-free solo entries + // if ref does not exist: pass 'undefined' to trigger validation for required value onChange(ref ? stringifyEntityRef(ref as Entity) : undefined); } else { if (reason === 'blur' || reason === 'create-option') { From 85067e381755997893e51000575c700eeba1bb6c Mon Sep 17 00:00:00 2001 From: Martin Morales Date: Tue, 4 Apr 2023 09:45:37 -0500 Subject: [PATCH 3/3] Updated comments Signed-off-by: Martin Morales --- .../components/fields/EntityPicker/EntityPicker.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx index d71e6cb776..598616aaa3 100644 --- a/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/EntityPicker/EntityPicker.test.tsx @@ -425,7 +425,7 @@ describe('', () => { // Verify that the input is empty expect(input).toHaveValue(''); - // Verify that the handleChange function was called with null + // Verify that the handleChange function was called with undefined expect(onChange).toHaveBeenCalledWith(undefined); }); }); @@ -522,7 +522,7 @@ describe('', () => { // Verify that the input is empty expect(input).toHaveValue(''); - // Verify that the handleChange function was called with null + // Verify that the handleChange function was called with undefined expect(onChange).toHaveBeenCalledWith(undefined); }); }); @@ -620,7 +620,7 @@ describe('', () => { // Verify that the input is empty expect(input).toHaveValue(''); - // Verify that the handleChange function was called with null + // Verify that the handleChange function was called with undefined expect(onChange).toHaveBeenCalledWith(undefined); }); }); @@ -718,7 +718,7 @@ describe('', () => { // Verify that the input is empty expect(input).toHaveValue(''); - // Verify that the handleChange function was called with null + // Verify that the handleChange function was called with undefined expect(onChange).toHaveBeenCalledWith(undefined); }); });