🇧🇪Belgium @svendecabooter

svendecabooter → created an issue.

Added a fix that should address this issue.

Thanks, that seems like a good addition.

Could we also take the opportunity to set autowire: true in the services defined in externalauth.services.yml?
Autowiring is supported since 9.3, and since our min. required version is 9.5, that should work.

svendecabooter → made their first commit to this issue’s fork.

It is made backwards compatible from Drupal 10.1 on, with the #[LegacyHook] attribute.
See https://www.drupal.org/node/3442349#hook_convert →
That's already added in this MR - so could still go into 3.2.x if desired.

No longer needed - see 📌 Deprecate module for ai 1.2.x Active

No longer needed - see 📌 Deprecate module for ai 1.2.x Active

Rebasing / fixing of merge conflict causes a new issue in test Drupal\Tests\openid_connect\Functional\Update\IssAllowedSchemaUpdate30004Test::testUpdateHook30003

Not sure how this is related, except that earlier hook also updated plugin config. Maybe it now fails because in our update hook we also update the plugin config further.
Any insights from maintainers as to how to solve this would be appreciated.

Updated the MR to fix a merge conflict with hook_update_x numbering.

svendecabooter → created an issue.

Does this issue still exist on the latest version of the module?

That seems more like an issue of a bad workflow regarding config management...
But the proposed fix is fairly straight forward, so will add it as an extra safeguard.

Improved the logic from ✨ Add permission for resending emails Needs review to make it compatible with other modules, that provide a workaround for the broad 'administer users' permission. Now you can access the feature when you have 'administer users' permission, or the module-specific permission.

svendecabooter → made their first commit to this issue’s fork.

Added the new permission, while keeping the existing access logic intact.
You can now use this functionality if you are able to update user entities (current access logic), OR apply the action to user entities if you have the newly added permission granted.

I guess you would need to debug this via XDebug or something...
Alternatively, you could switch to using the Drupal Symfony Mailer Plus → module with an SMTP transport configured, and install Drupal Symfony Mailer Log → module. This will create a log entry for each outgoing email, making it easier to debug if the mail is going out or not.

Sorry for not coming back to this.
That seems like a useful feature to be added to an optional submodule, for users seeking that functionality.
Or within the module itself as a configuration option (but disabled by default).

Looks good, thanks!

Weird, I can't seem to reproduce this (although I'm testing with Drupal 11.2 currently).
Version 1.3.6 didn't introduce anything that should break this again, so it's probably due to core changes.

@nidhi27 yeah that was my rough idea... not sure if that's easily doable.

I tested this logic, both updating fields created in the 3.x version, and adding new fields.
I have found no real issues with it.
Seems like a good improvement to me.

svendecabooter → created an issue.

Merging now.

OK those seem like 2 valid points to stick with the current approach.
If the current logic would pose unexpected problems, we can still change the implementation later on.

Thanks for the clarification.
Sounds like a good improvement to me.

Isn't this (partially) implemented in 📌 Add token usage to streamed chat Active , or am I reading this incorrectly?

Thanks for the MR. Logic looks good to me.
Only thing I'm wondering: couldn't we use a QueueWorker plugin to process the deletion of the stale log entries?
Then we wouldn't need the batch processing logic within the cron hook.
The processing time could be configured in the QueueWorker plugin - e.g. \Drupal\locale\Plugin\QueueWorker\LocaleTranslation has this set to 30 seconds.

Yeah I have set it up with "Details" wrapper currently.
Would be good to allow for 1-click collapsing / expanding though, to easily switch between "editing" mode and "reordering" mode.

svendecabooter → created an issue.

svendecabooter → created an issue.

Tested patch as well, and works like a charm.

svendecabooter → created an issue.

I can confirm the fix in MR 12461 restores the height of the navigation sidebar.

Looks good to me.

Tested this MR with the newly released 1.2.0-alpha1 release of the AI module, and everything works as expected.

Do we also need to document that file / image extraction will fail without the core patch 🐛 InvalidArgumentException: Invalid translation language (en) specified Active ?

svendecabooter → created an issue.

@valthebald
Can you also credit user "arwillame", because a chunk of code for Layout Builder translation support got taken from https://www.drupal.org/project/ai_translate_lb_asymmetric →

I tested the current MR, and this seems to work as intended.
Maybe the Drupal logo color changing according to the environment indicator settings could be an option?
Although from what I can see this is managed in CSS, so probably not very easy to make this configurable...

svendecabooter → created an issue.

Renamed issue for clarity.

Adding a simple patch file to just restore the canonical link, to fix this issue via Composer patching workflow (only recommended for websites not using the flattened site settings loader obviously), while awaiting a more robust solution in the next release.

I have added a new MR now, to attempt to fix this issue.
In the first commit, I just restore the canonical link for SiteSettingEntity.

In the second commit, I add a configuration setting to not render the site entity on its canonical link, but its edit form instead.
I have added an option in the config form to disable this.

