Skip to content

Commit

Permalink
fix: 3260 by properly mapping missing required fields
Browse files Browse the repository at this point in the history
fix rjsf-team#3260

The required field errors weren't mapped to the fields they were associated to in the `ErrorSchema`
This mapping issue was fixed as follows:
- In `@rjsf/core`, updated the `Form` and `validate` tests to fix the mapping issue in the test data for the required fields
- In `@rjsf/validator-ajv6`, added additional tests for testing top-level and nested required field errors to verify things worked with ajv6
- In `@rjsf/validator-ajv8`, fixed the `transformRJSFValidationErrors()` function in the validator to look for `missingProperty: 'field'` information in the `params` property using that to properly map the error `property`
  - Replicated the tests made in `@rjsf/validator-ajv6` to this package to verify that the fix is working properly
- Updated the `CHANGELOG.md` accordingly
  • Loading branch information
heath-freenome committed Dec 22, 2022
1 parent e35f61a commit 6cd19af
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -37,6 +37,7 @@ should change the heading of the (upcoming) version to include a major version b

## @rjsf/validator-ajv8
- Updated the validator to use the `ErrorSchemaBuilder` in the `toErrorSchema()` function to simplify the implementation
- Updated the validator to properly map missing required field errors in the `ErrorSchema`, fixing [#3260](https://github.com/rjsf-team/react-jsonschema-form/issues/3260)

## Dev / docs / playground
- Fixed the documentation for `ArrayFieldItemTemplate` as part of the fix for [#3253](https://github.com/rjsf-team/react-jsonschema-form/issues/3253)
Expand Down
4 changes: 3 additions & 1 deletion packages/core/test/Form_test.js
Expand Up @@ -2220,7 +2220,9 @@ describeRepeated("Form common", (createFormComponent) => {
field1: {
__errors: ["must be number"],
},
__errors: ["must have required property 'field2'"],
field2: {
__errors: ["must have required property 'field2'"],
},
},
},
"root_field1"
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/validate_test.js
Expand Up @@ -618,7 +618,7 @@ describe("Validation", () => {
message: "must have required property 'foo'",
name: "required",
params: { missingProperty: "foo" },
property: "",
property: "foo",
schemaPath: "#/required",
stack: "must have required property 'foo'",
},
Expand Down Expand Up @@ -943,7 +943,7 @@ describe("Validation", () => {
message: "must have required property 'foo'",
name: "required",
params: { missingProperty: "foo" },
property: "",
property: "foo",
schemaPath: "#/required",
stack: "must have required property 'foo'",
},
Expand Down
65 changes: 65 additions & 0 deletions packages/validator-ajv6/test/validator.test.ts
Expand Up @@ -224,6 +224,71 @@ describe("AJV6Validator", () => {
]);
});
});
describe("Validating required fields", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
describe("formData is not provided at top level", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
};

const formData = { pass1: "a" };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(".pass2 is a required property");
});
it("should return an errorSchema", () => {
expect(errorSchema.pass2!.__errors).toHaveLength(1);
expect(errorSchema.pass2!.__errors![0]).toEqual(
"is a required property"
);
});
});
describe("formData is not provided for nested child", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
properties: {
nested: {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
},
},
};

const formData = { nested: { pass1: "a" } };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
".nested.pass2 is a required property"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.nested!.pass2!.__errors).toHaveLength(1);
expect(errorSchema.nested!.pass2!.__errors![0]).toEqual(
"is a required property"
);
});
});
});
describe("No custom validate function, single additionalProperties value", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
Expand Down
11 changes: 9 additions & 2 deletions packages/validator-ajv8/src/validator.ts
Expand Up @@ -203,15 +203,22 @@ export default class AJV8Validator<
): RJSFValidationError[] {
return errors.map((e: ErrorObject) => {
const { instancePath, keyword, message, params, schemaPath } = e;
const property = instancePath.replace(/\//g, ".");
let property = instancePath.replace(/\//g, ".");
let stack = `${property} ${message}`.trim();
if ("missingProperty" in params) {
property = property
? `${property}.${params.missingProperty}`
: params.missingProperty;
stack = message!;
}

// put data in expected format
return {
name: keyword,
property,
message,
params, // specific to ajv
stack: `${property} ${message}`.trim(),
stack,
schemaPath,
};
});
Expand Down
201 changes: 201 additions & 0 deletions packages/validator-ajv8/test/validator.test.ts
Expand Up @@ -254,6 +254,73 @@ describe("AJV8Validator", () => {
]);
});
});
describe("Validating required fields", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
describe("formData is not provided at top level", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
};

const formData = { pass1: "a" };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.pass2!.__errors).toHaveLength(1);
expect(errorSchema.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
describe("formData is not provided for nested child", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
properties: {
nested: {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
},
},
};

const formData = { nested: { pass1: "a" } };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.nested!.pass2!.__errors).toHaveLength(1);
expect(errorSchema.nested!.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
});
describe("No custom validate function, single additionalProperties value", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
Expand Down Expand Up @@ -695,6 +762,73 @@ describe("AJV8Validator", () => {
]);
});
});
describe("Validating required fields", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
describe("formData is not provided at top level", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
};

const formData = { pass1: "a" };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.pass2!.__errors).toHaveLength(1);
expect(errorSchema.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
describe("formData is not provided for nested child", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
properties: {
nested: {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
},
},
};

const formData = { nested: { pass1: "a" } };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.nested!.pass2!.__errors).toHaveLength(1);
expect(errorSchema.nested!.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
});
describe("No custom validate function, single additionalProperties value", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
Expand Down Expand Up @@ -1137,6 +1271,73 @@ describe("AJV8Validator", () => {
]);
});
});
describe("Validating required fields", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
describe("formData is not provided at top level", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
};

const formData = { pass1: "a" };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.pass2!.__errors).toHaveLength(1);
expect(errorSchema.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
describe("formData is not provided for nested child", () => {
beforeAll(() => {
const schema: RJSFSchema = {
type: "object",
properties: {
nested: {
type: "object",
required: ["pass1", "pass2"],
properties: {
pass1: { type: "string" },
pass2: { type: "string" },
},
},
},
};

const formData = { nested: { pass1: "a" } };
const result = validator.validateFormData(formData, schema);
errors = result.errors;
errorSchema = result.errorSchema;
});
it("should return an error list", () => {
expect(errors).toHaveLength(1);
expect(errors[0].stack).toEqual(
"must have required property 'pass2'"
);
});
it("should return an errorSchema", () => {
expect(errorSchema.nested!.pass2!.__errors).toHaveLength(1);
expect(errorSchema.nested!.pass2!.__errors![0]).toEqual(
"must have required property 'pass2'"
);
});
});
});
describe("No custom validate function, single additionalProperties value", () => {
let errors: RJSFValidationError[];
let errorSchema: ErrorSchema;
Expand Down

0 comments on commit 6cd19af

Please sign in to comment.