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

Incorrect validation of required child properties of optional properties? #2708

Closed
1 task
dieseldjango opened this issue Feb 11, 2022 · 9 comments
Closed
1 task
Labels
bug possibly close To confirm if this issue can be closed

Comments

@dieseldjango
Copy link
Contributor

Prerequisites

Description

Form validation fails if a required child property of an optional object property is missing.

Steps to Reproduce

Add the following property schema to the "Simple" example in the playground:

    "address": {
      "type": "object",
      "properties": {
        "street": {
          "type": "string"
        },
        "zip": {
          "type": "string"
        }
      },
      "required": ["zip"]
    }

Submit the form without entering any address data

Expected behavior

Because address is optional and I did not enter any address, validation should succeed and the produced form should have no address property.

Actual behavior

Validation fails because the required zip property is missing.

Version

3.2.1

@dieseldjango
Copy link
Contributor Author

I think this is related to #2669, which mentions pruning empty leaf objects before validation.

@jacqueswho jacqueswho added the bug label Feb 22, 2022
@devinsm
Copy link

devinsm commented Feb 24, 2022

Hi there! I would love to contribute a fix if no one has started working on this yet. I have forked the repo and gotten the playground running locally. This is my first time contributing to this repo, so any pointers are welcome.

@dieseldjango
Copy link
Contributor Author

Hi @devinsm I'm new to this repo too, so I can't provide any guidance. I can't use the package as-is without fixing this and need to deliver an alpha of my app very soon. Have you looked into the fix at all? Otherwise, I may poke around and see what I can figure out.

@dieseldjango
Copy link
Contributor Author

I think one problem in dealing with this will be to determine when an object really is empty and can be pruned. There is bound to be ambiguity, such as differentiating between an empty string and an undefined one, a false boolean vs. undefined, an empty array vs. undefined, etc. Also, with enum properties it will be necessary to provide an empty, '--' option so the value can be unset. (That really should be supported in general for any optional enum as well.)

I'm thinking to prune recursively starting at leaf objects. If the object is optional and all of its properties are empty or false then delete it.

@dieseldjango
Copy link
Contributor Author

It also looks like the whole way 'required' is handled needs reworking. The isRequired() method in ObjectField simply looks at the required property on the schema. The problem is that a 'required' property on an optional nested schema is not necessarily required. It would be better to simply rely on ajv validation so this code isn't basically repeating validation logic that's correctly implemented in ajv.

@epicfaace
Copy link
Member

epicfaace commented Mar 1, 2022 via email

dieseldjango added a commit to LansonLabs/react-jsonschema-form that referenced this issue Mar 7, 2022
dieseldjango added a commit to LansonLabs/react-jsonschema-form that referenced this issue Mar 7, 2022
@pkcpkc
Copy link

pkcpkc commented Mar 9, 2022

The "required" attribute is misinterpreted, when an NOT required element contains required element itself.

Checking out this example:
An object with "name" (obligatory) and "link" (mandatory); the editor does NOT accept leaving "link" empty, because its inner fields are obligatory.

{ "title": "My Test", "description": "My description", "type": "object", "properties": { "name": { "description": "The name of the menu item", "type": "string" }, "link": { "$ref": "#/definitions/link" } }, "additionalProperties": false, "required": [ "name" ], "definitions": { "target": { "enum": [ "article", "push", "link", "dataprivacy", "licenses", "stage" ] }, "link": { "type": "object", "properties": { "url": { "type": "string" }, "target": { "$ref": "#/definitions/target" } }, "required": [ "url", "target" ] } } }

Possible but ugly workaround:
"link": { "oneOf": [ {"type": "null"}, {"$ref": "#/definitions/link"} ] }

@stale
Copy link

stale bot commented Jun 8, 2023

This issue has been automatically marked as possibly close because it has not had recent activity. It will be closed if no further activity occurs. Please leave a comment if this is still an issue for you. Thank you.

@stale stale bot added the possibly close To confirm if this issue can be closed label Jun 8, 2023
@stale
Copy link

stale bot commented Jul 9, 2023

This issue was closed because of lack of recent activity. Reoopen if you still need assistance.

@stale stale bot closed this as completed Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug possibly close To confirm if this issue can be closed
Projects
None yet
Development

No branches or pull requests

5 participants