wrapp-archive / validate.js

Declarative validation written in javascript
http://validatejs.org/
178 stars 18 forks source link

Add new 'properties' validator which recurses into object. #16

Closed goodgravy closed 10 years ago

goodgravy commented 10 years ago

Implement support for nested objects (#7).

The approach I took was to create a properties validator which recursively invokes runValidations.

A way to do validations over arrays would be cool, but outside the scope. A similar approach should work for that, but it would be a bit more fiddly, I think.

goodgravy commented 10 years ago

This has problems when the constraints reference properties which don't exist in the input – e.g. passing in an empty object to be validates against nested constraints.

Working on a solution – will update PR.

goodgravy commented 10 years ago

Fixed the problem of missing properties missing in the input.

The semantics I went for are described in the 06b95ff4ec48133fbed73051d41f72c485159e01 commit message.

ansman commented 10 years ago

Sorry, I've been pretty bust this week, I'll take a look at this when I have time.

ansman commented 10 years ago

Ok, sorry for the delay. Do you have some high level examples of the input and output with this feature? Overall it looks pretty good, I'm glad someone actually took time to write tests :)

ansman commented 10 years ago

I'm also not sure about the dot syntax, I'm a bit torn here and I'm interested if you have any pros or cons one way or the other.

The other way to represent the errors would be this:

["Foo is not valid", {"attribute": "bar", errors: ["Bar is not valid"]}]

I don't really like that version either.

How are errors formatted using the dot syntax? Does foo.bar become Foo bar? Because that might cause some issues with errors sounding weird (Home postal is too short)

basghar commented 10 years ago

In my humble opinion following way of describing errors would be more useful. No? IF

var person = {
    firstName: String,
    lastName:String,
    address: {
        street: String,
        suburb: String,
        city: String,
        country: String
    }
}

THEN

var errors = {
    firstName : ['First Name is required'],
    address : {
        street : ['Street is required']
    }
}

Having an address in errors implicitly means address is invalid.

ansman commented 10 years ago

@basghar That won't work, address might have errors of its own:

var constraints = {
  address: {
    presence: true,
    someOtherValidator: true,
    properties: {
      street: {
        presence: true
      }
    }
  }
}
basghar commented 10 years ago

So how do you deal with root level object ? I mean even the root level object may need a object level validator ?

ansman commented 10 years ago

How do you mean, do you have an example?

basghar commented 10 years ago

Semantics should be same for both parent(which could be root) and a child object. So there may be two type of validators.

  1. Property level that validates a individual property.
  2. Object level that validates object as a whole.

Thinking of example.

basghar commented 10 years ago

Just for the sake of discussion... Most of validation are at properties level. Presence is special and it is not possible to have presence result and properties result at the same time.. isn't it?

This means either it will be

var errors = {
    firstName : ['First Name is required'],
    address : ['Address is required']
}

OR

var errors = {
    firstName : ['First Name is required'],
    address : {
        street : ['Street is required']
    }
}
basghar commented 10 years ago

I am not sure if its over-design but here is a proposal.

var errors = {
    properties: {
        firstName : {
            presence: 'First Name is required'
        },
        address : {
            properties: {
                street : {
                    presence: 'Street is required'
                }
            }
        }
    }
}

This would mean that min, max, range are separate validators not merged in one. This also means that now we have a consistent structure

var errors = {
    validatorName: validatorMessage, // or
    validatorName: {
        validatorName: validatorMessage
    }
}

In other words, validator can only test one aspect and thus only one message with an exception of compound validator properties.

Opinions please ?

basghar commented 10 years ago

Here is another proposal inspired from angular

var errors = {
    $errors: [], // object level 
    propertyName: {
        $errors: [] // property level.
    }
}

The pattern is nestable. Please if you think what I am suggesting, deviates too much from original design, let me know.

ansman commented 10 years ago

It is most definitely possible to have presence and properties at the same time.

One common example is that entering an address is optional but if you do the street name must follow a specific format.

Out of the examples you posted angular makes the most sense, though I don't want to break backward compatibility if possible.

ecolman commented 10 years ago

Bump.

goodgravy commented 10 years ago

My incentive has gone to push this forward – decided to go with JSON schema instead where nested validation is required.