Skip to content
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 support for i18n lookup with the controller path instead of nested keys #285

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

timfsw
Copy link
Contributor

@timfsw timfsw commented Jul 21, 2023

Add option for standard nested separator

This commit adds a new configuration option called i18n_use_slash_separator
to the Route Translator gem. This option controls whether or not to use the /
separator for nested route names in the gem's translation files.

The approach used in Route Translator was to remove the slash and prefer
a nested key. However, this commit changes the default separator to a slash
(/) character to match Rails' nested route syntax.

The old approach of using a nested key instead of a separator would have
prevented the following scenario:

Rails.application.routes.draw do
  localized do
    resources :appointments, only: :index do
      post :reservations, on: :member
    end
    namespace :appointments do
      resources :reservations, only: :index
    end
  end
end

because of the necessary double use of reservations under appointments.

Here's an example of how the new separator works in a translation file:

routes:
  controllers:
    people:
      products:
        favourites: fans

routes:
  controllers:
    people/products:
      favourites: fans

By default, the i18n_use_slash_separator option is set to false to preserve
backward compatibility with existing translation files.

However, if this option is set to true, Route Translator will use the slash
separator instead.

In the next major release of Route Translator, the i18n_use_slash_separator
option will be removed and the behavior will be forced to use the slash
separator.

Therefore, we recommend that users update their translation files to use the
slash separator and set the i18n_use_slash_separator option to true to
ensure compatibility with future versions of the gem.

Upgrade guide

No nested controllers

You are good to go. Just set i18n_use_slash_separator to true

Nested controllers

Let's say that you have People::Products controller

Change people.products to people/products

README.md Outdated Show resolved Hide resolved
@tagliala
Copy link
Collaborator

Thanks.

Is there any use case that should prefer this approach rather than the other one?

@coveralls
Copy link

coveralls commented Jul 21, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling 00c1576 on timfsw:feature/translator_lookup into f518539 on enriclluelles:master.

@tagliala
Copy link
Collaborator

@timfsw

Is there any use case that should prefer this approach rather than the other one?

@timfsw
Copy link
Contributor Author

timfsw commented Jul 23, 2023

@timfsw

Is there any use case that should prefer this approach rather than the other one?

Yes, sorry for the late reply.

I'll try to define the following routes:

Rails.application.routes.draw do
  localized do
    resources :appointments, only: :index do
      post :reservations, on: :member
    end
    namespace :appointments do
      resources :reservations, only: :index
    end
  end
end

For these defined routes, the yaml file should look like this:

---
de:
  routes:
    controllers:
      appointments:
        appointments: termine
        reservations: reservierungen
        reservations:
          appointments: termine
          reservations: reservierungen

And this is not possible because of the necessary double use of "reservations".

undefined method `gsub' for {:appointments=>"termine", :reservations=>"reservierungen"}:Hash

This is my current configuration:

RouteTranslator.config do |config|
  config.force_locale = false
  config.disable_fallback = false
  config.available_locales = [:de]
  config.hide_locale = true
end
  • Ruby: 3.1.1
  • Rails: 7.0.4

@tagliala
Copy link
Collaborator

Thanks, I will take a second look tomorrow.

Do you think that the old behavior should be deprecated and eventually replaced by the new one because of this bug?

@timfsw
Copy link
Contributor Author

timfsw commented Jul 23, 2023

I think so.

Up to now, it has probably been based on the structure of how the translations for the views are used. There is no such problem.

In the models, the entire name is used on one level and not nested further. Otherwise this problem would also occur there.

timfsw and others added 2 commits July 28, 2023 09:20
This commit adds a new configuration option called `i18n_use_slash_separator`
to the Route Translator gem. This option controls whether or not to use the `/`
separator for nested route names in the gem's translation files.

The approach used in Route Translator was to remove the slash and prefer
a nested key. However, this commit changes the default separator to a slash
(`/`) character to match Rails' nested route syntax.

The old approach of using a nested key instead of a separator would have
prevented the following scenario:

```rb
Rails.application.routes.draw do
  localized do
    resources :appointments, only: :index do
      post :reservations, on: :member
    end
    namespace :appointments do
      resources :reservations, only: :index
    end
  end
end
```

because of the necessary double use of `reservations` under `appointments`.

Here's an example of how the new separator works in a translation file:

```yml
routes:
  controllers:
    people:
      products:
        favourites: fans

routes:
  controllers:
    people/products:
      favourites: fans
```

By default, the `i18n_use_slash_separator` option is set to false to preserve
backward compatibility with existing translation files.

However, if this option is set to `true`, Route Translator will use the slash
separator instead.

In the next major release of Route Translator, the `i18n_use_slash_separator`
option will be removed and the behavior will be forced to use the slash
separator.

Therefore, we recommend that users update their translation files to use the
slash separator and set the `i18n_use_slash_separator` option to `true` to
ensure compatibility with future versions of the gem.
@tagliala
Copy link
Collaborator

@timfsw

I did some changes to this branch and I've force pushed. Please fetch from the latest remote and test

I've changed the configuration option name and added a deprecation warning

@timfsw
Copy link
Contributor Author

timfsw commented Jul 28, 2023

@tagliala

Thanks, works great in my current project.

@tagliala tagliala merged commit 2a70e9c into enriclluelles:master Jul 28, 2023
21 checks passed
@tagliala
Copy link
Collaborator

Thanks. I'm going to release 13.2.0 and wait for some time

Then we'll go for 14.0.0 and remove the new option, assuming it is set to true

tagliala added a commit that referenced this pull request Sep 2, 2023
tagliala added a commit that referenced this pull request Sep 2, 2023
@tagliala
Copy link
Collaborator

tagliala commented Sep 2, 2023

Hi @timfsw, I've just released 14.0.0 which removes this option making it the default behavior forced to true

I've also took the chance to remove support for legacy ruby and rails versions, if you can upgrade please let me know

@timfsw
Copy link
Contributor Author

timfsw commented Sep 13, 2023

I've also took the chance to remove support for legacy ruby and rails versions, if you can upgrade please let me know

@tagliala I was able to update without problems. Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants