openfoodnetwork icon indicating copy to clipboard operation
openfoodnetwork copied to clipboard

Avoid missing i18n translations

Open kristinalim opened this issue 6 years ago • 5 comments

Description

There doesn't seem to be an easy way right now to make sure that translation keys we use indeed have corresponding entries in the main language file.

There's a chance to miss it if a developer introduces a new translation key but forgets to add it to the language file.

And if a translation is missing, the non-underscored version of the key is used, so a missing "new_feature" appears as "New Feature" (wrapped in span.translation_missing). This translation often still makes sense for the development language en, so you would think everything is okay when manually testing in the browser.

These are small enough changes that can help us avoid these missing translations:

  • For translations used in the Ruby code, in automated tests (only test environment), raise error when encountering a missing translation key. We have good test coverage, so I think this would be very helpful.
  • For translations used in the JS, warn in the browser console if a translation is missing.
  • Make it more obvious in the UI if a certain text is only from the non-underscored version of the translation key. As mentioned, these are wrapped in span.translation_missing so can easily be styled.

Expected Behavior

Warnings when using a translation key that is missing from the language file

Actual Behavior

No changes in actual behaviour if translations are okay

Steps to Reproduce

  1. Add t("missing_key") to a page, and check page. The page would not complain, and render this as "Missing Key".
  2. The following test would not complain:
require 'spec_helper'

describe "Missing translation" do
  it "raises error when a translation is missing" do
    I18n.t("missing_key")
  end
end

Context

Lack of confidence when adding/updating translation keys

kristinalim avatar Aug 31 '18 08:08 kristinalim

When I submitted this issue, I was only thinking about how this would affect application code, but it turns out this is also relevant in the decision of whether we should use I18n.t in specs or not. There is a discussion on this here.

About Task 1, @sauloperez recommended this article which has code for this. The code sets this up not just for the test environment but also for development. Should we do the same?

If we also do Task 1 for development, Task 3 will no longer be necessary because then span.translation_missing would never be generated there.

kristinalim avatar Sep 18 '18 18:09 kristinalim

I ran a CI run with exceptions on missing translations. https://github.com/openfoodfoundation/openfoodnetwork/commit/1c928474d1eebf98a8d9b59ea64766b26a5e136c

Here are the results:

Failures:

  1) Account and Billing Settings updating as an admin user attributes can be changed
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: billing_and_account_settings
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/controllers/admin/accounts_and_billing_settings_controller.rb:12:in `update'
     # ./lib/open_food_network/rack_request_blocker.rb:37:in `call'
     # ------------------
     # --- Caused by: ---
     # Capybara::CapybaraError:
     #   Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
     #   ./spec/spec_helper.rb:92:in `block (2 levels) in <top (required)>'

  2) Admin::AccountsAndBillingSettingsController update as super admin when required settings have values sets global config to the specified values
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: billing_and_account_settings
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/controllers/admin/accounts_and_billing_settings_controller.rb:12:in `update'
     # ./spec/controllers/admin/accounts_and_billing_settings_controller_spec.rb:94:in `block (5 levels) in <top (required)>'

  3) UserConfirmationsController requesting confirmation instructions to be resent redirects the user to login
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: spree_user.send_instructions
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/controllers/user_confirmations_controller.rb:15:in `create'
     # ./spec/controllers/user_confirmations_controller_spec.rb:63:in `block (3 levels) in <top (required)>'

  4) UserConfirmationsController requesting confirmation instructions to be resent sends the confirmation email
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: spree_user.send_instructions
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/controllers/user_confirmations_controller.rb:15:in `create'
     # ./spec/controllers/user_confirmations_controller_spec.rb:70:in `block (4 levels) in <top (required)>'
     # ./spec/support/matchers/email_confirmation_matchers.rb:3:in `block (2 levels) in <top (required)>'
     # ./spec/controllers/user_confirmations_controller_spec.rb:69:in `block (3 levels) in <top (required)>'

Failures:

  1) Spree::Payment applying transaction fees to Stripe payments when the payment information is invalid makes the transaction fee ineligible and finalizes it
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: payment_method_not_supported
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/models/spree/order_decorator.rb:339:in `block in process_payments!'
     # ./app/models/spree/order_decorator.rb:336:in `each'
     # ./app/models/spree/order_decorator.rb:336:in `process_payments!'
     # ./spec/models/spree/payment_spec.rb:178:in `block (5 levels) in <module:Spree>'

  2) Managing users as super-admin resending confirmation email displays success
     Failure/Error: raise "missing translation: #{key}"
     
     RuntimeError:
       missing translation: spree_user.send_instructions
     # ./spec/support/i18n_error_raising.rb:4:in `block in <top (required)>'
     # ./app/controllers/user_confirmations_controller.rb:15:in `create'
     # ./lib/open_food_network/rack_request_blocker.rb:37:in `call'
     # ------------------
     # --- Caused by: ---
     # Capybara::ExpectationNotMet:
     #   expected to find text "Resend done" in "Logged in as : [email protected] Account Logout Store DASHBOARD ORDERS PRODUCTS REPORTS CONFIGURATION USERS ORDER CYCLES GROUPS ENTERPRISES CUSTOMERS Editing User BACK TO USERS LIST GENERAL SETTINGS Email confirmation is pending. We've sent a confirmation email to [email protected]. Resend... EMAIL ROLES Admin ENTERPRISE LIMIT PASSWORD CONFIRM PASSWORD UPDATE or CANCEL API ACCESS NO KEY GENERATE API KEY"
     #   ./spec/features/admin/users_spec.rb:40:in `block (5 levels) in <top (required)>'

These may be covered in other issues: #2874, #2374, #1829. We should fix them.

mkllnk avatar Oct 26 '18 05:10 mkllnk

I wonder what else we have to do here after #3006

luisramos0 avatar Dec 10 '19 09:12 luisramos0

I ran a CI run with exceptions on missing translations

Integrating this into the CI build would be great!

Matt-Yorkley avatar Jun 19 '20 10:06 Matt-Yorkley

This is already there Matt, I wonder if it is working correclty though: https://github.com/openfoodfoundation/openfoodnetwork/pull/3006/files

luisramos0 avatar Jun 19 '20 10:06 luisramos0

Closing as per comment.

filipefurtad0 avatar Apr 10 '24 14:04 filipefurtad0