loopbackio / loopback-datasource-juggler

Connect Loopback to various Data Sources
http://www.loopback.io
Other
277 stars 364 forks source link

validation for unrequired properties bug #541

Closed drish closed 7 years ago

drish commented 9 years ago

Hi guys,

How should one validate a field only when it is sent ? Currently if we set a property to required:false and add a validation to its property we get a validation error on trying to create this model instance.

Example:

On customer.json

{
  "name": {
      "type": "string",
      "required": false 
 },
 "email": {
      "type": "string",
      "required": true
  },
  "password": {
      "type": "string",
      "required": true   
  }
} 

On validations.js

  customer.validatesFormatOf('name', {
    with: /^\D*$/,
    message: 'name should contain only letters'
  });

If we send only email and password in a request, we get a validation error, name : null, which is incorret because name is not required, it should be validated only when it is present.

Juggler version "loopback-datasource-juggler": "2.21.0"

Thanks in advance.

marcellodesales commented 9 years ago

There are no examples about data validation, only documentation text...

Please, include validations to the actual examples on the site...

thanks!

drish commented 9 years ago

New example on how to reproduce:

user.json:

{
  "name": {
      "type": "string"
  }
}

And a validation like:

user.validatesLengthOf('name', { min: 4, max: 50})

The validation will trigger everytime, even when the name is not sent.

A workaround for this is, instead of having validatesLengthOf, or whatever builtin validator, I used a custom one, and checked if the field exists.

  user.validate('name', function(err) {
    // check if name exists
    if (this.name) {
      if (this.name.length < 4) {
        err('Name is too short');
      }
      if (this.name.length > 50) {
        err('Name is too long');
      }
    }
  });

It is like the builtin validators automatically set validatesPresenceOf. validation should be triggered only when the field is sent, for not required fields, of course.

Morriz commented 8 years ago

what is the status of this...it's driving me nuts to see validations triggered on non required fields with undefined values sent

AndersonZacharyT commented 8 years ago

+1

jkvim commented 8 years ago

+1

Irvandoval commented 8 years ago

+1

Sergius411 commented 8 years ago

Check allowNull and allowBlank options in validators. example: Model.validatesNumericalityOf('property', {allowNull: true});

Morriz commented 8 years ago

Where is that documented?

On 28 apr. 2016, at 11:01, Sergius411 notifications@github.com wrote:

Check allowNull and allowBlank options in validators. example: Model.validatesNumericalityOf('property', {allowNull: true});

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/strongloop/loopback-datasource-juggler/issues/541#issuecomment-215358485

Sergius411 commented 8 years ago

I've just found this solution by myself (all validators call nullCheck and check for isBlank with those options) and i can not find any documentation for these options.

Morriz commented 8 years ago

@richardpringle, could you put that on the roadmap? That prevents quite some future pain I believe :)

richardpringle commented 8 years ago

@davidcheung, I believe that you should know where this lands on the roadmap?

I'm not sure if we have it documented in there, but maybe you could point @Morriz in the right direction.

@Morriz, from my understanding, the answer is "soon", I will try to see if @davidcheung can back that up.

davidcheung commented 8 years ago

@Morriz This definitely seems like a valid and common use case, yeah I believe the current solution is still the workaround provided above to use custom validation

@richardpringle but I don't have a roadmap on that either


or as above suggested by @Sergius411

User.validatesFormatOf('gender',
  {
    with: /[mf]/,
    message: {with: 'm or f only'},
    allowNull: true,
    allowBlank: true
  });
richardpringle commented 8 years ago

@bajtos, there are a few "+1s" on this issue. Should we bump up the priority? What do you think?

bajtos commented 8 years ago

@richardpringle I am fine with bumping up the priority. However, I don't think this is a bug, I'd say it's a missing feature to make the framework easier to use. Unless I am missing something? On the second thought, it would be great if somebody could summarize what needs to get changed before we can close this issue as resolved.

BTW validations provide two more settings if and unless that can be used like follows (note that I did not run the code myself, it may not work OOTB):

customer.validatesFormatOf('name', {
    with: /^\D*$/,
    message: 'name should contain only letters',
    if: function(inst) {
      var val = inst.name;
      return val !== undefined && val !== null && val !== '';
    }
  });

// this may work too, depending whether "name" is always
// converted to String for cases like booleans and numbers
customer.validatesFormatOf('name', {
    with: /^\D*$/,
    message: 'name should contain only letters',
    if: 'name' });
Amir-61 commented 8 years ago

Interesting... I ran into this GH issue while reviewing some community PRs and issues.

I don't think this is a bug, I'd say it's a missing feature to make the framework easier to use.

👍

If the intention of this missing feature and the functionalities of allowNull, allowBlank, if and unlessand ... not documented, they can be cause confusion for users. I propose to at least document these allowNull, allowBlank, if, unless in our doc to avoid more confusion. @bajtos thought?

I'll assign it to myself.

bajtos commented 8 years ago

I propose to at least document these allowNull, allowBlank, if, unless in our doc to avoid more confusion.

I think that's a great idea, let's do it!

bmaupin commented 3 years ago

If I'm reading the documentation correctly, validatesFormatOf has an allowNull property, but when I enable it, validation errors are thrown for blank and undefined values:

  Model.validatesFormatOf("name", {
    with: /^([a-z0-9-]+)$/,
    message: "Name contains invalid characters",
    allowNull: true,
  });

However, if I use allowBlank instead, it works, even though it doesn't seem to be documented for validatesFormatOf 🤔:

  Model.validatesFormatOf("name", {
    with: /^([a-z0-9-]+)$/,
    message: "Name contains invalid characters",
    allowBlank: true,
  });

https://apidocs.loopback.io/loopback-datasource-juggler/#validatable-validatesformatof