lykmapipo / sails-hook-validation

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

Issues with override #10

Closed NoRKin closed 9 years ago

NoRKin commented 9 years ago

Hi,

I'm not sure if it's a bug or me not doing the right thing. I haven't had much time looking for the root of my issue.

I have the latest version of sails 0.11.1, I couldn't make the override work with blueprint ( haven't tried without).

For my blueprint create, I managed to make it work by replacing line 51 of create.js.

Before:

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

After:

if (error.ValidationError) {
                        error.invalidAttributes = 
                            _.merge(error.invalidAttributes, validateCustom(model, error.ValidationError));
                    }

Are you overriding error message or "annotating" them?

lykmapipo commented 9 years ago

@NoRKin sails-hook-validation does not override the error object returned by sails methods instead it add Errors as a key to retrieve custom error messages from the error object.

If am getting you, To merge custom error messages into error.invalidAttributes you should consider to merge error.Errors which is the storage of the custom errors.

NoRKin commented 9 years ago

@lykmapipo thanks for the quick answer.

I explained myself terribly bad yesterday. I had time to investigate a bit more so here is a quick report.

The issue: The server will return a JSON that doesn't include error.Errors

Apparent root of the issue: When the error is sent back to the client, error.toJSON(); is called.

console.log(error.toJSON); gives

return {
    error: this.code,
    status: this.status,
    summary: this.reason,
    model: this.model,
    invalidAttributes: this.invalidAttributes
  };

That explains why error.Errors isn't returned in the final JSON and why merging invalidAttributes was working for me.

NoRKin commented 9 years ago
 if (error.ValidationError) {
                    error.Errors = (validateCustom(model, error.ValidationError));
                    error.toJSON = function() {
                        return {
                            error: this.code,
                            status: this.status,
                            summary: this.reason,
                            model: this.model,
                            invalidAttributes: this.invalidAttributes,
                            Errors: this.Errors
                        };
                    }
                }

Does the trick

lykmapipo commented 9 years ago

@NoRKin ok

NoRKin commented 9 years ago

Not sure why you are closing this issue, there is a legitimate bug in your library that your unit testing won't pick up since you are not serializing your error object to json in your tests.

lykmapipo commented 9 years ago

@NoRKin Your issue was related on how to make custom validation errors to be in your blueprint responses. That is not the intention of sails-hook-validation, which aim to give you a way to add custom validation error message so that you can use it as you want.

As with your blueprints or sails-blueprints, I expected custom implementation of blueprints that utilize sails-hook-validation as source of custom error messages.

May you please add the spec that will fail so that I can understand your concern and where the bug exist.

lykmapipo commented 9 years ago

@NoRKin Note that sails-hook-validation does not aim to ovveride sails error object. It just add custom error message to Errors so that other error details can be obtained as if it was not hooked into sails lifecycle.

NoRKin commented 9 years ago

I will open another issue tomorrow which will be clearer. I understand that your library doesn't not override the error message. But In my current case, the error.Errors is never returned by the server.

I'll write a spec file so you can understand what I mean.

Cheers,