I'm not really sure why site settings shouldn't have a canonical view actually... When you use the revision system of that entity (by deselecting the config option "Hide the administrative advanced options"), it would make sense that you can view the latest revision of the entity + earlier revisions, for comparison. They are only viewable for people with the "view published site setting entities" or "view unpublished site setting entities" permissions...

I'm not sure what the exact issue with the flattened site settings loader is exactly, that causes an infinite loop, but there might be another way to fix that, rather than messing too much with the default Drupal entity link templates...

Can someone check if my solution that renders the edit form instead, fixes the flattened site settings loader issue?
And could use some extra thoughts on whether the edit form rendering is a good idea to begin with...

We are encountering similar issues.
Site settings entities are no longer translatable after 📌 Remove the canonical route of the site setting entity type Active has been committed.

The translatability of an entity is checked by \Drupal\content_translation\ContentTranslationManager::isSupported().
This checks if there is a link template "drupal:content-translation-overview".

Looking into where this template is defined - \Drupal\content_translation\Hook\ContentTranslationHooks::entityTypeAlter, we see it is only defined after the following logical check:
if ($entity_type->hasLinkTemplate('canonical')) {}

Which is now no longer the case.

I think the safest solution would be to restore the canonical route, and perhaps just load the entity "edit" form for that route?

Tested this update, and it works well with ✨ Add text extractor plugins for image and link field types Active .

Tested exporting and importing with this MR checked out, together with the MR in default_content ( https://www.drupal.org/project/default_content/issues/3532596 ✨ Add support for custom_field module Active )

I was able to export and import all custom_field data, including entity references (tested with Media and Taxonomy term) + custom_field file and image properties.

Everything worked as expected.

@apmsooner

I have tested with the following setup:

- Drupal 10.5.1
- AI module with this MR branch checked out
- Core patch applied: https://www.drupal.org/project/drupal/issues/3386915#comment-16174131 🐛 InvalidArgumentException: Invalid translation language (en) specified Active
- Configured core content_translation and language modules.
- Enabled AI Translate module + set up "Entity reference translation" settings to include Content block / File / Content / Paragraph / Taxonomy term - max reference depth set to 5.
- Created content type with a bunch of fields (plain text, body, image, term reference) + Paragraphs module entity reference (paragraph had same type of fields added: plain text, body, image, term reference).
- Created a 2nd content type with a bunch of fields (plain text, body, image, term reference) + enabled Layout Builder. Created a block type with plain text field, body field, image field, term reference field

I encountered 2 issues:
- "Warning: Undefined array key 0" PHP warning - although functionality kept working. Fixed now
- Symmetric layout builder translation didn't work fully yet (since it was untested as stated above). Added fix for that as well now.

I'm unsure how to reproduce your behaviour that translation doesn't work at all.
Maybe there is some specific configuration that is interfering, since it requires a bit of setup to get all pieces in place...
Perhaps you can share a config export of your installation via Slack, so I can try to reproduce the unwanted behavior.

I have tested now with 2 different setups (D10.5 with Paragraphs & symmetric LB + D11.2 with asymmetric LB + custom_field) and can get everything translated as expected...

Done

Can this perhaps be added as a merge request?
That would make it easier for reviewing...

Agreed, these can all be disabled.
Specific websites can reactivate them via config_split if needed, but I don't see a particular reason for that currently.

It's a lot of config changes, but they seem sensible to me as a basis for the different sites, which can use config_split to update configuration specifically for their instance.

FYI: I have not tested the latest version of the MR yet with a regular Layout Builder setup (without layout_builder_at module installed).

Based on feedback above, I have also incorporated the logic of the ai_translate_lb_asymmetric module into the LbFieldExtractor.
This provides a single FieldTextExtractor for the Layout Builder field type, reducing conflicts between modules / plugins.
Based on the selected Layout Builder strategy for a given installation, it takes care of the proper AI translation.

This eliminates the use of the separate ai_translate_lb_asymmetric module. When this improvement gets committed, it seems fair to credit author(s) of that module as well.

Thanks for your work on this refactor / improvement @apmsooner.
I've been testing it out, and it works very well.

Tested with simple fields on a node, with a custom_field setup (with mentioned custom_field branch checkout), and a complex entity reference setup multiple levels deep.

I have added an additional commit for ReferenceFieldExtractor, that implements the logic I described in the MR comments.
It removes the needs for translatable_properties annotations, since it infers whether they are translatable or not, from the relevant (recursive) FieldTextExtractor plugin that gets called for that field.

Tested the same scenario's as described above after this change, and functionality kept working.
Let me know if you think this isn't a correct approach, then the commit can be reverted in the MR I guess.

I tried applying MR 103 on a Drupal 11.2 website, but am getting the following errors when setting up a current environment:

Drupal\Core\Render\Component\Exception\InvalidComponentException: [navigation:toolbar-button/icon] String value found, but an object is required. The provided value is: "development". in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 234 of /var/www/html/web/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php)
#0 /var/www/html/web/core/lib/Drupal/Core/Template/ComponentsTwigExtension.php(124): Drupal\Core\Theme\Component\ComponentValidator->validateProps()
#1 /var/www/html/web/core/lib/Drupal/Core/Template/ComponentsTwigExtension.php(106): Drupal\Core\Template\ComponentsTwigExtension->doValidateProps()
#2 /var/www/html/vendor/twig/twig/src/Environment.php(420) : eval()'d code(48): Drupal\Core\Template\ComponentsTwigExtension->validateProps()
#3 /var/www/html/vendor/twig/twig/src/Template.php(402): __TwigTemplate_644fe1f4d3b3cc70a78b679130cdd653->doDisplay()
#4 /var/www/html/vendor/twig/twig/src/Environment.php(420) : eval()'d code(136): Twig\Template->yield()
#5 /var/www/html/vendor/twig/twig/src/Template.php(402): __TwigTemplate_4bb116eb700f12a2c42b5883d177516f->doDisplay()
#6 /var/www/html/vendor/twig/twig/src/Template.php(358): Twig\Template->yield()
#7 /var/www/html/vendor/twig/twig/src/Template.php(373): Twig\Template->display()
#8 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(51): Twig\Template->render()
#9 /var/www/html/web/core/themes/engines/twig/twig.engine(34): Twig\TemplateWrapper->render()
#10 /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php(380): twig_render_template()

