Loopback-datasource-juggler: validatesInclusionOf should not enforce null check

[bitmage]

Currently there is no way to enforce an enumeration, but allow undefined/null. Even putting them in the list of allowed options does nothing because a nullCheck() short circuits this:

https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/validations.js#L289

My suggestion is to simply return from the inclusion validator without doing any checks if the value is null or undefined. This decouples the inclusion/required validations and allows the app programmer to select which properties he wants.

Thoughts on whether this is the right way to go? Concerns for backwards compatibility? I'm happy to create a pull request if there's consensus about the approach.

Thanks!

[mrfelton]

+1

[raymondfeng]

Isn't it fixed at https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/validations.js#L289? Please reopen it otherwise.

[bitmage]

No. That's precisely the logic that I pointed out as being the problem.

For an enumeration, it _always_ checks to see if the record is null. I'm saying that it should not do this.

I don't think I have access to re-open. Do you mean to create a new ticket?

[mrfelton]

Pretty sure this issue shouldn't be closed. This is a real problem and makes most of the built in validators useless for non-required fields.

[jamesreimer]

This issue really should be reopened. I agree with mrfelton on this making validators useless for non-required fields. Obviously you can do your own validation to get around it, but you really shouldn't have to if the validators exist.

Edit: For those who come across this looking for an answer in the meantime, with a few extra steps you can implement validation for unrequired fields as follows...

User.validate('name', customValidator, {message: 'Bad name'});
function customValidator(err) {
    if (this.name) { // If value is not null or undefined
        // Insert your custom validation here. Call err() on failure.
    }
});

[wprater]

you can just pass in allowNull to allow them in addition to your exclusion list https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/validations.js#L608

Customer.validatesInclusionOf('gender', {
    in: ['Male', 'Female', 'Other'],
    allowNull: true
  });

[beeman]

Thanks @wprater, this works in the case where I want to validate an option zip code:

  // Ensure that the zipCode is either 5 or 5-4 chars long
  Person.validatesFormatOf('zip', {
    with: /(^\d{5}$)|(^\d{5}-\d{4}$)/,
    allowNull: true
  });

[jamesobd]

WHY is allowNull not documented? Can someone add it to the documentation for the validatesInclusionOf options here: https://apidocs.strongloop.com/loopback-datasource-juggler/#validatable-validatesinclusionof

[blackvirus18]

allowNull really should be added to the doc.

[superkhau]

@crandmck ^

[crandmck]

Added to http://loopback.io/doc/en/lb2/Validating-model-data.html#options-object and also added to the API docs.

[sabliao]

Even after I use allowNull: true, I still get an error about an optional field (that, if included, needs to be from a short list of valid values) if I don't have it as part of the payload, and the error that comes back is ["inclusion.blank"]. Is this because undefined isn't the same as null? If so, could we have allowNull also allow undefined?

[brawaga]

@sabliao, you can also use allowBlank: true, as I see in the code of loopback validations.js file.

[Caperious]

@sabliao thank you, your suggestion was the one i was looking for!