Translation of recipe input and config actions [monolingual]

This is a great question. As this issue points towards - there's not necessarily any relation between the config action arguments and the config schema. Therefore I think the only way we're going to be able to do this is to somehow mirror the config schema stuff for action arguments - where they are an array. The big question arising from that is where to put them: in the attribute definition or in a config schema file?

The easier to explain and do would feel like the config schema file. The potentially better for long term Drupal is in the attribute (or a different attribute)... this is because potentially this will lead to being able to declare a config entity's schema in the config entity class file and not in a schema file.

moved to Drupal Core

While considering configuration schema for this task, I'm thinking that will introduce a great deal of complexity and dependencies so I'm looking into some simpler options.

This is a rough first patch presenting some alternate approach, using Symfony YAML tags to mark translatable strings in YAML files. The idea has been presented and discussed in this related thread https://www.drupal.org/project/drupal/issues/3488972#comment-15867505

This is how translatable strings would look like in yaml. Note the newly introduced '!translate' tags.

  actions:
    contact.form.feedback:
      createIfNotExists:
        label: !translate 'Website feedback'
        message: !translate 'Your message has been sent.'
        redirect: ''

So far I've only added these tags for the 'feedback_contact_form' recipe, which is an interesting example because it has both 'input' and 'config/actions' translatable strings. Not sure about using them for 'input' strings but since it was easy enough...

To manually test it, try applying the recipe with a different default language. You'll get translatable/translated strings for prompts and the newly created feedback form :
- Apply patch
- Enable language, locale modules
- Set a new default language
- Try: drush recipe core/recipes/feedback_contact_form
- The first time you'll get English strings but they will be available for translation. Once translated they will be used when re-applying the recipe.

Have not tested but patches should be in MRs please. Also can issue summary sections be filled in

@jose reyero awesome to see you in this issue! And great idea.

Given we're only using Symfony's YAML parser now the idea of using tags is viable. The thing that jumps out is what about translation context? I don't think there is a way to provide an argument to a tag unless we agree some funky character with which to split the string... wouldn't be the first time we've used such an approach in translation. Also I really wish that translation did a fallback approach to context. If it did we could add the action ID as a context to all strings and be done.

Hi @alexpott,

It's actually Gabor's idea :)

About translation context, yes, that is a hard one... Looking at how it's done in existing yaml files (module links.yml, routing.yml ...) I don't think that will work here (adding 'title_context' keys ) ... for more info see 'Translation string sharing and context' here https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Language%...

So I'm thinking some other options - that I've tested, and are ok with the parser - let me know which one looks better, or any better idea...:

input:
  recipient:
    data_type: email
    # Plain list, faster to write, but not that clear
    description: !translate ['The email address that should receive submissions from the feedback', 'First context']

    prompt:
      method: ask
      arguments:
        # Yup, tags work for nested elements ! :)
        question: !translate 
          string: 'What email address should receive website feedback?'
          context: 'Second context'

    # Same as previous one, just formatting it differently
    form:
      '#type': email    
      '#title': !translate { string: 'Feedback form email address', context: 'Third context' }

And still we can have the original '!translate' tag with a simple string, same tag can handle all cases.,

I like this version best :) Will post in Slack to get more feedback :)

input:
  recipient:
    data_type: email
    # Plain list, faster to write, but not that clear
    description: !translate ['The email address that should receive submissions from the feedback', 'First context']

So I'm thinking some other options - that I've tested, and are ok with the parser

I was inclined for the third one because might be easier to handle from potx extraction. But if the three are feasible, I don't have an strong opinion.

input:
  recipient:
    data_type: email
    # Plain list, faster to write, but not that clear
    description: !translate ['The email address that should receive submissions from the feedback', 'First context']

What if the author wants no context?

    description: !translate ['The email address that should receive submissions from the feedback']

Could that be shortcut to

    description: !translate 'The email address that should receive submissions from the feedback'

?
Then recipe writers I assume would prefer that one.

That looks like a great approach to add translation options to recipes.

