Ensure the value of default option must be a string when calling I18n.t #1737
Conversation
mlzhuyi
force-pushed
the
ensure_default_option_a_string_when_call_i18.t
branch
from
863655c
to
e9393c5
Compare
lib/grape/exceptions/base.rb
Outdated
| end.join(', ') | ||
| end | ||
|
|
||
| def translate_attribute(key, **options) | ||
| translate("#{BASE_ATTRIBUTES_KEY}.#{key}", default: key, **options) | ||
| translate("#{BASE_ATTRIBUTES_KEY}.#{key}", default: key.to_s, **options) |
There was a problem hiding this comment.
Maybe I am nitpicking, but feels like we should do this inside translate, no?
There was a problem hiding this comment.
It is generally better. But I'm not sure if there is any need to pass an alternative symbol key as value of :default in other cases, though, there isn't any example now.
CHANGELOG.md
Outdated
| @@ -7,6 +7,7 @@ | |||
| #### Fixes | |||
|
|
|||
| * Your contribution here. | |||
| * [#1737](https://github.com/ruby-grape/grape/pull/1737): Ensure the value of default option must be a string when calling i18n.t - [@mlzhuyi](https://github.com/mlzhuyi). | |||
There was a problem hiding this comment.
This fixes a bug. What's the error that you get without this fix? That's what should be in the changelog, please.
mlzhuyi
force-pushed
the
ensure_default_option_a_string_when_call_i18.t
branch
from
e9393c5
to
bad7bf3
Compare
CHANGELOG.md
Outdated
| @@ -7,6 +7,7 @@ | |||
| #### Fixes | |||
|
|
|||
| * Your contribution here. | |||
| * [#1737](https://github.com/ruby-grape/grape/pull/1737): Fix translating error whe pass symbols as params in custom validations - [@mlzhuyi](https://github.com/mlzhuyi). | |||
mlzhuyi
force-pushed
the
ensure_default_option_a_string_when_call_i18.t
branch
3 times, most recently
from
0924c22
to
b7fc5bf
Compare
|
It has been changed @dblock |
lib/grape/exceptions/base.rb
Outdated
| @@ -71,6 +71,7 @@ def translate_message(key, **options) | |||
| end | |||
|
|
|||
| def translate(key, **options) | |||
| options[:default] &&= options[:default].to_s | |||
There was a problem hiding this comment.
This modifies the caller's options, which can easily be a frozen hash or whatever.
There was a problem hiding this comment.
* and ** in arguments act a different way with common argument.
def common_argument_test(args)
p args.object_id
end
def keyword_argument_test(**args)
p args.object_id
end
args = { a: 1 }
p args.object_id # => 70338004137000
common_argument_test args # => 70338004137000
keyword_argument_test args # => 70338004136780In there code, args in keyword_argument_test is a new hash, create by the method itself.I think we can treat it as a most basic hash.
There was a problem hiding this comment.
I guess we always declared it the old fashioned way as options = {}. I worry that this is different version from version, especially reading this SO. Can we just do this the old fashioned way and avoid surprises?
There was a problem hiding this comment.
Do you mean refactoring out the code, for there here are many ** now?
There was a problem hiding this comment.
Sorry if I wasn't clear. The double splat is fine. I mean this:
options = options.dup
options[:default] &&= options[:default].to_sWe dup arguments everywhere as far as I can see.
There was a problem hiding this comment.
It's a really good habit in case of unknown changes.
I have added it :)
mlzhuyi
force-pushed
the
ensure_default_option_a_string_when_call_i18.t
branch
from
b7fc5bf
to
a8058ba
Compare
mlzhuyi
force-pushed
the
ensure_default_option_a_string_when_call_i18.t
branch
from
a8058ba
to
a23ffad
Compare
|
Thanks, merged. |
#1735