spiral-project / daybed

Form validation and data storage API
http://daybed.rtfd.org/
BSD 3-Clause "New" or "Revised" License
53 stars 8 forks source link

Fix group field validation #193

Closed leplatrem closed 10 years ago

leplatrem commented 10 years ago

I'm stuck with group fields, and I'm currently thinking of getting rid of them.

My first idea was just to allow grouping of fields, in a way that does not affect their individual validation at all.

For example, the following model :

{name: 'person',
 fields: [
    {name: 'age', type: 'int'},
    {description: 'address',
     type: 'group',
     fields: [{name: 'street', type: 'string'},
              {name: 'city', type: 'string'}]
    }
 ]
}

could validate this record

{age: 12, street: 'bakery', city: 'london'}

I must admit that the group field type I had in mind belongs more to the presentation layer (grouping widgets in forms) than the model layer.

The current implementation is completely wrong : it does not validate at all ! (see #189, #192)

I tried to hack around, but I cannot find I found a way to flatten the fields list (i.e. expending the fields at the model level with the ones from the group fields).

And if we validate in a dict (e.g. {age: 12, '': {street: 'bakery', city: 'london'}}), then there is no difference with the object field, which is well tested and rather intuitive...

Any idea ? Opposition to group field type removal ?

almet commented 10 years ago

Hmm, interesting. How one could test that the items of a list follow some semantics (for instance that they're integers)?

I may not understand completely how all this work, and I think that having a full fledged documentation about how do deal with this would be useful.

Also (might be related), I'm currently playing a bit with jsonschema, we might take some of their logic. See https://spacetelescope.github.io/understanding-json-schema/reference/array.html for instance.

leplatrem commented 10 years ago

What you mention here (a list of integers) was implemented in https://github.com/spiral-project/daybed/pull/161 and it has nothing to do with this PR :)

This one is about fixing the validation of groups of fields (see #11, #88)

Natim commented 10 years ago

All I am asking is please don't remove the group fields. We are using it for n1k0/kept and I was so happy to find it. I think we should do recursive validation for this and handle group field as if it was an anonymous model inside a model

leplatrem commented 10 years ago

You were happy to find it because it was the only field type whose validation was not working. See #192.

What you need for kept is object, and it already does recursive validation of anonymous models!

Natim commented 10 years ago

Can you please give me the model definition you are taking about? Le 9 août 2014 09:36, "Mathieu Leplatre" notifications@github.com a écrit :

You were happy to find it because it was the only field type whose validation was not working. See #192 https://github.com/spiral-project/daybed/issues/192.

What you need for kept is object, and it already does recursive validation of anonymous models!

— Reply to this email directly or view it on GitHub https://github.com/spiral-project/daybed/pull/193#issuecomment-51679944.

leplatrem commented 10 years ago

I merge and close this, since this PR is only a fix on existing code and should not introduce more confusion.

I will take the time to write some documentation of the fields.

@Natim the model for kept can be something like this :

   'tasks': {
       'type': 'list',
       'itemtype': 'object',
       'parameters': {
            'fields': [
                {'type': u'enum',
                 'name': u'status',
                 'choices': [u'todo', u'done']}
            ]
       }
   }