lykmapipo / sails-hook-validation

Custom validation error messages for sails model with i18n support
104 stars 29 forks source link

undefined ValidationError field for waterline returned error. #6

Closed victorbadila closed 9 years ago

victorbadila commented 9 years ago

Hi, I'm not sure if this classifies as an issue. I have a Sails v.11.0 project and have installed this validation module. Seeing it not working as expected in my particular case (an update scenario) I went with the debugger and traced my problem to the patched 'update.js' in this module, line 42:

if (error.ValidationError) {
                        error.Errors =
                            validateCustom(model, error.ValidationError);
                    }

The error coming from Waterline after failed update does not have any ValidationError field. It basically has a 'details' field: "Details: MongoError: E11000 duplicate key error index: mydb.user.$email_1 dup key: { : \"a@a.com\" }\n"' and besides this no other relevant attribute that can be linked to a validator module. Am I missing something? As far as I can see my sails' waterline module is of the same version as the one listed on github.

lykmapipo commented 9 years ago

@victorbadila Are you using it inside sails or in waterline?

victorbadila commented 9 years ago

I'm using it inside a Sails controller. I found out however that in Sails v 0.11 you can call a 'validate' method from a static model on a certain entity, which validates against certains validators (is email, value in certain array, etc) but not against the database (uniqueness, for example). This method takes a callback in which an error does have a ValidationErrors field, something like this:

User.validate(specificUserObject, function validateCb(err) {
  // if there are validation errors, they will appear in ValidationErrors field of the err callback var
}
lykmapipo commented 9 years ago

@victorbadila may you please share with me your model and controller codes that rise the problem

victorbadila commented 9 years ago

Sure, here it is:

User:

module.exports = {
//...
attributes: {
  //..
  email: {type: 'email', unique:'true'}
  //..
}

validationMessages: {
  email: {unique: 'custom message'}
}
//...
}

Controller part:

//..
User.update(userId, {email: duplicateEmail}, function updateCb(err, updatedUser) {
  // ! here an error is returned but it does not contain any field 'ValidationErrors' or anything like it.  
});
//..
ctimoteo commented 9 years ago

I don't know if my hint fixes the problem,

but based on sailsjs documentation you shoud set "unique" attribute as boolean true, not string 'true'

please check sails models documentation here: http://sailsjs.org/#!/documentation/concepts/ORM/Attributes.html

Bye

lykmapipo commented 9 years ago

@ctimoteo :+1:

@victorbadila may you please check repo spec cause unique constraints has been handled and as far as it is they work perfect.

If @ctimoteo suggestions fix your problem, your may consider to close the issue

victorbadila commented 9 years ago

@lykmapipo My bad, I copied it wrong, it is actually the boolean true, as in email: {unique: true}, I've checked my repo now. @ctimoteo thanks for the observation. As a matter of fact the database (which is constructed by sails' waterline) does reject the duplicate value, so the db validation works as I have mentioned in my first post, it just doesn't return validation errors. I followed the flow right through the sails-hook-validation overwritten 'update' method, it seems to rely on certain properties of the err object. In my case those properties don't get populated for some reason, I will look further into this.

lykmapipo commented 9 years ago

@victorbadila custom error message will avaliable on the error object returned and can be obtained by key Errors i.e error.Errors is where you custom messages resides.

May you try to use full criteria definition on your update i.e

User
     .update({ id: userId }, { email: duplicateEmail }, function afterUpdate(error, updatedUser) {
                    // ! here you can retrieve custom errors on error.Errors

                     expect(error.Errors.email).to.exist;

                    expect(error.Errors.email[0].message)
                        .to.equal(User.validationMessages.email.unique);

                  ....

});

Also please check this spec it resemble your issue and it works fine.

Hope it help.

victorbadila commented 9 years ago

Hello, I have cloned the hook repo, run the tests and the ones concerning updates pass indeed. While further investigating my problem I found out certain aspects of it: 1) While running the application locally on a ubuntu vm environment with the User model binded to sails-disk database the hook works OK and the error returned in the callback indeed has the Errors attribute, just as described. 2) If I do the same as above but with the User model binded to a mongo database (it's intended connection for production mode) via sails-mongo adapter I don't get the Errors field. 3) If I do run the application on a windows environment (my current dev environment) I don't get the Errors field whatever connection I specify for the User model. My guess is that the problem might be somewhere at the waterline / database adapter level. At any rate thank you for all the support, if I find anything else that might shed some light on this I will post here.

victorbadila commented 9 years ago

Ok, I have finally tracked the problem to this issue: https://github.com/balderdashy/sails-mongo/issues/193 It seems to be a sails-mongo problem. Sorry for all the hassle :)

lykmapipo commented 9 years ago

@victorbadila should we close the issue?

victorbadila commented 9 years ago

Yes. Since it's actually a problem of sails-mongo I think we should close the issue.