krakenjs / swaggerize-routes

Swagger document driven route builder.
Other
58 stars 57 forks source link

Add option to show parameter names in Validation Errors #32

Closed duncanhall closed 9 years ago

duncanhall commented 9 years ago

Implementation for https://github.com/krakenjs/swaggerize-builder/issues/31

Adds a useNamedValidators property to the options object.

If this value is set to true then all ValidationError messages will show the name of the parameter that was being validated.

For backwards compatibility, if no useNamedValidators property is set, then 'value' is used as before (ie, the property defaults to false).

tlivings commented 9 years ago

This information should already be present in the error itself. For example:


  "name": "ValidationError",
  "details": [
    {
      "message": "lastName is required",
      "path": "lastName",
      "type": "any.required"
    }
  ],
  "_object": {
    "firstName": "John",
    "age": 45,
    "tags": [
      "man",
      "human"
    ]
  }
}

Are you seeing something different?

duncanhall commented 9 years ago

Yes, as mentioned it is always value is required rather than lastName is required.

In the PR I submitted you should see I have added tests for the new functionality. If you remove the implementation I created, but leave the tests you will see that they fail (because it expects to see the parameter name in the error message.)

tlivings commented 9 years ago

The snippet I posted above is from enjoi - without your additions. Let me dig into why something different is being seen here.

tlivings commented 9 years ago

I see what's going on now. When a validation occurs for a parameter, joi has no context regarding the parameter being validated (as opposed to validating an object's properties in my example above).

So, on the face of it, I see nothing wrong with modifying the error object in this way. I don't think you actually need to make this optional, since this should be expected behavior (since we do know the context).

If you would, please modify the PR to update the error object always, and ensure it modifies error.message, error.details[n].message, and error.details[n].path for consistency.

Sound like a plan?

tlivings commented 9 years ago

Example:

if (error) {
    error.message = error.message.replace('value', parameter.name);
    error.details.forEach(function (detail) {
        detail.message = detail.message.replace('value', parameter.name);
        detail.path = parameter.name;
    });
    utils.debuglog('%s', error.message);
    callback(error);
    return;
}
duncanhall commented 9 years ago

Thanks, this is done as suggested, with some extra tests.

tlivings commented 9 years ago

:+1: