From 5eea8eee47bcebe03979bb8ec6bcbb1890ff828e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?laussel=20lo=C3=AFc?= Date: Thu, 9 Dec 2021 14:02:08 +0100 Subject: [PATCH 1/4] fix liveOmit and omitExtraData when additionnalProperties is set to False doesn't remove extra data --- packages/core/src/components/Form.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/Form.js b/packages/core/src/components/Form.js index 4e4c0ce655..e72d040cbf 100644 --- a/packages/core/src/components/Form.js +++ b/packages/core/src/components/Form.js @@ -198,7 +198,11 @@ export default class Form extends Component { if (typeof _obj[key] === "object") { let newPaths = paths.map(path => `${path}.${key}`); // If an object is marked with additionalProperties, all its keys are valid - if (_obj[key].__rjsf_additionalProperties && _obj[key].$name !== "") { + if ( + _obj[key].__rjsf_additionalProperties && + _obj[key].$name !== "" && + _obj[key].additionalProperties !== false + ) { acc.push(_obj[key].$name); } else { getAllPaths(_obj[key], acc, newPaths); From f0a96902b4cf4547519a4ab01363f73849beb2bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?laussel=20lo=C3=AFc?= Date: Thu, 9 Dec 2021 17:24:54 +0100 Subject: [PATCH 2/4] fix liveOmit and omitExtraData when additionnalProperties is set to False doesn't remove extra data --- packages/core/src/components/Form.js | 6 +----- packages/core/src/utils.js | 5 ++++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/Form.js b/packages/core/src/components/Form.js index e72d040cbf..4e4c0ce655 100644 --- a/packages/core/src/components/Form.js +++ b/packages/core/src/components/Form.js @@ -198,11 +198,7 @@ export default class Form extends Component { if (typeof _obj[key] === "object") { let newPaths = paths.map(path => `${path}.${key}`); // If an object is marked with additionalProperties, all its keys are valid - if ( - _obj[key].__rjsf_additionalProperties && - _obj[key].$name !== "" && - _obj[key].additionalProperties !== false - ) { + if (_obj[key].__rjsf_additionalProperties && _obj[key].$name !== "") { acc.push(_obj[key].$name); } else { getAllPaths(_obj[key], acc, newPaths); diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index cfc1ed73b0..67fc56892d 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -1038,7 +1038,10 @@ export function toPathSchema(schema, name = "", rootSchema, formData = {}) { return toPathSchema(_schema, name, rootSchema, formData); } - if (schema.hasOwnProperty("additionalProperties")) { + if ( + schema.hasOwnProperty("additionalProperties") && + schema.additionalProperties !== false + ) { pathSchema.__rjsf_additionalProperties = true; } From 48ecfb3fc7aad07fa73ed402e3e5d4e703afbb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?laussel=20lo=C3=AFc?= Date: Fri, 10 Dec 2021 16:54:40 +0100 Subject: [PATCH 3/4] add a dedicated test to validate the fix --- packages/core/test/Form_test.js | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/core/test/Form_test.js b/packages/core/test/Form_test.js index e94d98d009..197d953026 100644 --- a/packages/core/test/Form_test.js +++ b/packages/core/test/Form_test.js @@ -3193,6 +3193,45 @@ describe("Form omitExtraData and liveOmit", () => { }); }); + it("should remove extra data on change with omitExtraData=true and liveOmit=true if additionalProperties equal false", () => { + const omitExtraData = true; + const liveOmit = true; + const schema = { + type: "object", + properties: { + foo: { type: "string", additionalProperties: false }, + bar: { type: "string", additionalProperties: false }, + info: { + type: "object", + additionalProperties: false, + properties: { + name: { + type: "string", + additionalProperties: false, + }, + }, + required: ["name"], + }, + }, + additionalProperties: false, + }; + const formData = { + foo: "foo", + baz: "baz", + info: { majorVersion: "v1123", name: "foofoo" }, + }; + const { node } = createFormComponent({ + schema, + formData, + omitExtraData, + liveOmit, + }); + + Simulate.submit(node); + + expect(node.querySelectorAll(".errors li")).to.have.length.of(0); + }); + it("should rename formData key if key input is renamed in a nested object with omitExtraData=true and liveOmit=true", () => { const { node, onChange } = createFormComponent( { From 3f36ef311f01f57c5d668f62a63a887db928bb29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?laussel=20lo=C3=AFc?= Date: Tue, 14 Dec 2021 16:37:58 +0100 Subject: [PATCH 4/4] improve AJV error stack format to have a more human readable message --- packages/core/src/validate.js | 26 ++++++++++-- packages/core/test/ArrayField_test.js | 2 +- packages/core/test/Form_test.js | 14 +++---- packages/core/test/validate_test.js | 59 ++++++++++++++++++++------- 4 files changed, 76 insertions(+), 25 deletions(-) diff --git a/packages/core/src/validate.js b/packages/core/src/validate.js index f2bb6b9887..76770c6ffe 100644 --- a/packages/core/src/validate.js +++ b/packages/core/src/validate.js @@ -16,6 +16,7 @@ function createAjvInstance() { multipleOfPrecision: 8, schemaId: "auto", unknownFormats: "ignore", + verbose: true, }); // add custom formats @@ -145,16 +146,35 @@ function transformAjvErrors(errors = []) { } return errors.map(e => { - const { dataPath, keyword, message, params, schemaPath } = e; + const { dataPath, keyword, message, params, schemaPath, parentSchema } = e; let property = `${dataPath}`; - + // Get the latest element that represent the item + let paths = dataPath.split("."); + let currentProperty = paths.pop(); + let title = currentProperty; + //If schemaPath termine par currentProperty/name => parentSchema represent the current property + if ( + schemaPath.endsWith(`${currentProperty}/${keyword}`) && + parentSchema.title + ) { + title = parentSchema.title; + } else { + // otherwise we are a level upper e.g for required properties + if ( + parentSchema.hasOwnProperty("properties") && + parentSchema.properties.hasOwnProperty(currentProperty) && + parentSchema.properties[currentProperty].hasOwnProperty("title") + ) { + title = parentSchema.properties[currentProperty].title; + } + } // put data in expected format return { name: keyword, property, message, params, // specific to ajv - stack: `${property} ${message}`.trim(), + stack: `${title} ${message}`.trim(), schemaPath, }; }); diff --git a/packages/core/test/ArrayField_test.js b/packages/core/test/ArrayField_test.js index 2191cd584f..4338a9d48e 100644 --- a/packages/core/test/ArrayField_test.js +++ b/packages/core/test/ArrayField_test.js @@ -1026,7 +1026,7 @@ describe("ArrayField", () => { params: { limit: 3 }, property: ".multipleChoicesList", schemaPath: "#/properties/multipleChoicesList/minItems", - stack: ".multipleChoicesList should NOT have fewer than 3 items", + stack: "multipleChoicesList should NOT have fewer than 3 items", }, ]); }); diff --git a/packages/core/test/Form_test.js b/packages/core/test/Form_test.js index 197d953026..554c87635d 100644 --- a/packages/core/test/Form_test.js +++ b/packages/core/test/Form_test.js @@ -1730,7 +1730,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 8 }, property: ".level1.level2", schemaPath: "#/properties/level1/properties/level2/minLength", - stack: ".level1.level2 should NOT be shorter than 8 characters", + stack: "level2 should NOT be shorter than 8 characters", }, ]); }); @@ -1832,7 +1832,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 4 }, property: ".level1[1]", schemaPath: "#/properties/level1/items/minLength", - stack: ".level1[1] should NOT be shorter than 4 characters", + stack: "level1[1] should NOT be shorter than 4 characters", }, { message: "should NOT be shorter than 4 characters", @@ -1840,7 +1840,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 4 }, property: ".level1[3]", schemaPath: "#/properties/level1/items/minLength", - stack: ".level1[3] should NOT be shorter than 4 characters", + stack: "level1[3] should NOT be shorter than 4 characters", }, ]); }); @@ -1894,7 +1894,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 4 }, property: ".outer[0][1]", schemaPath: "#/properties/outer/items/items/minLength", - stack: ".outer[0][1] should NOT be shorter than 4 characters", + stack: "outer[0][1] should NOT be shorter than 4 characters", }, { message: "should NOT be shorter than 4 characters", @@ -1902,7 +1902,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 4 }, property: ".outer[1][0]", schemaPath: "#/properties/outer/items/items/minLength", - stack: ".outer[1][0] should NOT be shorter than 4 characters", + stack: "outer[1][0] should NOT be shorter than 4 characters", }, ]); }); @@ -1955,7 +1955,7 @@ describeRepeated("Form common", createFormComponent => { params: { limit: 4 }, property: "[1].foo", schemaPath: "#/items/properties/foo/minLength", - stack: "[1].foo should NOT be shorter than 4 characters", + stack: "foo should NOT be shorter than 4 characters", }, ]); }); @@ -2449,7 +2449,7 @@ describeRepeated("Form common", createFormComponent => { params: { format: "area-code" }, property: ".areaCode", schemaPath: "#/properties/areaCode/format", - stack: '.areaCode should match format "area-code"', + stack: 'areaCode should match format "area-code"', }, ]); }); diff --git a/packages/core/test/validate_test.js b/packages/core/test/validate_test.js index 9c11778643..379d8a5a30 100644 --- a/packages/core/test/validate_test.js +++ b/packages/core/test/validate_test.js @@ -171,8 +171,13 @@ describe("Validation", () => { pattern: "\\d+", type: "string", }, + datasetName: { + pattern: "\\d+", + type: "string", + title: "Name of the dataset", + }, }, - required: ["datasetId"], + required: ["datasetId", "datasetName"], type: "object", }, }, @@ -182,7 +187,7 @@ describe("Validation", () => { it("should return a validation error about meta schema when meta schema is not defined", () => { const errors = validateFormData( - { datasetId: "some kind of text" }, + { datasetId: "some kind of text", datasetName: "2" }, schema ); const errMessage = @@ -199,7 +204,33 @@ describe("Validation", () => { }); it("should return a validation error about formData", () => { const errors = validateFormData( - { datasetId: "some kind of text" }, + { datasetId: "some kind of text", datasetName: "2" }, + schema, + null, + null, + [metaSchemaDraft4] + ); + expect(errors.errors).to.have.lengthOf(1); + expect(errors.errors[0].stack).to.equal( + 'datasetId should match pattern "\\d+"' + ); + }); + it("should return a validation error with title attribute about formData", () => { + const errors = validateFormData( + { datasetId: "2", datasetName: "some kind of text" }, + schema, + null, + null, + [metaSchemaDraft4] + ); + expect(errors.errors).to.have.lengthOf(1); + expect(errors.errors[0].stack).to.equal( + 'Name of the dataset should match pattern "\\d+"' + ); + }); + it("should return a validation error with title attribute about formData required", () => { + const errors = validateFormData( + { datasetId: "2" }, schema, null, null, @@ -207,12 +238,12 @@ describe("Validation", () => { ); expect(errors.errors).to.have.lengthOf(1); expect(errors.errors[0].stack).to.equal( - '.datasetId should match pattern "\\d+"' + "Name of the dataset is a required property" ); }); it("should return a validation error about formData, when used with multiple meta schemas", () => { const errors = validateFormData( - { datasetId: "some kind of text" }, + { datasetId: "some kind of text", datasetName: "2" }, schema, null, null, @@ -220,7 +251,7 @@ describe("Validation", () => { ); expect(errors.errors).to.have.lengthOf(1); expect(errors.errors[0].stack).to.equal( - '.datasetId should match pattern "\\d+"' + 'datasetId should match pattern "\\d+"' ); }); }); @@ -253,7 +284,7 @@ describe("Validation", () => { expect(result.errors).to.have.lengthOf(1); expect(result.errors[0].stack).to.equal( - '.phone should match format "phone-us"' + 'phone should match format "phone-us"' ); }); @@ -277,7 +308,7 @@ describe("Validation", () => { expect(result.errors).to.have.lengthOf(1); expect(result.errors[0].stack).to.equal( - '.phone should match format "area-code"' + 'phone should match format "area-code"' ); }); }); @@ -481,7 +512,7 @@ describe("Validation", () => { params: { missingProperty: "foo" }, property: ".foo", schemaPath: "#/required", - stack: ".foo is a required property", + stack: "foo is a required property", }, ]); }); @@ -489,7 +520,7 @@ describe("Validation", () => { it("should render errors", () => { expect(node.querySelectorAll(".errors li")).to.have.length.of(1); expect(node.querySelector(".errors li").textContent).eql( - ".foo is a required property" + "foo is a required property" ); }); }); @@ -525,7 +556,7 @@ describe("Validation", () => { it("should render errors", () => { expect(node.querySelectorAll(".errors li")).to.have.length.of(1); expect(node.querySelector(".errors li").textContent).eql( - ".foo should NOT be shorter than 10 characters" + "foo should NOT be shorter than 10 characters" ); }); @@ -537,7 +568,7 @@ describe("Validation", () => { params: { limit: 10 }, property: ".foo", schemaPath: "#/properties/foo/minLength", - stack: ".foo should NOT be shorter than 10 characters", + stack: "foo should NOT be shorter than 10 characters", }, ]); }); @@ -783,7 +814,7 @@ describe("Validation", () => { params: { missingProperty: "foo" }, property: ".foo", schemaPath: "#/required", - stack: ".foo is a required property", + stack: "foo is a required property", }, ]); }); @@ -887,7 +918,7 @@ describe("Validation", () => { params: { pattern: "\\d+" }, property: ".datasetId", schemaPath: "#/properties/datasetId/pattern", - stack: '.datasetId should match pattern "\\d+"', + stack: 'datasetId should match pattern "\\d+"', }, ]); onError.resetHistory();