Don't allow nil to be submitted as a key to i18n.translate() - #236
Conversation
|
Hey @jcstringer, sorry for the delay. Could you please update this against the latest master? I'd be keen to get this into the 0.8.0 release. |
|
@radar great to hear from you on this PR, I will make some time to catch this up with master as you have requested in the next few weeks. Cheers! |
|
@jcstringer this is a great PR! would love to see it included in |
|
@sandstrom absolutely just need to find some time hopefully this week? |
|
@sandstrom @radar I've caught this up to master. Locally the specs pass for me on ruby 1.9.3 and 2.3.0. I'm seeing Travis breaking on some of the Rails versions it is testing against, not sure what is expected with this gem and its CI, the few that I checked out in Travis appeared to be related to Gem dependency issues which I would assume are not related to the changes in this PR, but that said let me know if you want help chasing them down especially if they are in fact related to these changes in some way. |
|
@jcstringer Everything looks good to me! Lets see what @radar says. (Yes, the |
|
@sandstrom @radar any updates on the next release of this gem and related PRs? |
|
@jcstringer I don't have commit rights, so I cannot influence the releases or merge PRs. Sorry that I cannot give a more informative answer! |
|
I have commit rights and therefore can influence releases and merge PRs. Sorry that I did not attend to this sooner. I will take a look now. |
|
@jcstringer there's some new and exciting conflicts. Could you take a look again please? |
|
@radar done! |
|
Done! :) Thanks for your patience @jcstringer |
|
This change broke cancancan (see the linked CanCanCommunity/cancancan#415). I'm not sure what their intent is, or why passing |
Based on the conversation in this previously filed issue #207 I've taken a stab at preventing nil from being accepted as a key when calling the I18n.t method. These changes cause a I18n::ArgumentError to be raised, and I also added the same behavior on the exists? method which seemed appropriate.
As it turns out there were a lot of tests which passed in nil as the key so I had to make quite a few changes there. That led me to having to make a related change to the Fallbacks module which was explicitly sending a nil key to the t method. My change there was to just use the original key instead of nil, so I'd ask that the maintainers take a close look at that to make sure that doing so won't cause any other issues. Test suite is passing for me locally with these changes.