I don't have strong feelings regarding the exact syntax, but personally prefer the options where the parameters are named explicitly (so 2nd or 3rd).

I'm assuming the 3rd could also be written as

   form:
      '#type': email    
      '#title': !translate { 
        string: 'Feedback form email address', 
        context: 'Third context' 
      }

As it would avoid having everything on 1 line, especially with larger strings you would need to do some horizontal scrolling in your IDE.
Come to think of it, then I probably prefer the 2nd version, as it avoids the extra brackets as well..

Done some patch update, the important changes:

- Moved all tag parsing to Yaml class. This will allow reusability for things other than recipes. Still looking into simplifying this part.
- Implemented string translation context, with the option 2/3 - named parameters -. This doesn't need to be a definitive one but I was needing some working implementation to move on...
- Updated two other recipes for translation: core_recommended_admin_theme, core_recommended_front_end_theme
- Handled yaml validation properly with Type(['string', Stringable::class])
- Fixed arguments types in ConsoleInputCollector.

About the context thing / notation:

- Yes, when no context it can be written as a simple string: !translate 'My simple string'
- About the option implemented I think it has some advantages: it is self documenting, looks better for potx parsing - as pointed out by @penyaskito - and allows extendability with more parameters... (plurals maybe for other usages). Anyway this context thing doesn't happen that often and it is usually needed only for very short strings.

So just to clarify, it currently looks like:

message: !translate {string: 'Your message has been sent.', context: 'Contact form'}

description: !translate 'Just a plain string when no context is needed'

I think explicit options that match the arguments of the object is definitely the way to go. Really like the way this is going. Lovely work.

@jose reyero #13 didn't have the updated patch. Can we use the MR workflow? See the issue fork stuff at the bottom of the issue summary. Thanks!

@alexpott,
Yes, sorry, I forgot to mention I'm using the MR workflow instead of patch files for further development.

@jose reyero I created the merge request for you - https://git.drupalcode.org/project/drupal/-/merge_requests/12121

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

Fixed coding standards issues and the 'No core in component' one by moving the callback function to
Drupal\Core\StringTranslation\YamlTagTranslator.php

Still cannot get over this one:

