Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(select/radio): Accept just the values in options (plus '' and null for backward-compatibility) #18

Merged
merged 19 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,18 @@ function checkIfConditionMatches(node, formValues, formFields) {
return currentProperty.enum.includes(value);
}

const { inputType } = getField(name, formFields);

return validateFieldSchema({ ...currentProperty, inputType, required: true }, value);
const field = getField(name, formFields);

return validateFieldSchema(
{
options: field.options,
sandrina-p marked this conversation as resolved.
Show resolved Hide resolved
// @TODO/CODE SMELL. We are passing the property (raw field), but buildYupSchema() expected the output field.
...currentProperty,
inputType: field.inputType,
required: true,
},
value
);
});
}

Expand Down
315 changes: 210 additions & 105 deletions src/tests/createHeadlessForm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
schemaInputTypeRadioDeprecated,
schemaInputTypeRadio,
schemaInputTypeRadioRequiredAndOptional,
schemaInputRadioOptionalNull,
schemaInputRadioOptionalConventional,
schemaInputTypeRadioOptionsWithDetails,
schemaInputTypeSelectSoloDeprecated,
schemaInputTypeSelectSolo,
Expand Down Expand Up @@ -420,6 +422,37 @@ describe('createHeadlessForm', () => {
});

describe('field support', () => {
function assertOptionsAllowed({ handleValidation, fieldName, validOptions }) {
sandrina-p marked this conversation as resolved.
Show resolved Hide resolved
const validateForm = (vals) => friendlyError(handleValidation(vals));

// All allowed options are valid
validOptions.forEach((value) => {
expect(validateForm({ [fieldName]: value })).toBeUndefined();
});

// Any other arbitrary value is not valid.
expect(validateForm({ [fieldName]: 'blah-blah' })).toEqual({
[fieldName]: 'The option "blah-blah" is not valid.',
});

// Given undefined, it says it's a required field.
expect(validateForm({})).toEqual({
[fieldName]: 'Required field',
});

// As required field, empty string ("") is also considered empty. @BUG RMT-518
// Expectation: The error to be "The option '' is not valid."
expect(validateForm({ [fieldName]: '' })).toEqual({
[fieldName]: 'Required field',
});

// As required field, null is also considered empty @BUG RMT-518
// Expectation: The error to be "The option null is not valid."
expect(validateForm({ [fieldName]: null })).toEqual({
[fieldName]: 'Required field',
});
}

it('support "text" field type', () => {
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeText);

Expand Down Expand Up @@ -522,38 +555,43 @@ describe('createHeadlessForm', () => {
});

it('support "select" field type @deprecated', () => {
const result = createHeadlessForm(schemaInputTypeSelectSoloDeprecated);
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeSelectSoloDeprecated);

expect(result).toMatchObject({
fields: [
{
description: 'Life Insurance',
label: 'Benefits (solo)',
name: 'benefits',
placeholder: 'Select...',
type: 'select',
options: [
{
label: 'Medical Insurance',
value: 'Medical Insurance',
},
{
label: 'Health Insurance',
value: 'Health Insurance',
},
{
label: 'Travel Bonus',
value: 'Travel Bonus',
},
],
},
],
expect(fields).toMatchObject([
{
description: 'Life Insurance',
label: 'Benefits (solo)',
name: 'benefits',
placeholder: 'Select...',
type: 'select',
options: [
{
label: 'Medical Insurance',
value: 'Medical Insurance',
},
{
label: 'Health Insurance',
value: 'Health Insurance',
},
{
label: 'Travel Bonus',
value: 'Travel Bonus',
},
],
},
]);

assertOptionsAllowed({
handleValidation,
fieldName: 'benefits',
validOptions: ['Medical Insurance', 'Health Insurance', 'Travel Bonus'],
});
});

it('support "select" field type', () => {
const result = createHeadlessForm(schemaInputTypeSelectSolo);
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeSelectSolo);

const fieldSelect = result.fields[0];
const fieldSelect = fields[0];
expect(fieldSelect).toMatchObject({
name: 'browsers',
label: 'Browsers (solo)',
Expand All @@ -576,6 +614,12 @@ describe('createHeadlessForm', () => {
});

expect(fieldSelect).not.toHaveProperty('multiple');

assertOptionsAllowed({
handleValidation,
fieldName: 'browsers',
validOptions: ['chr', 'ff', 'ie'],
});
});

it('supports "select" field type with multiple options @deprecated', () => {
Expand Down Expand Up @@ -666,96 +710,157 @@ describe('createHeadlessForm', () => {
});

it('support "radio" field type @deprecated', () => {
const result = createHeadlessForm(schemaInputTypeRadioDeprecated);
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeRadioDeprecated);

expect(result).toMatchObject({
fields: [
{
description: 'Do you have any siblings?',
label: 'Has siblings',
name: 'has_siblings',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: true,
schema: expect.any(Object),
type: 'radio',
},
],
});
expect(fields).toMatchObject([
{
description: 'Do you have any siblings?',
label: 'Has siblings',
name: 'has_siblings',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: true,
schema: expect.any(Object),
type: 'radio',
},
]);

const fieldValidator = result.fields[0].schema;
expect(fieldValidator.isValidSync('yes')).toBe(true);
expect(() => fieldValidator.validateSync('')).toThrowError('Required field');
assertOptionsAllowed({
handleValidation,
fieldName: 'has_siblings',
validOptions: ['yes', 'no'],
});
});
it('support "radio" field type', () => {
const result = createHeadlessForm(schemaInputTypeRadio);
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeRadio);

expect(result).toMatchObject({
fields: [
{
description: 'Do you have any siblings?',
label: 'Has siblings',
name: 'has_siblings',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: true,
schema: expect.any(Object),
type: 'radio',
},
],
});
expect(fields).toMatchObject([
{
description: 'Do you have any siblings?',
label: 'Has siblings',
name: 'has_siblings',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: true,
schema: expect.any(Object),
type: 'radio',
},
]);

const fieldValidator = result.fields[0].schema;
expect(fieldValidator.isValidSync('yes')).toBe(true);
expect(() => fieldValidator.validateSync('')).toThrowError('Required field');
assertOptionsAllowed({
handleValidation,
fieldName: 'has_siblings',
validOptions: ['yes', 'no'],
});
});

it('support "radio" optional field', () => {
const result = createHeadlessForm(schemaInputTypeRadioRequiredAndOptional);
const { fields, handleValidation } = createHeadlessForm(
schemaInputTypeRadioRequiredAndOptional
);
const validateForm = (vals) => friendlyError(handleValidation(vals));

expect(result).toMatchObject({
fields: [
{},
{
name: 'has_car',
label: 'Has car',
description: 'Do you have a car? (optional field, check oneOf)',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: false,
schema: expect.any(Object),
type: 'radio',
},
],
expect(fields).toMatchObject([
{},
{
name: 'has_car',
label: 'Has car',
description: 'Do you have a car? (optional field, check oneOf)',
options: [
{
label: 'Yes',
value: 'yes',
},
{
label: 'No',
value: 'no',
},
],
required: false,
schema: expect.any(Object),
type: 'radio',
},
]);

expect(
validateForm({
has_siblings: 'yes',
has_car: 'yes',
})
).toBeUndefined();
expect(validateForm({})).toEqual({
has_siblings: 'Required field',
});
});

describe('support "radio" optional field - more examples @BUG RMT-518', () => {
function assertCommonBehavior(validateForm) {
// Note: Very similar to assertOptionsAllowed()
// We could reuse it in a next iteration.

// Happy path
expect(validateForm({ has_car: 'yes' })).toBeUndefined();

// Accepts undefined field
expect(validateForm({})).toBeUndefined();

// Does not accept other values
expect(validateForm({ has_car: 'blah-blah' })).toEqual({
has_car: 'The option "blah-blah" is not valid.',
});

// Does not accept "null" as string
expect(validateForm({ has_car: 'null' })).toEqual({
has_car: 'The option "null" is not valid.',
});

// Accepts empty string ("") — @BUG RMT-518
// Expectation: Does not accept empty string ("")
expect(validateForm({ has_car: '' })).toBeUndefined();
}

it('normal optional (conventional way)', () => {
const { handleValidation } = createHeadlessForm(schemaInputRadioOptionalConventional);
const validateForm = (vals) => friendlyError(handleValidation(vals));

assertCommonBehavior(validateForm);

// Accepts null, even though it shoudln't @BUG RMT-518
// This is for cases where we (Remote) still have incorrect
// JSON Schemas in our Platform.
expect(validateForm({ has_car: null })).toBeUndefined();
// Expected:
// // Does NOT accept null value
// expect(validateForm({ has_car: null })).toEqual({
// has_car: 'The option null is not valid.',
// });
});

const fieldValidator = result.fields[0].schema;
expect(fieldValidator.isValidSync('yes')).toBe(true);
expect(() => fieldValidator.validateSync('')).toThrowError('Required field');
it('with null option (as Remote does)', () => {
const { handleValidation } = createHeadlessForm(schemaInputRadioOptionalNull);
const validateForm = (vals) => friendlyError(handleValidation(vals));

assertCommonBehavior(validateForm);

// Accepts null value
expect(validateForm({ has_car: null })).toBeUndefined();
});
});

it('support "radio" field type with extra info inside each option', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/tests/helpers.custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export const schemaInputTypeHidden = {
...mockSelectInputSolo,
title: 'Select hidden',
'x-jsf-presentation': { inputType: 'hidden' },
default: 'Travel Bonus',
default: 'chr',
},
a_hidden_select_multiple: {
...schemaInputTypeCountriesMultiple.properties.nationality,
Expand Down
Loading