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

Nested whens don't work #632

Closed
mgartner opened this issue Apr 22, 2015 · 10 comments
Closed

Nested whens don't work #632

mgartner opened this issue Apr 22, 2015 · 10 comments
Assignees
Labels
bug Bug or defect feature New functionality or improvement
Milestone

Comments

@mgartner
Copy link
Contributor

Why doesn't this work in my schema:

var schema = Joi.object().keys({
  setting: Joi.alternatives().when('$auth.credentials.cond1', {
    is: true,
    then: Joi.number().integer().required().when('$auth.credentials.cond2', {
      is: true,
      then: Joi.invalid(100, 101)
    }),
    otherwise: Joi.forbidden()
  })
});

When $auth.credentials.cond1 is true, and setting is not provided in the input object, there is no validation error when there should be because it is required.

@DavidTPate
Copy link
Contributor

I think you're just using this incorrectly. If you look at any.when() you'll notice that for then and otherwise it uses these as an alternative schema whereas you are trying to treat them as a builder pattern style alternative.

So what your schema is saying is that the setting value when $auth.credentials.cond2 is true then it should validate against the schema Joi.invalid(100, 101) not against Joi.number().integer().required().invalid(100, 101). If you wanted to do that you would need to break out the schema Joi.number().integer().required() into its own variable and reference it in your then and otherwise.

@mgartner
Copy link
Contributor Author

Ahh thanks for clarifying the docs.

So I tried using alternatives.when, but that leads to problems with required fields as well. In the example below, setting does not end up being required when $auth.credentials.cond1 is true.

var schema = Joi.object().keys({
  setting: Joi.alternatives().when('$auth.credentials.cond1', {
    is: true,
    then: Joi.alternatives().when('$auth.credentials.cond2', {
      is: true,
      then: Joi.number().integer().invalid(100, 101).required(),
      otherwise: Joi.number().interger().required()
    }),
    otherwise: Joi.forbidden()
  })
});

I tried to fix this below, but I get the error Cannot merge with another type: number.

var schema = Joi.object().keys({
  setting: Joi.when('$auth.credentials.cond1', {
    is: true,
    then: Joi.when('$auth.credentials.cond2', {
      is: true,
      then: Joi.number().integer().invalid(100, 101).required(),
      otherwise: Joi.number().interger().required()
    }),
    otherwise: Joi.forbidden()
  })
});

Any advice on how I should get the functionallity I want?

@mgartner
Copy link
Contributor Author

Also, regarding your comment above about any.when(), in my experience this correctly ensures that setting is an integer, which contradicts what you said:

var schema = Joi.object().keys({
  setting: Joi.number().integer().when('$auth.credentials.cond1', {
    is: true,
    then: Joi.required(),
    otherwise: Joi.forbidden()
  })
});

@Marsup
Copy link
Collaborator

Marsup commented Apr 23, 2015

There might be a problem in the concat.

Using this :

var Joi = require('joi');

var schema = Joi.object().keys({
  setting: Joi.when('$auth.credentials.cond1', {
    is: true,
    then: Joi.when('$auth.credentials.cond2', {
      is: true,
      then: Joi.number().integer().invalid(100, 101).required(),
      otherwise: Joi.number().integer().required()
    }),
    otherwise: Joi.forbidden()
  })
});

var input = { setting: <whatever> };

Joi.validate(input, schema, { context: { auth: { credentials: { cond1: true, cond2: true }}}}, function(err, value) {
  console.log('err:', err);
  console.log('value:', value);
});

with the latest master works. Can you check if it covers your needs ?

@mgartner
Copy link
Contributor Author

@Marsup you're right - it works in master, but fails with the below error in the v6.2.0 release. Is there any timeline for cutting a new version with the fix?

/Users/marcus/workspace/joi-test/node_modules/joi/node_modules/hoek/lib/index.js:683
    throw new Error(msgs.join(' ') || 'Unknown error');
          ^
Error: Cannot merge with another type: number
    at Object.exports.assert (/Users/marcus/workspace/joi-test/node_modules/joi/node_modules/hoek/lib/index.js:683:11)
    at internals.Any.concat (/Users/marcus/workspace/joi-test/node_modules/joi/lib/any.js:134:10)
    at internals.Any.when (/Users/marcus/workspace/joi-test/node_modules/joi/lib/any.js:356:48)
    at Object.<anonymous> (/Users/marcus/workspace/joi-test/index.js:6:15)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)

@Marsup
Copy link
Collaborator

Marsup commented Apr 23, 2015

Can be tonight. Does that address your PR too ?

@mgartner
Copy link
Contributor Author

Awesome! 👏 Thanks for the help.

I'm not sure if this fix will provide a way for me to rename keys after validation like my PR does (or do something which is semantically equal). I'll look into that particular case and report back soon. This fix will definitely be fixing a specific case for me though, so thanks.

@Marsup Marsup self-assigned this Apr 23, 2015
@Marsup Marsup added bug Bug or defect request labels Apr 23, 2015
@Marsup Marsup added this to the 6.3.0 milestone Apr 23, 2015
@Marsup
Copy link
Collaborator

Marsup commented Apr 23, 2015

Fixed by 7867225.

@Marsup Marsup closed this as completed Apr 23, 2015
@Marsup
Copy link
Collaborator

Marsup commented Apr 23, 2015

Published. I'm still not convinced by the necessity of your PR, so don't expect this one anytime soon.

@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

4 participants