Een hart voor de Drupal Community

Problem/Motivation

This came up as part of the refactoring for #3549153: Translate: use prompt entities instead of custom configurations. In that issue, we transform regular textual prompts being stored in config, to a reference to an ai_prompt config entity, that stores the prompt value and variables information.

In \Drupal\ai_translate\Form\AiTranslateSettingsForm::validateForm() there was already logic to validate that the prompt text has a minimal length of 50 characters.

I'm wondering if this is something that can be generalized: can we assign validation logic to ai_prompt entities of a certain ai_prompt_type, on an entity level?

This would:

- provide a validation framework for similar validation logic (haven't checked if there is similar logic elsewhere in the AI module), instead of one-off code. - already validate the prompt text when the ai_prompt entity gets created, not afterwards when it gets selected. It could then also validate when the prompt gets created outside of the AI Translate settings form (e.g. via code, recipe, general prompt management page, ...) - leverage the Drupal / Symfony validation constraints framework, instead of custom $form_state->setError() calls.

Remaining tasks

- Discuss if this is needed / useful - Investigate where this can be used, within current AI functionality - Provide validation logic on entity / schema level where relevant, instead of via form

Problem/Motivation

For historical reasons, ai_ckeditor stores its prompts in a custom configuration (ai_ckeditor.settings) together with other settings.

Switch to AI Prompt entities for storing default prompts would standardize the settings schema, as well as the logic in the settings form for selecting the correct prompt.

See also #3549153: Translate: use prompt entities instead of custom configurations for a similar change.

Proposed resolution

1. Specify prompt types for the different AiCKEditor plugins 2. Specify default prompt config entities for each of the prompt types 3. Add update logic to migrate from current textarea prompts to AI prompt entities

Remaining tasks

1. Change the logic 2. Write update hook

Problem/Motivation

The `ai.ai_prompt.*` schema definition seems to specify non-existing properties, and could use extra validation.

While working on #3549153: Translate: use prompt entities instead of custom configurations, I used the documentation in docs/developers/ai_prompt_management.md. A few things could be improved / added to this document, to make it easier for future implementations.

@loparev we're waiting for our customer's feedback. Looks like they have a lot of incomplete translation jobs, also very old ones.

Hi @loparev, thanks for having a look at this!

As you already figured out yourself, the key isn't only used for previewing but also for accessing the source URL (JobItem::getSourceUrl). Granting the access is done in tmgmt_content_entity_access. You are correct about the "View published content" permission, when that one is disabled the tmgmt functionality isn't working either. Drupal core seems to force this behavior (NodeAccessControlHandler::access) and doesn't allow alteration. When we want to follow tmgmt strategy we should implement the same access logic on job item, thats why we need the item and not the job alone.

We already tried https://git.drupalcode.org/project/tmgmt_smartling/-/merge_requests/29/d... but that didn't work either. The session cookie that is generated is still lacking an ID, like this: SSESSc2b280e207aa821b24606f896b740d6d=.

Have you tried using the contextUsername with tmgmt anonymous_access disabled? Can you verify if the session cookie is build correctly?

Oops, I didn't catch this either on a clean install.

There must have been something is that was already calling core/once.

Glad it's fixed. Thanks!

Problem/Motivation

As reported in #3548205: Use once in tocbot_init.js

This needs core/once declared as a dependency in tocbot.libraries.yml to work on a clean install

Fons added a comment on drupal.org: Use once in tocbot_init.js

I've added the code to fix this:

https://git.drupalcode.org/issue/tocbot-3548205/-/commit/33b667f6b23574e...

Problem/Motivation

I've noticed tocbot_init.js sometimes fires multiple times.

I've also seen edge cases where it didn't work properly in combination with Bigpipe.

Proposed resolution

Make sure the tocbot_init.js is only running once.

f0ns created an issue. See original summary.

I have been looking into this because it indeed would be an improvement to the module if no empty block:

<div class="js-toc-block"></div>

would be rendered.

The only way to fix this, in my opinion, is to do a DOM check on the contentSelector if it has any headingSelectors and if not don't even bother rendering the div.

Fix PHPStan errors, as reported via Gitlab CI

Fix PHPCS errors, as reported via Gitlab CI

Fix ESLint errors, as reported via Gitlab CI

Fons added a comment on drupal.org: Strip ? characters out of automatically generated id's

https://git.drupalcode.org/issue/tocbot-3548026/-/commit/6c93c476687f659...

Fixes this issue for me.

Fons added a comment on drupal.org: Strip ? characters out of automatically generated id's

Problem/Motivation

I've noted when checking the create automatic ids checkbox with heading selector h2, h3

<h2>Is this a test?</h2>

Would generate an id with a ? in it which is normally not allowed id format.

f0ns created an issue. See original summary.

Fons added a comment on drupal.org: .is-active-li jumps from first to last item on touching the scroll

Casting all properties that are numbers to an int (interpreted as a string otherwise) fixed it for me.

https://git.drupalcode.org/issue/tocbot-3547569/-/commit/4d1066e82927f51...

The only thing I want to prevent are timeouts and errors in our logs. These are caused by uploadAndMatchContextSync because it uses the wait method to check status of the (slow?) matching process on Smartling side.

That's why I would split the functionality up in 2 steps/queues but keep all existing logic, that way we give Smartling the time it needs to finish processing without failing on our side:

1. trigger uploadAndMatchContext, retrieve the processUid and create a new item in a separate queue
2. • queue worker uses DelayedRequeueException instead of waiting in the PHP process itself
   • when process is finished, fetch all information using uploadAndMatchContext again and log messages
   • trigger (improved) uploadContextMissingResources

The major difference it that the waiting part is not done in PHP but in a queue with the correct exceptions, that won't block the queue. Processing of other queue items will continue while waiting for slow ones.

Part of my comment here https://www.drupal.org/project/tmgmt_smartling/issues/3546398 is related to this topic as well:

I think uploading all of these assets one by one also makes the process incredibly slow. Why not use Guzzle http client instead of curl commands directly after which you could easily execute async upload commands for all of those assets: https://docs.guzzlephp.org/en/stable/quickstart.html#concurrent-requests

Hmm, after further investigation, setting this to a non-translatable data type also removes its config translatability. The question is, does this need to be translatable at all?

If not: I feel like this patch is a valid fix, if not: we should find a different solution to store the excluded entities, perhaps in an array (sequence/mapping) rather than a long unbroken string?

Randal added a comment on drupal.org: Long translatable string in user interface translations

randalv created an issue. See original summary.

Problem/Motivation

Upon installing this module, the config item "notifications_widget.settings:excluded_entities" adds a long unbroken translatable string to the user interface translation.

There are two problems with this:

1. The user interface translation table gets stretched out, the left column being incredibly long leaving nearly no space for the content input on both Claro and Gin
2. Config should not be translatable in the user interface translation, the translation is luckily not used anywhere, but it still shouldn't be in there.

Steps to reproduce

1. Install the module
2. Visit the user interface translation page
3. If the string isn't on the first page, look for the default value by searching for: "block,block_content_type"

Proposed resolution

We should change the data type from "text" (which is translatable) to "string" (which isn't).

Data model changes

(Ha, I get to use this heading for once) The "excluded_entities"-key in the "notifications_widget.settings"-config should be a non-translatable data type.

Fons added a comment on drupal.org: JQuery downgrade

Hello thank you for the information.

I just did a quick scan and I don't see any dependencies that are stated in the Sticky module (except for using the latest version of the Sticky library).

{
    "type": "package",
    "package": {
        "name": "garand/sticky",
        "version": "1.0.4",
        "type": "drupal-library",
        "source": {
            "url": "https://github.com/garand/sticky.git",
            "type": "git",
            "reference": "1.0.4"
        }
    }
},