xogroup / felicity

Javascript object constructors and sample data based on Joi schema.
Other
111 stars 9 forks source link

Felicity should default to pull valids and provide an optional override #118

Closed WesTyler closed 7 years ago

WesTyler commented 7 years ago

Per discussion in #116 - The default Felicity.example behavior should be to utilize values in a schema's valids array. To support bypassing that behavior (as when a schema has .allow(null) ), there should be an option passed to .example to ignore the valids, such as ignoreValids.

ignoreValids should default to false.

danielo515 commented 7 years ago

Can you think in any other scenario (despite the flag allow(null)) where you want to ignore the valids ? In my opinion, if allow null flag is present you should just push a null value into the valids array and pick a value randomly.

WesTyler commented 7 years ago

To me, .allow() is a way to say "these aren't the desired values, but don't error out if they are there". So I'd like a way to not include those in example() generation, even if it isn't by default.

danielo515 commented 7 years ago

And is that different from what I said ?

WesTyler commented 7 years ago

Nah, reading back over your comment I actually think we are saying the same thing. I'm working on this a bit this morning, so I may have something up soon. :)

danielo515 commented 7 years ago

I'm still don't understand why you need an ignore valids flag. Is it to generate an example that endured the .allow values are included ? If that is the case, I understand it. However I still thinking that allow values should be merged with the valids list for the rest of situations

WesTyler commented 7 years ago

both .allow() and .valid() put the values into the schema's valids array. The only difference is that .valid also sets the allowOnly flag. So I need ignoreValids for some use cases where we .allow(null) but do not want null in examples.

Joi.any().allow(null).describe()
// { type: 'any', valids: [ null ] }

Joi.any().valid(null).describe()
// { type: 'any', flags: { allowOnly: true }, valids: [ null ] }

Joi.any().valid('valid').allow('allowed').describe()
// { type: 'any', flags: { allowOnly: true }, valids: [ 'valid', 'allowed' ] }
danielo515 commented 7 years ago

Ok, I was mixing the valid method with the values on the valids array. What you say makes sense. However seems a bit odd that you only want to allow one single value but sets a property to allow another value. A bit non sense from my point of view ,but maybe there is room for something like that

WesTyler commented 7 years ago

In our case we have models that look something like

const schema = Joi.object.keys({
    name: Joi.string(),
    nickname: Joi.string().allow(null)
}).options({ presence: 'required' });

For the nickname property, we do not want to fail out the validation when null is passed in. But, for generating mocks for our tests (like seeding 100k records into a test DB) we may not want null set on nickname. So this lets us maintain the permissive schema while automatically excluding null from test mocks. :)

[EDIT]

seems a bit odd that you only want to allow one single value but sets a property to allow another value

Yeah, I agree. I don't know of anyone actually doing that, but the Joi API allows it and it demonstrates the value concatenation into the valids array ;)

danielo515 commented 7 years ago

Hello @WesTyler Your example makes a lot of sense, and as usual is very instructive.

Yeah, I agree. I don't know of anyone actually doing that, but the Joi API allows it and it demonstrates the value concatenation into the valids array ;)

Glad you agree. Then we conclude it is just possible.

Thanks for your patience!