Problem/Motivation

Configuration Translation module only uses hasTranslation() from the LocaleConfigManager which is a 1 line function that uses the configurable language manager, but has no dependency within locale. This is the only thing that requires locale in config translation, so we should decouple them.

Steps to reproduce

Proposed resolution

  • Remove the dependency.
  • Update tests to remove locale references as much as possible.
  • Add locale dependency to tests where the explicit integration is being tested.

Remaining tasks

Discuss if this is an allowed API change and when. If not allowed as such, we can hollow out the existing constructor argument, do runtime checking of the configurable language manager, etc, but that would be more lots of added boilerplate :)

User interface changes

None.

Introduced terminology

None.

API changes

Are the two mapper files API?

Data model changes

None.

Release notes snippet

Configuration translation does not depend on Interface Translation module anymore, but it still depends on Language module. The prior dependency was only for a one line utility function in Interface Translation module that did not actually require Interface Translation functionality or data anyway.

CommentFileSizeAuthor
#13 3614335-nr-bot_9jk8iuds.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3614335

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

gábor hojtsy created an issue. See original summary.

gábor hojtsy opened merge request !16507

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Config translation module's dependency on locale is a lie, easy to fix » Config translation module's dependency on locale is a lie, it is only a 2 line method call
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Active » Needs review
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Config translation module's dependency on locale is a lie, it is only a 2 line method call » Config translation module's dependency on locale is a lie, it is only a 1 line method call
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Checked contributed project use of the two classes where we modify the constructor:

None of them seem to extend the constructor or rely on the locale config manager in their implementation, so it sounds feasible to make the change to the constructor without disrupting contributed projects. We don't know about custom code, but if this is done in main only, then the changed will need to be evaluated in a major Drupal upgrade anyway.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Wrote up a change record draft that explains the constructor argument changes too: https://www.drupal.org/node/3614775

nicxvan’s picture

nicxvan commented

This looks good!

Is it worth mentioning in the CR that users can uninstall locale if they only used it for config translation?

nicxvan’s picture

nicxvan commented

I searched both classes in contrib and didn't see any classes overriding the constructor.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Does this really matter in practice? Feels like a very theoretical thing to me, because basically any non-EN site, single language or not, is going to have locale installed?

I can definitely see a use case in being able to disable the locale/config translation sync, because that has so many problems, but uninstalling locale is not an option, I can't think of a use case where you need config translation but not interface translation?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Could be interesting for headless setups where you need some config translated but have no interest in translating the UI (and to handle all the potential problems locale comes with :)

Also ultimately (not in this issue) this module should be named "Config Translation UI" as it does not add in any data handling on its own. Ideally Content Translation would also be Content Translation UI but it also does some metadata handling on top of the entity API. But config translation does not even have that. So those modules that only need to integrate with the API of config translation would clearly be aware that this module is not something they should be dealing with.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3614335-nr-bot_9jk8iuds.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.