🇧🇪Belgium @svendecabooter

svendecabooter → changed the visibility of the branch 3528586-support-drupal-core to hidden.

OK it should be possible after all I think.

Say we can integrate with the contrib Default Content module, to produce output as follows:

field_logos:
    -
      logo:
        entity_type: 'media'
        entity: eab657e0-3320-41d2-ab4e-53a09b397758
      link: 'https://google.com'
      link__title: ''
      link__options: { }

(if not possible, worst case we need to document the exported YAML files have to be manually edited).

Then in CustomFieldEntityReference::setValue(). we can do something like:

  /**
   * {@inheritdoc}
   */
  public function setValue($value, $notify = TRUE): void {
    $entity = $value['entity'] ?? NULL;
    // START ADDED LOGIC.
    if (is_array($value)) {
      if ($value['entity_type'] && Uuid::isValid($value['entity'])) {
        $entity = \Drupal::entityTypeManager()->getStorage($value['entity_type'])->loadByProperties(['uuid' => $value['entity']]);
        $entity = !empty($entity) ? current($entity) : NULL;
      }
    }
    // END ADDED LOGIC.
    if ($entity instanceof EntityInterface) {
      ...
   }

I can spend some time to look further into this, but am currently time restrained, so just doing some braindump here.
Will try to elaborate on it next week.
Let me know if it would be a workable solution to extend the setValue() logic, for the use case of default content exports / imports.

Did some more digging, and it seems the datatype used in custom_field module is "CustomFieldEntityReference", which does not inherit from core "EntityReference" data type, so guess this will never work :'(

svendecabooter → created an issue.

1.2.2 has been released now

Thanks a lot!

Thanks - committed!

svendecabooter → created an issue.

Thanks for the fix, and sorry for the delay in committing.

Thanks for the fix.
Committed now.

Thanks for looking into this sarvjeetsingh.
It hadn't occurred to me that this could be the source of the issue.
Fix in MR resolves the issue for me.

Added MR that adds this functionality + a patch file for Composer-based patching workflows.
Thanks for considering merging this functionality.

svendecabooter → created an issue.

svendecabooter → created an issue.

Merge request added.
This probably also can be committed to 1.0.x and 1.2.x branch, but I made the MR against 1.1.x, since I'm actively working with that branch.

svendecabooter → created an issue.

Added patch file for the current state of the MR, for composer based patching workflows.

I added a MR to add this functionality.
Whether or not the Provider Configuration fieldset is open by default or not, is managed via config. The default setting for this config is to remain open, to keep backwards compatibility.

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

EDIT: seems to be unrelated to this patch, because I'm getting the same errors when patch is not applied.

Tested with OpenAI, and getting the following errors:

Warning: Undefined array key "fids" in Drupal\media_library\Form\FileUploadForm->validateUploadElement() (line 212 of /var/www/html/web/core/modules/media_library/src/Form/FileUploadForm.php)

ypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count() (line 212 of /var/www/html/web/core/modules/media_library/src/Form/FileUploadForm.php).

Haven't investigated the root cause yet...

All code review issues have been fixed, and extra explanation has been provided.
Setting this to Needs Review.

Thanks for the bug report, MR and review! Merging now.

Added a patch file, to be used in Composer-based patching workflows.

svendecabooter → created an issue. See original summary → .

Rebased the MR on current 11.x to resolve merge conflict.

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

This looks like a duplicate of ✨ Provide config action to set editor plugin configuration Active ?

svendecabooter → changed the visibility of the branch 3526978-add-providerid-context-1.1.x to hidden.

Created 2nd branch with same commit - branched from 1.1.x - since I needed that for my testing purposes.

Ah yes I did add the CSS like in #9 - forgot to document here.

@anjaliprasannan shouldn't the MR be against 1.1.x branch?
Not sure what the branching strategy for AI is exactly, but seems to me active dev is against 1.1.x now.

@sarvjeetsingh:

I'm not sure if that would work - I'm still tipping my toes into how all of this works...

It would be something like this, in the ChatOutput constructor?:

    if ($normalized instanceof StreamedChatMessageIteratorInterface) {
      $normalized->triggerEvent();
    }

In my initial testing, that doesn't seem to work. I then get the same error as in the tests:
TypeError: Drupal\ai\Event\PostStreamingResponseEvent::__construct(): Argument #1 ($request_thread_id) must be of type string, null given

Not sure if that's something that also needs to be fixed, or if this means this approach is not workable...

svendecabooter → created an issue. See original summary → .

Basic integration logic has been committed in 1.0.x branch, but still needs some work.

Test PromptJsonDecoderTest seems to fail now, because \Drupal\ai\Event\PostStreamingResponseEvent::__construct doesn't receive a request_thread_id string in its constructor.
Not sure if this value is NULL in this test by design, or by accident.
If by design, then the PostStreamingResponseEvent $request_thread_id parameter should be allowed to be nullable?

+1 - this fix is needed for ✨ Abstract token usage support Active to work.

Added related issues in AI module, that need to be fixed first, before both token usage for both regular chat and streamed chat can be captured.

MR now triggers the event, when the StreamedChatMessageIterator object gets destructed.

svendecabooter → created an issue.

Seems like the failure of PostStreamingResponseEvent not being triggered, is a separate issue.
See 🐛 PostStreamingResponseEvent never gets triggered Active .

The code in this MR can probably be already reviewed, but will not be functional until this other issue is fixed first.

svendecabooter → created an issue.

Thanks for the merge.
I guess we can now create followup tickets, so different local sites can request to merge in their own config_split.config_split.la_[LOCAL_SITE].yml and appropriate config/la_[LOCAL_SITE] config split folder.

Thanks for the added logic sarvjeetsingh.
I am unable to make this work on my local installation though, as mentioned above.

Test scenario:
- Enable AI CKEditor integration module
- Add AI button to CKEditor
- Enable "Fix spelling" for example
- Select a text in CKEditor that needs spelling fixes.
- The action gets performed, but my debugger never enters the method `\Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent`

Can also be tested by installing dev version of https://www.drupal.org/project/ai_usage_limits → module, and configuring it with token usage limits. The usage stats will not go up in the test scenario mentioned above, even though they should...

svendecabooter → created an issue.

svendecabooter → created an issue.

Fixed in 1.0.x

svendecabooter → created an issue.

Adding some logic to the MR, in an attempt to also provide support for token usage, in case of streamed responses.
This needs https://www.drupal.org/project/ai/issues/3524435 📌 Add token usage to streamed chat Active to be fixed first.

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

I have created a MR to start implementing this functionality.

MR currently contains logic for the first action point:

Add so that you can collect tokens in the StreamedChatMessage, but do not add it to the interface (no breaking changes).

This allows this to be used in the ai_provider_openai module for example:

class OpenAiChatMessageIterator extends StreamedChatMessageIterator {

  /**
   * {@inheritdoc}
   */
  public function getIterator(): \Generator {
    foreach ($this->iterator->getIterator() as $data) {
      $metadata = $data->usage ? $data->usage->toArray() : [];
      $streamedChatMessage = new StreamedChatMessage(
        $data->choices[0]->delta->role ?? '',
        $data->choices[0]->delta->content ?? '',
        $metadata
      );
      if (!empty($metadata)) {
        $streamedChatMessage->setInputTokenUsage($metadata['prompt_tokens']);
        $streamedChatMessage->setOutputTokenUsage($metadata['completion_tokens']);
        $streamedChatMessage->setTotalTokenUsage($metadata['total_tokens']);
        $streamedChatMessage->setCachedTokenUsage($metadata['prompt_tokens_details']['cached_tokens']);
        $streamedChatMessage->setReasoningTokenUsage($metadata['completion_tokens_details']['reasoning_tokens']);
      }
      yield $streamedChatMessage;
    }
  }

}

I am a bit stuck on the second action point:

Add so we can collect it in the PostStreamingResponseEvent

I've been looking through the code in the AI module, but can't find where the PostStreamingResponseEvent gets triggered.
Well, it gets triggered in \Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent(), but I cannot find where that method is called. Is that supposed to be called by the OpenAI provider for example (because it doesn't currently)?
I'm a bit confused as to what the exact code flow should look like...

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

svendecabooter → created an issue. See original summary → .

It does seem to work if I add it like this in `mytheme/css/themes/fancy.pcss.css`:

:root:has(input.theme-controller[value=fancy]:checked),[data-theme="fancy"] {
  color-scheme: "light";
  --color-base-100: #FAF7F2;
  ...

As per documentation at https://daisyui.com/docs/themes/#how-to-add-a-new-custom-theme, under "If you're using CDN and you want to use a custom theme, use it like this".

I would create a MR to add this to the documentation, but I'm not sure if this is the correct way to implement a custom theme...

esmoves → credited svendecabooter → .

esmoves → credited svendecabooter → .

I was trying to add my own custom theme, and followed the following process:

- Create a new theme based on the starterkit - as per `starterkits/ui_suite_daisyui_starterkit/README.md`
- Run the `npm install` and `npm run build` commands
- Add new theme file in `mytheme/css/themes/fancy.pcss.css` with contents like this:

@plugin "daisyui/theme" {
  name: "fancy";
  default: true;
  prefersdark: false;
  color-scheme: "light";
  --color-base-100: #FAF7F2;
  ...
}

- Add file `mytheme/mytheme.ui_skins.themes.yml` with contents like this:

# No value because same as plugin ID.
fancy:
  label: "Fancy"
  label_context: "color"
  key: "data-theme"
  target: html

- Run the `npm install` and `npm run build` commands again
- Go to `/admin/appearance/settings/mytheme` and select the "Fancy" theme from the "Theme" dropdown

The CSS file for fancy theme is included in the HTML output, and my tag contains `data-theme="fancy".
I can't see any of the colors though...

Is this because I use hex color codes, instead of oklch()?
Or anything else in the process I missed?

svendecabooter → created an issue.

svendecabooter → created an issue.

Thanks for your work!

Rebased MR on the latest 1.0.x code.
Not sure how to fix the failed pipeline though...

Thanks for the summary and both for your work on this!

I created a MR to add this functionality.
For the selectable prefill attributes for form elements I didn't add an option, as this needs to be configured per field widget manually anyway, AFAIK.

Attached is also a patch version of the MR changes, for Composer based workflows.

svendecabooter → created an issue.

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..

Thanks for the notice.
Since this module has more usage and already has some releases, I think it would make sense to deprecate your module, and provide its extra functionalities in this module. I'd be happy to review merge requests to get extra functionality in.

Sorry for the delay. Had a bit of time to review this now.

This does allow the user to select the type of message to send, but only when the action plugin is triggered.
When editing a user and clicking the button shown on the form, the old behaviour is still used.
So that introduces 2 different behaviours, according to the path you take to trigger it.

Also, would it be an option to reuse the functionality that now decides in code which mail to send, to provide a default value to the user? Some editorial users might not know which email is the most appropriate, so guiding them in the right direction would be good.

The next version of the module will no longer support Drupal 9, so marking this ticket as outdated.

Thanks for the MR.
Updated code a bit, and committed.

Thanks for the feedback michaelsoetaert.
Closing this issue then.

The value is expected to always be an array, either through import of the config via /config/install, or for existing sites, by running database updates. Specifically layout_builder_iframe_modal.post_update.php method layout_builder_iframe_modal_post_update_custom_routes_config() should make this an array, if it isn't already.