Validation messages

[nicolasazrak]

Is it possible to get error messages? When an object doesn't pass validation I would like to get errors messages to display, for example:

var  rules = {
  name: [ new Validator.Assert().Length( { min: 10 } ) ],
  email: [ new Validator.Assert().NotBlank() ],
  lastname: [new Assert().IsString()]
};

var model = {
  name: 'Mike',
  lastname: 'Perez'
};

var constraint = new Constraint(rules)
constraint.check(model).messages

and get

{
  name: ['Length should be greater than 10'],
  email: ['This field is required']
}

[cuteboi]

The custom messages seem to work with i18n at least. Shouldn't the message be inside the options object so that it can be dynamic and descriptive of the text being used? Also typos/spelling mistake with "constraint" vs your "contraint". Also, using default English as the default message language makes this API a lot more restrictive and forcing everyone to define a language before hand, instead of assigning a custom message after the fact.

Also coding style for @guillaumepotier is a little different, and you should follow it (spaces after the if( comparison ) { vs your if(comparison){. A lot of line breaks. Just saying there is some consistency issues.

He may reject it, but I am certainly going to use your idea and apply a more i18n friendly pattern for my exported private fork, since this project owner isn't actively responding on this thread.

[nicolasazrak]

Thanks for the feedback. About coding style I thought I've use it all the times but I must forgotten in some places, I'll fix it in the next few days. Typo was because English isn't my native language, can be fixed, too.

About messages, I used English but I thought most people is going to change them anyway and use a custom instead. How would you use it in options? Would it be something like this?

var options = {
  "messages": {
    "Required": "This field is required",
    "NotNull": "This field can not be null"
  }
}

var constraint = new Constraint(rules, options);

The downside of this approach is that if you have multiple fields with the same validator you can't use different messages. For example you have name and age, you might want to have as messages, "Please complete your name" and "Please complete your age"

[cuteboi]

That format would make it i18n compatible. If you think about it, the developer/end user can load up many language files that way, and switch them out, or have no messages by default. A default would be nice for the end user to base it off of.

But wording would be fine for the end user to change, which is very handy with what you're specifying.

[guillaumepotier]

I there,

Sorry for not answering in a timely fashion. I'm quite uncomfortable with this thread, since I do not see clearly myself how deep I want to implement i18n in this lib, since i18n have always been a tricky matter.

In your PR @nicolasazrak I'm not comfortable with the API, and the message we have to cart around in every Assert / Constraint / Validation..

Why do not implement a validator.i18n factory that would analyse violations and map them to error messages? And btw, +1 for @cuteboi suggestion

WDYT?

[cuteboi]

@guillaumepotier , That's a LOT cleaner. I didn't think of that, I was doing it as an option for i18n to the constructor, if a string present, return. But your method of just Running the results of Assert through the validator itself is sooo much cleaner. I noticed that you return some duplicates for certain errors, because they are shared in other Validations, so the wording has to be generic like I had suggested. Fieldname being included in the message wouldn't be the best way to go, but implementing the i18n:

for example, the Validate an Object example

var stringList = {
  // Generic must_be_a_string for
  must_be_a_string: "must be a string"
  Email: {
    // Specific to the this.__class__ mentioned in the Violation
    must_be_a_string: "email must be a string",
    default: "email is invalid";
  },
  Length: {
    must_be_a_string_or_array: "must be a String or an Array"
    default: function(violation){
    if(!violation)return;
    return "must be between " + violation.min + " and " + violation.max;
    }
  },  
};
validator.i18n(stringList, validator.validate( object, constraint ));

could return:

{ email: "email is invalid", firstname: "must be a string" }

Thanks for the +1 ;)

[guillaumepotier]

Yes this is something that suits me better. We should also work on an intelligent i18n method to replace violation values inside strings, to have an i18n file that looks like this one in my other project Parsley

[nicolasazrak]

I've made the update and added a message method. The api now is:

var  rules = {
  name: [ new Validator.Assert().Length( { min: 10 } ).Message( 'The name is required' ) ],
  email: [ new Validator.Assert().NotBlank() ],
  lastname: [ new Assert().IsString() ]
};

var model = {
  name: 'Mike',
  lastname: 'Perez'
};

var violations = new Constraint( rules ).check( model );
validator.i18n( violations, {
  NotBlank: function( violation ) { return 'This value cannot be blank'; },
  IsString: 'Must be a string'
} );

/* Would return */
{
  name: [ 'The name is required' ],
  email: [ 'This value cannot be blank' ]
}

The messages can be specified in the constraint, or in the i18n method, that accepts functions or string values.

[pgom]

Hey @guillaumepotier, I believe that the i18n feature for constraints were dropped along the way. This would extremely helpful, IMO. Any chance that this could be re-added anytime soon? Should I jump in and create a PR with feature in mind?

[guillaumepotier]

Hi @pgom

That's right, it's been some time I haven't been here, and this topic went idle.

As of today, I'm still convinced that this should be an external new object/method maybe in a different optional file (to avoid shipping it systematically with main validatorjs core) (like discussed above) and we should use default english string file like in Parsley (with sprintf equiv to replace min/max placeholder values in the i18n strings).

These messages of course should be replaced globally AND specifically (eg: field by field, to replace "This value should not be blank" by "You must enter your name")

I'd be glad to assist you in a PR as a reviewer, but not much more as my time is currently very short :)

[pgom]

Thanks for getting back @guillaumepotier. I agree with you in not having the core have such a file but getting the possibility of defining a specific label by field (even if the constraint is the same) would make much sense for this. I'll try to get around a solution on my own and when I feel it will be good enough, I'll happily create a PR with it.