Skip to content

Commit 4675d36

Browse files
AlexandraBuzilasdirix
authored andcommitted
Do not filter out errors for oneOf enums
The errorsAt function, used to filter the errors that belong to a control, ignores the errors AJV produces for oneOf schemas. Most of these errors should not be shown in the UI (e.g. when the oneOf property does not match exactly one subschema), however the errors produced directly for oneOf enums are an exception. Validation errors for oneOf enum controls are valid and should be shown in the UI. Contributed on behalf of STMicroelectronics
1 parent 1e1ccad commit 4675d36

File tree

6 files changed

+130
-11
lines changed

6 files changed

+130
-11
lines changed

packages/core/src/reducers/core.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import {
4545
UPDATE_CORE,
4646
UpdateCoreAction,
4747
} from '../actions';
48-
import { createAjv, Reducer } from '../util';
48+
import { createAjv, isOneOfEnumSchema, Reducer } from '../util';
4949
import type { JsonSchema, UISchemaElement } from '../models';
5050

5151
export const validate = (
@@ -389,7 +389,11 @@ export const errorsAt =
389389

390390
return filter(errors, (error) => {
391391
// Filter errors that match any keyword that we don't want to show in the UI
392-
if (filteredErrorKeywords.indexOf(error.keyword) !== -1) {
392+
// but keep the errors for oneOf enums
393+
if (
394+
filteredErrorKeywords.indexOf(error.keyword) !== -1 &&
395+
!isOneOfEnumSchema(error.parentSchema)
396+
) {
393397
return false;
394398
}
395399
const controlPath = getControlPath(error);
@@ -400,12 +404,13 @@ export const errorsAt =
400404
// In the primitive case the error's data path is the same for all subschemas:
401405
// It directly points to the property defining the anyOf/oneOf.
402406
// The same holds true for errors on the array level (e.g. min item amount).
403-
// In contrast, this comparison must not be done for errors whose parent schema defines an object
407+
// In contrast, this comparison must not be done for errors whose parent schema defines an object or a oneOf enum,
404408
// because the parent schema can never match the property schema (e.g. for 'required' checks).
405409
const parentSchema: JsonSchema | undefined = error.parentSchema;
406410
if (
407411
result &&
408412
!isObjectSchema(parentSchema) &&
413+
!isOneOfEnumSchema(parentSchema) &&
409414
combinatorPaths.findIndex((p) => instancePath.startsWith(p)) !== -1
410415
) {
411416
result = result && isEqual(parentSchema, schema);

packages/core/src/testers/testers.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ import type {
3737
JsonSchema,
3838
UISchemaElement,
3939
} from '../models';
40-
import { deriveTypes, hasType, resolveSchema } from '../util';
40+
import {
41+
deriveTypes,
42+
hasType,
43+
isOneOfEnumSchema,
44+
resolveSchema,
45+
} from '../util';
4146

4247
/**
4348
* Constant that indicates that a tester is not capable of handling
@@ -353,11 +358,7 @@ export const isEnumControl = and(
353358
*/
354359
export const isOneOfEnumControl = and(
355360
uiTypeIs('Control'),
356-
schemaMatches(
357-
(schema) =>
358-
schema.hasOwnProperty('oneOf') &&
359-
(schema.oneOf as JsonSchema[]).every((s) => s.const !== undefined)
360-
)
361+
schemaMatches((schema) => isOneOfEnumSchema(schema))
361362
);
362363

363364
/**
@@ -517,7 +518,7 @@ export const isObjectArrayWithNesting = (
517518
if (val.anyOf || val.allOf) {
518519
return true;
519520
}
520-
if (val.oneOf && !isOneOfEnumControl(uischema, val, context)) {
521+
if (val.oneOf && !isOneOfEnumSchema(val)) {
521522
return true;
522523
}
523524
if (hasType(val, 'object')) {

packages/core/src/util/schema.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525

2626
import find from 'lodash/find';
27+
import { JsonSchema } from '../models';
2728

2829
export const getFirstPrimitiveProp = (schema: any) => {
2930
if (schema.properties) {
@@ -38,3 +39,11 @@ export const getFirstPrimitiveProp = (schema: any) => {
3839
}
3940
return undefined;
4041
};
42+
43+
/**
44+
* Tests whether the schema has an enum based on oneOf.
45+
*/
46+
export const isOneOfEnumSchema = (schema: JsonSchema) =>
47+
schema?.hasOwnProperty('oneOf') &&
48+
schema?.oneOf &&
49+
(schema.oneOf as JsonSchema[]).every((s) => s.const !== undefined);

packages/core/test/reducers/core.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,80 @@ test('errorAt filters array inner value', (t) => {
861861
t.deepEqual(filtered[0], state.errors[1]);
862862
});
863863

864+
test('errorAt filters oneOf enum', (t) => {
865+
const ajv = createAjv();
866+
const schema: JsonSchema = {
867+
type: 'object',
868+
properties: {
869+
oneOfEnum: {
870+
type: 'string',
871+
oneOf: [
872+
{ title: 'Number', const: '1' },
873+
{ title: 'Color', const: '2' },
874+
],
875+
},
876+
},
877+
};
878+
const data: { oneOfEnum: string } = { oneOfEnum: '3' };
879+
const v = ajv.compile(schema);
880+
const errors = validate(v, data);
881+
882+
const state: JsonFormsCore = {
883+
data,
884+
schema,
885+
uischema: undefined,
886+
errors,
887+
};
888+
const filtered = errorAt('oneOfEnum', schema.properties.oneOfEnum)(state);
889+
t.is(filtered.length, 1);
890+
// in the state, we get 3 errors: one for each const value in the oneOf (we have two consts here) and one for the oneOf property itself
891+
// we only display the last error on the control
892+
t.deepEqual(filtered[0], state.errors[2]);
893+
});
894+
895+
test('errorAt filters array with inner oneOf enum', (t) => {
896+
const ajv = createAjv();
897+
const schema = {
898+
type: 'object',
899+
properties: {
900+
parentArray: {
901+
type: 'array',
902+
items: {
903+
type: 'object',
904+
properties: {
905+
oneOfEnumWithError: {
906+
type: 'string',
907+
oneOf: [
908+
{ title: 'Number', const: '1' },
909+
{ title: 'Color', const: '2' },
910+
],
911+
},
912+
},
913+
},
914+
},
915+
},
916+
};
917+
const data: { parentArray: { oneOfEnumWithError: string }[] } = {
918+
parentArray: [{ oneOfEnumWithError: 'test' }],
919+
};
920+
const v = ajv.compile(schema);
921+
const errors = validate(v, data);
922+
const state: JsonFormsCore = {
923+
data,
924+
schema,
925+
uischema: undefined,
926+
errors,
927+
};
928+
const filtered = errorAt(
929+
'parentArray.0.oneOfEnumWithError',
930+
schema.properties.parentArray.items.properties.oneOfEnumWithError
931+
)(state);
932+
t.is(filtered.length, 1);
933+
// in the state, we get 3 errors: one for each const value in the oneOf (we have two consts here) and one for the oneOf property itself
934+
// we only display the last error on the control
935+
t.deepEqual(filtered[0], state.errors[2]);
936+
});
937+
864938
test('errorAt filters oneOf simple', (t) => {
865939
const ajv = createAjv();
866940
const schema: JsonSchema = {

packages/examples/src/examples/arrays.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ export const schema = {
4545
type: 'string',
4646
const: 'foo',
4747
},
48+
oneOfEnum: {
49+
type: 'string',
50+
oneOf: [{ const: 'foo' }, { const: 'bar' }],
51+
},
4852
},
4953
},
5054
},
@@ -95,6 +99,7 @@ export const data = {
9599
{
96100
date: new Date().toISOString().substr(0, 10),
97101
message: 'Get ready for booohay',
102+
oneOfEnum: 'test',
98103
},
99104
],
100105
};

packages/examples/src/examples/enum.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ export const schema = {
3535
type: 'string',
3636
enum: ['foo', 'bar'],
3737
},
38+
enumWithError: {
39+
type: 'string',
40+
enum: ['foo', 'bar'],
41+
},
3842
oneOfEnum: {
3943
type: 'string',
4044
oneOf: [
@@ -51,6 +55,14 @@ export const schema = {
5155
{ const: 'foobar', title: 'FooBar' },
5256
],
5357
},
58+
oneOfEnumWithError: {
59+
type: 'string',
60+
oneOf: [
61+
{ const: 'foo', title: 'Foo' },
62+
{ const: 'bar', title: 'Bar' },
63+
{ const: 'foobar', title: 'FooBar' },
64+
],
65+
},
5466
constEnum: {
5567
const: 'Const Value',
5668
},
@@ -90,6 +102,10 @@ export const uischema = {
90102
autocomplete: false,
91103
},
92104
},
105+
{
106+
type: 'Control',
107+
scope: '#/properties/enumWithError',
108+
},
93109
],
94110
},
95111
{
@@ -118,12 +134,21 @@ export const uischema = {
118134
autocomplete: false,
119135
},
120136
},
137+
{
138+
type: 'Control',
139+
scope: '#/properties/oneOfEnumWithError',
140+
},
121141
],
122142
},
123143
],
124144
};
125145

126-
export const data = { plainEnumSet: 'foo', oneOfEnumSet: 'bar' };
146+
export const data = {
147+
plainEnumSet: 'foo',
148+
enumWithError: 'bogus',
149+
oneOfEnumSet: 'bar',
150+
oneOfEnumWithError: 'bogus',
151+
};
127152

128153
registerExamples([
129154
{

0 commit comments

Comments
 (0)