53 | WARNING | Only string literals should be passed to t() where possible
    |         | (Drupal.Semantics.FunctionT.NotLiteralString

which is caused by this line we need:

return new TranslatableMarkup($string, [], $options);

Any idea about how to get rid of this warning?

Ok, fixed that issue, thx @goba @penyaskito @anjali15

Still, working on the patch a bit more. As we talked on the chat, I'm going to trim down this feature, just translating the 'config/actions' part and leaving the 'input' outside of this scope, creating a new task for that one. On one side it's looking to me more like a different case, and we may want to use the '!translate' tag for it or not, and also there are a lot of 'input' strings in the yaml files.

So let's focus on:
* config/actions
* new yaml !translate tag

Then once we have the feature in place, we can easily use it for more strings - or not.

Looks like tests are passing, finally, just re-running them. This is ready for review now.

A few questions/comments, and it would be good to get an explicit kernel test to prove that translatable strings are actually parsed into TranslatableMarkup objects when a recipe is parsed.

While reviewing the code I realised we could ditch the Drupal\Core\StringTranslation\YamlTagTranslator class completely and just use the TranslatableMarkup constructor. Which kind of brings us inline with PHP attributes which feels nice. It also means that the Yaml::decodeTags() addition is even more flexible. I did toy with making the tag !\Drupal\Core\StringTranslation\TranslatableMarkup but it is really ugly and PHPStorm's yaml parser doesn't really like it either - plus we'd need to introduce an interface so you couldn't just construct any class here.

@jose reyero I hope it is okay but I pushed up the changes so people can see what I'm on about - if you don't like feel free to revert.

Yes @alexpott, looks great to me, beautifully simple I'd say!

This looks great and glorious in its simplicity, but I think it should get an explicit test that this actually works with recipes and guarantees config action translatability. Nothing too tricky -- just apply an existing core recipe with a translatable config action and prove that the translated string is the one that ends up being stored in config.

@phenaproxima
> Can we add an explicit kernel test of applying a recipe...

Yes, working on that...

So one thing that's occurred to me and feels like a bit of a problem... this puts the onus of translatability of config actions on the recipe author and not the config action author and that feels kinda wrong. Because the same config action can be used by many different recipe authors.

Hi @alexpott, not sure I understand your latest comment

> this puts the onus of translatability of config actions on the recipe author and not the config action author

I mean.. aren't config actions part of the recipe ? - and have the same author/s... Or... is there any plan to have separate config actions that can be included later in the recipes?

So config actions are provided by modules and core. Which of the arguments are translatable is down to the config action. A config action can be used by many recipes. The current solution means that every recipe that uses a config action has to mark a value as translatable whereas if the config action could confer what is translatable then each recipe would not need to do this.

The analogy for configuration is any module can provide a view but only the views module needs to indicate that the view title is translatable (via the provision of views.schema.yml).

I see @alexpott point here, but I think that (sadly) this still needs to rely on recipe authors.

e.g. If I use a place block config action on my recipe (or add a section to a layout builder entity), it could have settings that could be translatable (depending on the actual recipe content), specially when dealing with plugins.

While Drupal runtime might be able to find out the config schema of those plugins (I'm not 100% sure, and if it can, it will definitely be complex), tools running without a Drupal runtime (e.g. potx) won't be able to extract those.

And for those recipes using inputs, only the recipe author knows if those are hardcoded/calculated or e.g. using 'ask' for getting the input from whoever is applying the recipe.

Oh, I get the point now and I somehow agree with @alexpott, the best / ideal way of having all this translatable / translated would be relying on config actions definition and the config schema.

Actually that was my first idea to approach the problem, but I found it way too complex, specially for the "string extraction" part and this is why we are proposing this "tag thing" instead. Some of the problems of using config schema are mentioned in this (parent) issue, around this comment https://www.drupal.org/project/drupal/issues/3488972#comment-16049599

Also I don't think putting the onus of translatability on the recipe author is a bad thing by itself. Because it means also "giving the power" of translatability to the recipe author - as opposed to maybe waiting forever for some module providing a config action to make some edge case translatable.

But mostly, it was the complexity of 'config actions + config schema + potx extractor' working together what pushed us to look for a different / simpler (way simpler I'd say) idea.

Fixed PHPSTAN. Now one test, that might be related, is failing:

    Time: 00:09.167, Memory: 8.00 MB
    
    Installer Isolation Level Existing Settings (Drupal\Tests\mysql\Functional\InstallerIsolationLevelExistingSettings)
     ✘ Installer
       ┐
       ├ Failed asserting that two strings are equal.
       ┊ ---·Expected
       ┊ +++·Actual
       ┊ @@ @@
       ┊ -'REPEATABLE-READ'
       ┊ +'READ-COMMITTED'
       │
       │ /builds/issue/drupal-3313863/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelExistingSettingsTest.php:58
       ┴
    
    FAILURES!
    Tests: 1, Assertions: 8, Failures: 1, PHPUnit Deprecations: 2.
---- Drupal\Tests\jsonapi\Functional\SettingsFormTest ----

The test-only test fails:

PHPUnit 11.5.22 by Sebastian Bergmann and contributors.
Runtime:       PHP 8.4.12
Configuration: /builds/issue/drupal-3313863/core/phpunit.xml.dist
...............................EE                                 33 / 33 (100%)
Time: 00:00.029, Memory: 8.00 MB
There were 2 errors:
1) Drupal\Tests\Component\Serialization\YamlTest::testDecodeTagsNoTagClasses
Error: Call to undefined method Drupal\Component\Serialization\Yaml::decodeTags()
/builds/issue/drupal-3313863/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php:111
2) Drupal\Tests\Component\Serialization\YamlTest::testDecodeTagsWithTagClasses
Error: Call to undefined method Drupal\Component\Serialization\Yaml::decodeTags()
/builds/issue/drupal-3313863/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php:134
ERRORS!
Tests: 33, Assertions: 33, Errors: 2.
Exiting with EXIT_CODE=2

So removing 'Needs tests' tag.

@penyaskito @jose reyero thanks for the thoughtful comments. Yeah there is a massive tension between what core might be capable of with respect to working out what is translatable in a config action and what POTX can do.

I was trying to look at whether we could use the existing configuration schema system as potx is already able to read schema and work out what is translatable in a yaml file. Let's look at a real case - https://git.drupalcode.org/project/drupal_cms/-/blob/2.x/recipes/drupal_... - here are some things from that that need to be translatable:

• recipe description
• input description
• input prompt question
• input form title
• input form description
• input form option text (maybe not in this case but it is certainly possible)
• simple config update action - ai_agents.settings:agent_settings.field_type_agentusage_instructions
• setupAiProvider config action key_label
• placeBlockInAdminTheme settings label and first_message

Of these we could potential leverage existing config schema for:

• simple config update action - ai_agents.settings:agent_settings.field_type_agentusage_instructions
• placeBlockInAdminTheme settings label and first_message

We'll have to add to stuff to work out how to translate the rest.

!translate tag

Problems

• Has to be done for every recipe and potential multiple times in the same recipe (for example input form titles)
• We already know that something are translatable, for example, simple config updates
• it's going to be really easy to forget to add a translate tag and there'll be tonnes of duplicate work

Strengths

• We have a working MR
• "Simple"

Recipe schemas leveraging config schema

Problems

• Wrapping reading the recipe yml and changing strings to TranslatableMarkup - duplicating the config_translation module kinda but also not
• Where to store recipe specific schema - do we pollute config schema?
• Complex cases like \Drupal\Core\Config\Action\Plugin\ConfigAction\CreateForEachBundle which can create any entity type foreach bundle.. see #3464550-11: Create config action which can create an entity for every bundle of another entity type

Strengths

• Existing recipes won't need changing and we'll get a lot info already from the config system.
• We'll be able to also use it to validate recipes

Whichever we we go will have to make changes to the POTX module to support. My gut feel is that !translate is likely to be simpler.

I've just discussed this with @borisson_ and it's led to an idea about a third way.

1. We add the !translate tag support in this issue.
2. We add support for !translate tag to POTX.
3. We open an issue to add CLI tooling to Drush to add !translate tags to recipes using a recipe schema and config schema.
4. If step leads to being able to fully describe a recipe using schema then we might bring this into core and use it to validate config actions and support translatablity.

This way we'll get recipe / config action translation soon and can make progress without having to make POTX and the recipe system super complex and can explore step 3 at a slower pace.

This is ready for reviews - I’ll open the Potx issue to support this. And address @phenaproxima’s comments when I’m in front of a computer and not a phone.

I have an MR for the POTX module to support extracting the strings from recipes - see #3516466: Extract strings from Recipes

Updated issue summary

Calling it.

I've added CRs, a release note and update the issue summary. Once those have been reviewed I think we're good here.

Found a few minor points. @bircher also pointed out in Slack that the $name and $description parameters of Recipe::__construct() need to be string|\Stringable, or they blow up if you try to translate them. So kicking this back for that and the requisite test coverage.

The CRs look really good and they seem easy to read. Should we mention in the recipe.yml translation CR what properties are automatically extracted by potx, so people don't needlessly add !translate to them?

Am I understanding it correctly that we could now add other yaml tags, that can be casted into a different stringable object as well? I can't immediately think of a good usecase, but that seems like a powerful thing to do.

The Contact module is being removed from core in #3520460: [meta] Tasks to deprecate the Contact module.

In #3553006: Remove Contact recipe we are running into issues with tests that use the contact module. Do the same issues appear here?

Both the plain in #39 and current code look good to me.

A few small things I noticed but none are commit blocking, although it'd be nice to fix them before landing this. But, if you'd prefer not to, I still say ship it.

Updated issue summary with clearer problem statement.

Reviewed the MR changes. The main question I have if there is some way that language matching is ensured? That I don't see in the MR, but I may be missing it? As far as I see translations are pulled from the language used when the recipe is applied, which may or may not match the language of the config that the config action is applied to. I think the translation should be applied to the language of the config, not based on what language is active when the recipe itself is run. Otherwise the config may end up with a jumble of languages.

Also at the time of the recipe being applied, the translatable pieces are known, so the recipe application could/should update (create?) the config translations to also save the appropriate translations of the values as available, not just apply the translation behaviour to the active config IMHO.

I understand an alpha deadline is tomorrow (https://www.drupal.org/about/core/blog/drupal-113x-alpha-phase-begins-oc...) so wondering if we can segment this feedback in a way where the committed version can be built on top later or get a core exception? :) Or it may be that I'm missing this in the MR and it is already there :)

which may or may not match the language of the config that the config action is applied to.

It will match because the system will have config translation overrides applied. Any config the recipe is changing will be loading in the language the site is running the recipe apply on. What this does bring up is that like code - strings in recipes must be in English and any config supplied by the recipe should be in English too. Also I don't think we have the facility to supply recipes in another language. But that's the same as code too.

I'm pretty sure we have to ensure that recipes are applied in the site's default language.

Also at the time of the recipe being applied, the translatable pieces are known, so the recipe application could/should update (create?) the config translations to also save the appropriate translations of the values as available, not just apply the translation behaviour to the active config IMHO.

This is probably the bigger issue and feels complex. We need to work out the desired behaviour for multilingual sites with English applying recipe in English, multilingual sites with English applying recipe in another language, multilingual sites without English and monolingual sites in a language other than English.

I'm pretty sure that we have the correct behaviour for monolingual sites in a language other than English.

I don't think we'll break multilingual sites with English applying recipe in English when English is the default language. But you're right we need a step to create the config translations. This takes as back to the problem with mapping the schema of config actions to config so we can create the correct config translation. Looking at a real world example of something like placeBlockInAdminTheme we will need to place the block in the site's default language and the loop over the site languages and create config translations for the block placement. Generalising this.... we're going to need to record all of the configuration touched by during the config action step and then loop over it and translate anything that is not already translated.

We're probably not good for multilingual sites with English applying recipe in another language or multilingual sites without English.

TLDR;
@gábor hojtsy is correct we have a load more work to do here. Nothing is wrong with the approach so far but we need to:

1. Enforce recipe application in the site default language. I think we start with a simple exception and then in the long run work on the ability to execute code in a language context (there have been a number of issues about this).
2. Test monolingual non-English
3. Test multilingual no-English available.
4. Work out how to create config translations and how we can do that without having to bring schema to recipes.

Unfortunately I do think that this makes this extremely unlikely to make 11.3.0.

Ensuring recipe application is in the site default language would still only be enough for monolingual sites. Multilingual sites could have config in any specific language in active config, so the right translation should be applied to each (and their config translations applied too). So the current approach does not work for multilingual sites, only monolingual sites even if the recipe application language is enforced. Rather IMHO the translation should be ran for each specific config in the config's language.

Multilingual sites could have config in any specific language in active config

oh dear... how right you are... that's going to be hilarious. So the action applier will need to be able to work this out if necessary. Digging out the issue to make it easier to execute code in a language context...

Well, right, this won't work for multilingual sites but the main reason for that is that Recipes just don't work for multilingual. Actually atm they just work for monolingual English sites. Right now recipes don't even apply configuration translations, see #3453331: RecipeConfigInstaller should process translation config files

So what we are trying to do here is to get them working for monolingual non-English sites too. Actually our only use case for now is getting the Drupal CMS installer working for other languages.

So yes, we are assuming the site is single-language and the configuration is of course in that single default language. For any other scenario we should fix Recipes, and I guess config actions too, first.

Updating title and description for the monolingual case.