Skip to content

Validate required array field #45

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

Closed
torstenrudolf opened this issue Jul 25, 2014 · 14 comments · Fixed by #63
Closed

Validate required array field #45

torstenrudolf opened this issue Jul 25, 2014 · 14 comments · Fixed by #63

Comments

@torstenrudolf
Copy link
Contributor

This is possibly related to issue #27 .

If you have an array of strings, the required flag is not validated.

The following will not be $invalid when the array is empty.

{
    "type": "object",
    "required": ["editors_used"],
    "properties": {
        "editors_used": {
            "type": "array",
            "title": "What editors do you use?",
            "minItems": 1,
            "items": {
                "type": "string",
                "enum": [
                    "vim", "emacs", "sublime", "kate"
                ]
            },
            "additionalItems": false
        }
    }
}
@davidlgj davidlgj added the bug label Jul 25, 2014
@davidlgj
Copy link
Contributor

The problem is in the scema-validate directive (schema-validate.js, row 26), it assumes any required is handled by ng-required, which of course is not the case with arrays. And therefore it never validates against the schema, it should probably check for presence of ng-required... marking it as a bug.

@torstenrudolf
Copy link
Contributor Author

@davidlgj do you think this change goes in the right direction (please ignore the un-minified dist file I added) ?
Was just quick try. Still doesn't solve the problem, when the checkboxes never where changed. But if you select one and then deselect all it will show you an error.

@davidlgj
Copy link
Contributor

Actually I don't know... I'm not sure what you're trying to do? Why a $watch?

I was thinking more along the lines of dropping the "ng-required" attribute on all inputs and instead re-implement it with the validation tv4js does. That would mean a slight difference in handling in modern browsers, but it would fix support for IE8 (which I need to fix next month or so anyway). tv4js does check for required so basically just removing this statement:

        //required is handled by ng-required
        if (angular.isUndefined(viewValue)) {
          return undefined;
        }

Should let it through. But alas if it was that easy, in draft 4 of json schema they moved the "required" attribute from the property to a list on the object (as you have written). In angular schema form we put it
on the form (so it can be overridden and so we can support both v3 and v4), so we have check the form, not the schema. This means that to validate we need to wrap our array value in an object and do the same with the schema snippet.

Something along this line: (warning, untested)

var result;
if (required === true) {
  result = tv4.validateResult({ wrapped: value },{ 
    type: "object",
    required: ['wrapped'],
     properties: { wrapped: schema}
  });
} else {
  result = tv4.validateResult(value,schema);
}

The second problem here is that we don't have the form definition (or rather info if the form is required or not), probably easiest to check an attribute for that. Maybe call it sf-schema-required :-) So basically replace ng-required with sf-schema-required.

As you might have noticed I haven't fixed any issues that have come the last two weeks, It's because I'm on vacation and don't have that much computer time, but basically from middle of august things should be back to normal and I'd have more time :-)

@torstenrudolf
Copy link
Contributor Author

Hey, thx David. Enjoy your holiday and don't spent because of me more time in front of the computer!

The problem is, that the parser never gets called for array fields. I think because the model of the array ($$value$$) is not the model of the individual checkboxes. Thats why I introduced the watch for the arrays.

@davidlgj
Copy link
Contributor

Hi @torstenrudolf,

I ended dropping reliance on ng-required and made it so tv4js validates required as well, it's in release 0.7.0. This plus validating on certain event now works with arrays. It's not that nice yet though, it does validate require and maxItems etc, but it doesn't stop you from pressing "Add" button, IMHO it should be disabled when you can't add more items.

@torstenrudolf
Copy link
Contributor Author

Thx @davidlgj

@torstenrudolf
Copy link
Contributor Author

I think this bug is not quite resolved, unfortunately.

You are right, it is working for tabarrays, but for the example above, with checkboxes, I can't see it working on the example page.

@jimmykane
Copy link

Is this resolved ? I am having this issue with latest stable.

@Anthropic
Copy link
Member

Anthropic commented Feb 18, 2017

@jimmykane it looks like it should be, have you tried with the alpha?
Actually it appears to work in the demo site, can you try again using the bootstrap decorator file from the bootstrap decorator repo rather than the bundled one if you are using bower?

@jimmykane
Copy link

@Anthropic Is alpha stable enough? We have our own decorators unfortunately and somehow I caught right now in the middle.
Might the bootstrap decorators (repo link?) work or should I test also against alpha?

@Anthropic
Copy link
Member

angular-schema-form-bootstrap repo is in this org, if you use the decorator file from there it is more recent than the bundled one in bower. Are you setting validate on render? If not it would only validate on submit.

@jimmykane
Copy link

@Anthropic Will check this out. Just saw some sf-new-array and the old one got deprecated. So let me check that.

@jimmykane
Copy link

AS far as I checked the following:

{
    "type": "object",
    "required": ["editors_used"],
    "properties": {
        "editors_used": {
            "type": "array",
            "title": "What editors do you use?",
            "minItems": 1,
            "items": {
                "type": "string",
                "enum": [
                    "vim", "emacs", "sublime", "kate"
                ]
            },
            "additionalItems": false
        }
    }
}

Will validatate for the whole form but not for the checkboxes. The form state is invalid but only after a change is triggered the checkboxes get the correct validity state

@jimmykane
Copy link

Here is the plnkr as well https://plnkr.co/edit/fjSB0WgkJ46GuQZW6sz8?p=preview

The problem is still there but not on alpha

Tested -> Alpha is ok
Tested -> Old .8 with new decorators not OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants