Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internationalize validateSchema messages #242

Closed
eddyystop opened this issue Aug 6, 2017 · 14 comments
Closed

Internationalize validateSchema messages #242

eddyystop opened this issue Aug 6, 2017 · 14 comments

Comments

@eddyystop
Copy link
Collaborator

feathersjs-ecosystem/docs#797 was left at

I added this to the docs

ProTip: You can consider using 'ajv-i18n', together with the 'messages' option, to internationalize your error messages.

I'm not sure how the above would be used with validateSchema. Perhaps validateSchema has to be modified around https://github.com/feathersjs/feathers-hooks-common/blob/master/src/services/validate-schema.js#L65

Could you tell me what you determined when you look into it? Thanks.

@poupryc
Copy link

poupryc commented Aug 6, 2017

Hum... What if common hook integrates ajv-error ?
According to what I saw it is not perfect, but its could suffice

@poupryc
Copy link

poupryc commented Aug 6, 2017

In my opinion, add an optional parameter in validateSchema() to pass personalized messages it's the better way

@eddyystop
Copy link
Collaborator Author

ajv has its own i18n mechanism. I suggest it would be a bad design for validateSchema to implement its own.

@poupryc
Copy link

poupryc commented Aug 7, 2017

@eddyystop But I can not find a way to configure it with a before hook, have you an idea ?

@poupryc
Copy link

poupryc commented Aug 7, 2017 via email

@eddyystop
Copy link
Collaborator Author

You can achomplish either internationalisation or changing error message the documented addNewError option. Copy the default function in the hook, customize it to your needs, and pass it to validateSchema.

https://docs.feathersjs.com/api/hooks-common.html#validateschema

addNewError (optional) - Custom message formatter. Its a reducing function which works similarly to Array.reduce().

Its signature is
{ currentFormattedMessages: any, ajvError: AjvError, itemsLen: number,
index: number }: newFormattedMessages

currentFormattedMessages - Formatted messages so far. Initially null.
ajvError - ajv error.
itemsLen - How many data items there are. 1-based.
index - Which item this is. 0-based.
newFormattedMessages - The function returns the updated formatted messages.

@poupryc
Copy link

poupryc commented Aug 7, 2017

Sound good !

Could you add an example in the doc, or just here. (I must admit, ajv seems a little complicated) ?

In any case, thank you for your work ! 😄

@eddyystop
Copy link
Collaborator Author

Make a copy of this https://github.com/feathersjs/feathers-hooks-common/blob/master/src/services/validate-schema.js#L65-L79 including some console.log to see what you are getting.

Pass that along as addNewError and see what gets logged.

Make changes as needed. Profit!

@poupryc
Copy link

poupryc commented Aug 7, 2017

Thank you, now that I have a work base, I think everything will go more easily ! 💟

If nobody has anything to add, I think the issue is resolved ! Take a beer for me!

@poupryc
Copy link

poupryc commented Aug 7, 2017

After multiple tests (successful, obviously), I asked myself a question: why common-hook, by default, only returned msg? In my mind, my function returns msg but also keyword as the type of the error, the value that caused the error and what was expected. At the end of my function I get this:

{
    "name": "BadRequest",
    "message": "Data does not match schema",
    "code": 400,
    "className": "bad-request",
    "data": {},
    "errors": {
        "value": "email",
        "keyword": "pattern",
        "expected": {
                "pattern": "^(([^<>()[\\]\\.,;:\\s@\"]+(\\.[^<>()[\\]\\.,;:\\s@\"]+)*)|(\".+\"))@((\\[[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\])|(([a-zA-Z\\-0-9]+\\.)+[a-zA-Z]{2,}))$"
        },
        "msg": "should match pattern "^(([^<>()[\\]\\.,;:\\s@"]+(\\.[^<>()[\\]\\.,;:\\s@"]+)*)|(".+"))@((\\[[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\])|(([a-zA-Z\\-0-9]+\\.)+[a-zA-Z
*)|(".+"))@((\\[[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\])|(([a-zA-Z\\-0-9]+\
\.)+[a-zA-Z]{2,}))$""
    }
}

It sounds much better, so we can keep a clean code base by not changing people's habits !

What do you think about a pull request @eddyystop ?

@eddyystop
Copy link
Collaborator Author

The default generates messages in the format commonly required by client-side form handling repo's. This way people don't have to do that conversion themselves.

If you are using your own or another type of form handler, you can get the messages you want by injecting your own function.

In any case, it is way to late to change the default. Too many programs depend on the way things are.

@eddyystop eddyystop reopened this Aug 7, 2017
@eddyystop
Copy link
Collaborator Author

eddyystop commented Aug 7, 2017

Leaving this open to keep a note of perhaps someday interfacing ajv's i18n capabilities.

@poupryc
Copy link

poupryc commented Aug 7, 2017 via email

@eddyystop
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants