-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Use with_translation helper to clean up I18n stored translations. #15083
Use with_translation helper to clean up I18n stored translations. #15083
Conversation
def test_send_mail | ||
with_translation 'de', email_subject: '[Signed up] Welcome' do | ||
get '/test/send_mail' | ||
assert_equal "Mail sent", @response.body |
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.
shouldn't this test at least have an assertion about the translation?
@senny I've changed the test case slightly so that the translation is also tested. Is it okay? I don't see why we need |
Was |
@senny yes but why do we put recipient in the subject? |
not sure... seems to be a static value. Should be fine to remove it. |
@@ -10,15 +10,15 @@ class I18nTestMailer < ActionMailer::Base | |||
def mail_with_i18n_subject(recipient) | |||
@recipient = recipient | |||
I18n.locale = :de | |||
mail(to: recipient, subject: "#{I18n.t :email_subject} #{recipient}", | |||
mail(to: recipient, subject: "#{I18n.t :email_subject}", |
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.
We can get rid of the String interpolation:
mail(to: recipient, subject: I18n.t(:email_subject),
@senny done. |
def setup | ||
I18n.backend.store_translations('de', email_subject: '[Signed up] Welcome') | ||
def test_send_mail | ||
with_translation 'de', email_subject: '[Anmeldung] Wilkommen' do |
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.
nitpick, Willkommen
with double l
@senny right... Sorry for my absolute zero knowledge in German. 😄 |
😏 This is looking good 👍 |
…controller_test Use with_translation helper to clean up I18n stored translations.
@senny please review. Thanks.