-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
I18n support for all Grape exceptions #323
Conversation
In Rack 1.5 Rack::Utils cookie functions now format expires in RFC 2822 format
end | ||
|
||
protected | ||
# TODO: translate attribute first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this TODO still valid? Could you please explain it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as an example, the validation error has a placeholder named attribute
coerce: 'invalid parameter: %{attribute}'
when locale is en: 'invalid parameter: age'
when locale is zh-CN: '参数不合法: 年龄'
so translate the attribute first. or it will be '参数不合法: age'
but some other error attribute placeholer do not need to be translate, eg:
missing mime type for %{new_format}
the new_format attribute should not be translate.
so currently ,the validation error message is not generate by Grape::Exceptions::Base#compose_message.
maybe the compose_message need an option to figure out whether the attribute need to be translated,
or use I18n.translate('xxx', default: {attr: attr}) to try to translate the attribute first, if nothing in the locale file,
it will fallback the attribute key
not sure is it a litter overdo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an actual scenario you can think of where we would want to translate a parameter? The coerce example is not valid IMO, since it's about an API parameter and you wouldn't want to have one that changes with locale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I can't find any scenario except for the validation error , in my opinion, the validation error will show for the end user(which use our app), they may not understand any english. so "please enter integer for age" will translate to "年龄必须为整数"(年龄 means age). if the age parameter is not translate , it will become "age必须为整数", then the chinese user will confusing, which form filed is wrong?
Or I'm wrong about the grape validate, it's for the user who using the api, more like a documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intent of grape's validate exceptions are indeed for the developer. But I can see why you just want to surface them - they read just fine.
Maybe an acceptable approach is to rescue a validation exception in the API and apply whatever translation rules are appropriate for the app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Your advice make me to think maybe it is the api consumer's responsibility to show the better error message for the end user, and how to correct it. The api provider make sure if there's an invalid param, just tell the api consumer, 'hey,your validation have some problem,check your code.'
And also I'll try to find is it possible to add a message options for the validation, just like the active_model's validation, and for a very simple resolution.
Thanks a lot @dblock
This is awesome, merged, thank you. I've updated CHANGELOG (please do it for the next PR) and modified the Rack spec change to use the cookiejar gem to avoid the date parsing problems. |
thanks a lot ,I'll remember to updated CHANGELOG next time |
Hi
Almost all exceptions message are generate by Grape::Exceptions::Base, can has I18n support. #306
Since my English problem, it's a little hard for me to write the problem/summary/resolution, so I only write the short version,the message can be written in
If some helped to make this more readable will be appreciated.
And the exception class may need renamed too.
thanks