-
-
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
add back message key in order to have error_codes #1366
Conversation
This needs tests and a CHANGELOG entry, please. |
@dblock Done. Thanks for the feedback |
@@ -9,6 +9,7 @@ class Validation < Grape::Exceptions::Base | |||
def initialize(args = {}) | |||
fail 'Params are missing:' unless args.key? :params | |||
@params = args[:params] | |||
@message_key = args[:message] if args.key?(:message) && args[:message].is_a?(Symbol) | |||
args[:message] = translate_message(args[:message]) if args.key? :message | |||
super |
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.
Can I nitpick? Is this better? Avoids checking key?
twice.
return unless args.key?(:message)
@message_key = args[:message] if args[:message].is_a?(Symbol)
args[:message] = translate_message(args[:message])
or with an if
, I don't care.
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.
Hmm wouldn't the return
bypass the super
method?
I like the idea of not checking the key twice though.
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.
Would you prefer that version ?
super && return unless args.key?(:message)
@message_key = args[:message] if args[:message].is_a?(Symbol)
args[:message] = translate_message(args[:message])
super
or
if args.key?(:message)
@message_key = args[:message] if args[:message].is_a?(Symbol)
args[:message] = translate_message(args[:message])
end
super
(I think I prefer the last one)
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.
definitely the latter 👍
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.
likewise I'm with @ridiculous
Thanks! 😄 What do you think? |
Merged squashed, thanks. |
In my situation, in the exception validations, I need message keys as well as the translations in order to pass error codes