This change might need a warning that it depends on ai 1.2.x, and including it into custom_field might need to be timed together with the first (stable?) release in that branch...
I don't suppose it would be possible to both support 1.1.x-style FieldTextExtractor plugins, as well as 1.2.x-style...

@apmsooner:

Yeah I reckoned the module would need to be updated. Was just checking if there might be ways for backwards compatibility that I didn't think about. If this refactor goes into 1.2.x branch, I guess modules that extend this functionality, would need to create a separate branch for AI 1.2.x compatibility. There is also https://www.drupal.org/project/ai_translate_paragraph_asymetric → that provides a similar plugin.

The custom FieldTextExtractor plugins for the assymetric modules could probably also be included into the ai_translate module, rather than a separate module, but then the FieldTextExtractor plugin might need some extra logic to decide whether the plugin needs to kick in or not. E.g.:
- for asymmetric LB translation --> check if layout_builder_at module is enabled
- for asymmetric Paragraphs translation --> check if paragraphs_asymmetric_translation_widgets module is enabled
Or could that logic go into shouldExtract()? Guess not, since that's more about logic on field level, than general condition checking...
Now this logic is enforced by those separate ai_translate_* modules having the dependencies set in their info.yml file. If they got included in ai_translate itself, they might be activated where they shouldn't (i.e. if translation logic is symmetric instead)

@eelkeblok: I have merged issue 📌 Support OOP hooks Active , so this functionality could be implemented with OOP hooks in mind.
Haven't reviewed the code yet, but described functionality looks promising :)

Remaining issues have been resolved, and this has been added to 1.0.x branch.
@eelkeblok would be good if you could rebase your MR branch on 1.0.x and also add support for OOP hooks. Sorry for the extra work.

Dependency on custom_field removed, by explicitly checking if the module is enabled.

FYI:
At this point in time I would suggest to include the https://www.drupal.org/project/role_delegation → into Drupal CMS, rather than https://www.drupal.org/project/roleassign → , given that the former is more actively maintained, and has a wider install base.

This only if there is a need to include anything at all, as per phenaproxima's comment above.

I encountered the same problem recently. Not sure where this got broken, because it seemed to work before.
I can confirm the fix by bisonblue resolves the issue.
MR is against 1.2.x, but this should also be fixed in 1.1.x

Tried testing the MR, but the changes to the modules/ai_translate/src/FieldTextExtractorInterface.php break contrib FieldTextExtractor plugin implementations, such as in "ai_translate_lb_asymmetric" or "custom_field" module that I'm using.
There is no way we can make this work without breaking backwards compatibility?

I think a plugin system for detecting the current environment would be really helpful.
While a lot of people would opt to use URL matching to discover the current environment, I can see other methods being used:
- a config override in settings.php
- a setting in \Drupal::state()
- a system environment variable (retrieved via getenv()) - e.g. set by a CI script targetting a specific environment
- ...

Together with the patch to default_content mentioned above, this MR allows custom_field data to be exported by default_content, and imported by default_content import OR core's DefaultContent API, used by recipes.

Attached is a patch file for the current state of the MR, useful for Composer based patching workflows.

A patch has been added to default_content module, to make sure the Default Content exported YML files use the UUID identifiers of referenced entities within a custom_field field, rather than their (numeric) entity ID.
The patch can be found at ✨ Add support for custom_field module Active