Add plurals dropped from rails-i18n in 7.0.6

[chao-xian]

The 7.0.6 release of rails-i18n dropped support for a number of plurals 1 and this causes our apps that make use of these languages to error and thus be unable to update to rails-i18n 7.0.6

This commit ports over these rules from rails-i18n to maintain the behaviour.

This update follows what was added into govuk_app_config 2

[kevindew]

I assume we need Maltese (mt) and Chinese (zt) too.

I'm a bit worried about this file as it's already out of sync with govuk_app_config and will error on some of them:

rails_translation_manager/config/locales/plurals.rb

Lines 60 to 63 in 6a7d153

	# Chinese Hong Kong
	'zh-hk' => { i18n: { plural: { keys: %i[other], rule: -> { :other } } } },
	# Chinese Taiwan
	'zh-tw' => { i18n: { plural: { keys: %i[other], rule: -> { :other } } } }

(where lambda's are missing an argument) so I'm assuming nothing actually uses this.

[chao-xian]

ah ok I'll add those in too - it was only cy that I needed for Frontend

[1pretz1]

As to the relevance of the plural rules file in Rail Translation Manager - IRC the original plan was to replace the plural file in govuk-app-config with this one as apps that use translations, should also include RTM but the work paused prematurely, due to the re-org of teams. We should probably continue with that plan, or point RTM to the file in govuk-app-config so we have one source of truth.

[chao-xian]

ah c*** sorry I forgot to update the CHANGELOG as well

[kevindew]

As to the relevance of the plural rules file in Rail Translation Manager - IRC the original plan was to replace the plural file in govuk-app-config with this one as apps that use translations, should also include RTM but the work paused prematurely, due to the re-org of teams. We should probably continue with that plan, or point RTM to the file in govuk-app-config so we have one source of truth.

Sure, is there any plan to do anything on it in the next 3-6 months? If not, and if it's not something on navigation and personalisation's radar, it's probably best to just delete this for now. It doesn't seem to serve a functional purpose and doesn't seem like something that would be difficult to restore if/when the plan gets re-spun up - whereas it'll be confusing and debty the whole time it exists in two places.

[1pretz1]

Agreed. It wasn't on my radar but happy to take it on (have some spare time tomorrow). Sorry it was left in this state, there were quite a few things left to finish when the team disbanded, believe some follow up / clean up cards were made.

[chao-xian]

Sorry is the consensus that we should remove plurals from govuk_app_config?

[1pretz1]

I think that's a better solution for a few reasons:

• maintaining two lists is confusing, as pointed out

• keeping all translation related code in one place is cleaner

• govuk-app-config is a dependency of many non-translated apps and so
  defining plural logic: GovukI18n isn't needed for the majority of applications

Do we agree?

[chao-xian]

@1pretz1 just to be clear, you're saying removing plurals.rb from here is better?

[1pretz1]

My suggestion would be to remove the plurals file defined in govuk_app_config and update the frontend apps to use RTM's version.

Don't think it matters too much but think using RTM's version is cleaner.

[chao-xian]

@1pretz1 the config in the frontend apps was the missing ingredient!!! So switching it in the frontend apps should happen first really

[kevindew]

We also need all apps to start using RTM in all environments, as nearly are flagged as development gems at all:

e.g: https://github.com/alphagov/whitehall/blob/407ad2b60561e9849084becac38accd8d827388f/Gemfile#L64-L71

Which is what through me before as I couldn't understand how the file would be used.

I'm of the view that if someone is genuinely going to work on killing it GovukAppConfig and changing over in the next few weeks or so then leave this file and go for it. If it's a we should do this, lets put it on the backlog then we should kill this plurals file until that day comes.

[chao-xian]

Right I'll raise a PR to remove from here first...

[1pretz1]

Yep, I imagine the sequence to be:

1. update RTM to use a wrapper class for it's plurals (like govuk_app_config) which will be easier to pull into each frontend application
2. release RTM change
3. update frontend apps to use RTM's version
4. remove plural file from govuk_app_config

I had a few moments so have started step 1: #51, also tested this against a frontend app and all works fine. +1 to Kev's comment about moving the Gem out of the dev group for the frontend apps.

I'm happy to carry this work forward as feel motivated to do so 😄

[chao-xian]

OK go for it @1pretz1 and ping me to help with this

[kevindew]

1. update RTM to use a wrapper class for it's plurals (like govuk_app_config) which will be easier to pull into each frontend application

I'm assuming the plurals.rb file gets auto loaded and doesn't need a wrapper class? not sure though, I assumed placing it in config was for it to be autoloaded as part of Rails initialisation, otherwise would presumably be a file in lib

[1pretz1]

Hmm, I assumed not as the config file isn't part of the apps config, rather the gems and it isn't explicitly required (simple Ruby app). However, I'm not sure so I'll verify your point.

[kevindew]

Hmm, I assumed not as the config file isn't part of the apps config, rather the gems and it isn't explicitly required (simple Ruby app). However, I'm not sure so I'll verify your point.

It looks like this does it:

rails_translation_manager/lib/rails_translation_manager.rb

Lines 25 to 29 in 5431893

	I18n.load_path.concat(
	Dir["#{rails_i18n_path}/rails/pluralization/*.rb"],
	["#{rails_translation_manager}/config/locales/plurals.rb"]
	)

I gave it a spin locally and can see that file gets executed.

What I imagine happens is that GovukAppConfig ones override these.

[1pretz1]

Ah okay, thanks for checking! Class wrapper may not be needed then.