Skip to content

Commit

Permalink
Sandrina's self-review
Browse files Browse the repository at this point in the history
  • Loading branch information
sandrina-p committed Jul 2, 2023
1 parent 2d2fdc6 commit 58e0efc
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
20 changes: 10 additions & 10 deletions src/tests/createHeadlessForm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ describe('createHeadlessForm', () => {
});

describe('field support', () => {
function assertOptionsValidation({ handleValidation, fieldName, validOptions }) {
function assertOptionsAllowed({ handleValidation, fieldName, validOptions }) {
const validateForm = (vals) => friendlyError(handleValidation(vals));

// All allowed options are valid
Expand All @@ -448,7 +448,7 @@ describe('createHeadlessForm', () => {
[fieldName]: 'Required field',
});

// As required field, empty string ("") is also considered empty. @BUG XXX-000
// As required field, empty string ("") is also considered empty. @BUG RMT-518
expect(
validateForm({
[fieldName]: '',
Expand Down Expand Up @@ -586,7 +586,7 @@ describe('createHeadlessForm', () => {
},
]);

assertOptionsValidation({
assertOptionsAllowed({
handleValidation,
fieldName: 'benefits',
validOptions: ['Medical Insurance', 'Health Insurance', 'Travel Bonus'],
Expand Down Expand Up @@ -620,7 +620,7 @@ describe('createHeadlessForm', () => {

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

assertOptionsValidation({
assertOptionsAllowed({
handleValidation,
fieldName: 'browsers',
validOptions: ['chr', 'ff', 'ie'],
Expand Down Expand Up @@ -738,7 +738,7 @@ describe('createHeadlessForm', () => {
},
]);

assertOptionsValidation({
assertOptionsAllowed({
handleValidation,
fieldName: 'has_siblings',
validOptions: ['yes', 'no'],
Expand Down Expand Up @@ -768,7 +768,7 @@ describe('createHeadlessForm', () => {
},
]);

assertOptionsValidation({
assertOptionsAllowed({
handleValidation,
fieldName: 'has_siblings',
validOptions: ['yes', 'no'],
Expand Down Expand Up @@ -814,7 +814,7 @@ describe('createHeadlessForm', () => {
});
});

describe('support "radio" optional field - more examples @BUG XXX-000', () => {
describe('support "radio" optional field - more examples @BUG RMT-518', () => {
function assertCommonBehavior(validateForm) {
// Happy path
expect(validateForm({ has_car: 'yes' })).toBeUndefined();
Expand All @@ -823,16 +823,16 @@ describe('createHeadlessForm', () => {
expect(validateForm({})).toBeUndefined();

// Does not accept other values
expect(validateForm({ has_car: 'jdsadjag' })).toEqual({
has_car: 'The option "jdsadjag" is not valid.',
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 XXX-000
// Accepts empty string ("") — @BUG RMT-518
// Expectation: Does not accept empty string ("")
expect(validateForm({ has_car: '' })).toBeUndefined();
}
Expand Down
4 changes: 2 additions & 2 deletions src/yupSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ const getOptionsAllowed = (field) => {
return optionsBroken;

/*
[1] @BUG XXX-000 - explanation
[1] @BUG RMT-518 - explanation
The "" (empty string) is to keep retrocompatibily with previous version.
The "" does NOT match the JSON Schema specs, as `oneOf` keyword does not allow "" value, ever.
The "" does NOT match the JSON Schema specs. In the specs the `oneOf` keyword does not allow "" value by default.
Preving its value in this PR#18 would be a major BREAKING CHANGE
because before any string was allowed but now only the options[].value are,
Expand Down

0 comments on commit 58e0efc

Please sign in to comment.