🇩🇪Germany @jurgenhaas

The MR is ready for a test and review.

OK, I've tested this. When we load the config with ECA::load(), modify it and save, the dependencies come back. As expected.

But the good news is, if we load the config entities with \Drupal::configFactory()->getEditable(), then we can modify and save them without any API interfering.

I'll create an MR to fix this.

Oh, we never thought of that correlation. What's happening is that ECA models that leverage any of the AI ECA plugins have a dependency on the ai_eca module which gets disabled during the update hook. So, what we should have done, and probable still may have to update, is that the update hook loads all existing ECA models, and removes the dependency on ai_eca before disabling that module. I hope that even works, and Drupal config management allows us to do that and saving it back without automatically adding back that dependency during the pre-save phase.

Moving this over to the AI module, since that's the place where this update hook lives.

@maxilein you should be able to recover from this situation on your sites by bringing back the ECA models from CMI or a backup, and before importing them again into Drupal, removing the single line ai_eca from the module dependency section.

@bigtomfelix any chance for some feedback on this?

The cache was not in place because it was a site:install via recipe.

Does that mean that if the cache is in place that the warning goes away?

Thanks @phenaproxima for the code improvements, I like them. I've pushed another commit to update modules/config/src/Plugin/Action/ConfigActionBase.php for consistency and modules/config/src/Plugin/Action/ConfigWrite.php to only loop over the values when an existing config gets updated, but not when we create a new one.

Please review and hopefully test this together with your related issue in Drupal CMS. When set to RTBC, I'm happy to merge this.

We intended to use isAjax in this issue 🐛 Ensure sticky action buttons to work with modals and ajax refresh-calls Needs review where we went through a huge number of contrib modules that had certain issues with sticky actions.

For some reason we jumped short, but I can't find the reason why.

So, we should consider doing that now. In order to do that, would anyone mind going through the test cases in the related issue to verify if changing to isAjax is not breaking any of the cases tested there? And then setting this issue to RTBC?

@unqunq I've looked into this and found a ocuple of issues:

• You're passing form_state around, but that's not used, neither in buildFormMetadata nor in generateProviderAgentForm
• The method generateProviderAgentForm is only called once from buildFormMetadata. I think that's not necessary, you can just add the content at that place instead of calling yet another method.
• The template provided in the description for the field ai_configuration is invalid, when being used, the form can not be saved. The keys need to be double quoted to be valid.

After that, I looked why the form values are not being saved. The reason for that is, that buildFormMetadata is called by providing the $config as the second argument. So, you don't require the entity in either buildFormMetadata or generateProviderAgentForm, instead all the current (or default) values get passed into buildFormMetadata inside the $config argument. And that array needs to be used when setting the #default_values for each of the form fields.

You can see that in \Drupal\ai_agents\Form\AiAgentForm::form where the method is called like this:

    $form = $this->buildFormMetadata(
      $form,
      [
        'label' => $this->entity->label(),
        'id' => $this->entity->id(),
        'description' => $this->entity->get('description'),
        'orchestration_agent' => $this->entity->get('orchestration_agent'),
        'triage_agent' => $this->entity->get('triage_agent'),
        'max_loops' => $this->entity->get('max_loops') ?? 3,
        'system_prompt' => $this->entity->get('system_prompt'),
        'secured_system_prompt' => $this->entity->get('secured_system_prompt') ?? '[ai_agent:agent_instructions]',
        'default_information_tools' => $this->entity->get('default_information_tools') ? Yaml::dump(Yaml::parse($this->entity->get('default_information_tools') ?? ''), 10, 2) : NULL,
        'structured_output_enabled' => $this->entity->get('structured_output_enabled') ?? FALSE,
        'structured_output_schema' => $this->entity->get('structured_output_schema') ?? '',
      ] + self::defaultConfigMetadata('id'),
      'id',
      $this->entity->isNew(),
      $this->moduleHandler->moduleExists('token'),
      $form_state,
    );

There you need to add the values for the new fields and then use them when defining the extra fields.

Also, you need to provide the new field and their defaults in \Drupal\ai_agents\Form\AiAgentForm::defaultConfigMetadata.

Let's try again after those changes have been made.

jurgenhaas → created an issue.

How can I loop through this array and trigger a custom event (entity aware), passing the Task entity and the user id to the custom event?

The entity aware custom event is deigned to work with a list of entities. Not with field values or any other array/list. For lists of any kind you can use loops like e.g. in Multi value field loop and either process each item inside that loop, or call ECA custom event inside the loop for each of the items that you poped off the list.

This is great. I've just removed the first heading as it would duplicate what's already on the page. I also added some links to the external platforms. Now merging.

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

jurgenhaas → created an issue.

hook_theme_suggestions_HOOK_alter should be covered:

Oh, I'll give that a try. Maybe worth an update of the IP then?

Started a major refactoring to leverage OO Hooks from 📌 Convert theme hooks to OOP Active . To test this out, the patch from MR !13148 in that core issue needs to be applied.

All available hooks have been converted into OO Hooks, and the following have been aggregated into new files:

• gin.preprocess.inc: contains all preprocess functions until there is support in core for them as well
• gin.suggestions.inc: contains all theme suggestion hooks until support will be added to the MR above
• gin.theme: contains all helper methods that will next be moved into some of the available classes

The includes directory with all the theme files now got removed.

Tests are currently failing because of the missing core patch.

We should probably merge this MR soon so that we don't have to double-check all the optimisations from other issues. This would mean that we have to always apply the patch to core while working on the 6.x branch for now.

Went through the "Problem/Motivation" and compared it with what we're working on while merging Gin into core by also incorporating all from Claro.

I found 2 missing hooks that are currently used by Claro and that are not in the hook list above:

• hook_theme_suggestions_HOOK_alter
• hook_views_pre_render

Then, I also found 2 special cases of library_info_alter and theme_registry_alter:

• gin_system_module_invoked_library_info_alter
• gin_system_module_invoked_theme_registry_alter

Are they supported as well? I mean, they're special as the hook is not just gin but gin_system_module_invoked. The comment in the current 5.x code reads like this:

/**
 * Called by system.module via its hook_library_info_alter().
 *
 * If the active theme is not Gin, but Gin is the admin theme, this alters
 * the toolbar library config so Gin's toolbar stylesheets are used.
 *
 * @see system_library_info_alter()
 */

Other than that we also have 51 preprocessors in Gin and Claro collectively. I guess that follow-up doesn't exist yet?

@maxilein would you mind providing this as an issue fork with an MR? That would allow us to review the code and also let the tests run on it.

Should this issue be moved to ECA then?

No, I think we should create a module eca_simplenews and move this issue there.

With regards to the routes, I don't think they are relevant for any ECA related activity. Routes are there to provide links for interactive usage by a user. When it comes to automating things, we need to look at what's happening when such a route is being visited and how that same functionality can be achieved without clicking a link.

Maybe the biggest problem is that simplenews doesn't appear to have an active/inactive setting for individual subscriptions, just for subscribers.

Automation, either by ECA or otherwise, can never achieve anything that isn't provided by the underlying technology in the first place. We shouldn't even try. If such an active/inactive flag is required for subscriptions, then simplenews needs to provide that as a feature first before we can start thinking on how to leverage that in automation.

This may be because the notification generation and the delivery are both being executed asynchronously. So, they get processed during cron runs on by explicit calls to drush commands that do that processing.

Changed this back to 2.0.x as the 2.1.x branch isn't used at this stage any longer.

While the "fix" certainly makes sense, I'm wondering if we have a more generic issue here. Why should this be related to that one plugin configuration only? Yes, having a TAB character being more likely to be used in the context of trimming, isn't the same also possible elsewhere? Receiving string values from HTML input forms is what's causing the issue here. I wonder, shouldn't all string for all fields in all plugins being unescaped?

I've committed this to the 3.0.x-dev branch, please give it a try.

Moving this to BPMN.io as the problem is that HTML doesn't allow tokens to be used in number fields. Since we've changed to using the Form API in v3, we now have the benefits of proper input fields. But that comes with limitations like this.

The solution will be that we change number fields to a text field.

Hmm, I can't reproduce this. The schema for third party settings in ECA models is defined in \Drupal\modeler_api\Hook\EntityHooks::configSchemaInfoAlter and it works just fine. It sounds like you may have to rebuild cache?

Hundreds of different sub-themes of Claro, really? Hmm, that's beyond imagination ;-)

But themes can also implement hooks, can't they? And each of your themes could have the same hook that adds its own name to the list of the supported ones. Doesn't that work?

@anybody that sounds like a good plan. Also, we could probably open an issue in the upstream server repository to ask if and how that code would have to be updated to work with v2.

From the stack trace it looks like there is also the auto_save module in action. It appears as if it is running into a loop together with the address field. Could you try and disable that module to see if that makes any difference?

This is on purpose; the registration form is only altered for admin users. See 🐛 Incorrect password on login Active . If you want that model to behave differently, you can edit it and make it work as you want it to.

Haven't tried this just now, but I suppose you can use an action to e.g. render markup or a link and store that in a token by providing a name in the field "Token name" and setting the "Build mode" to nothing. Then you can embed that token elsewhere afterwards.

Ah, the unit tests exposed an interesting issue, not in the modified implementation but in the test. This seems to confirm, that the new code works as expected, after I have adjusted the test.

Uh, how did I miss that one? Thanks @phenaproxima for the hint.

I've implemented this in the MR but haven't tested it yet.

The tests are now fixed. Once the 2 required traits ContentTypeCreationTrait and BodyFieldCreationTrait will be in current Drupal, those should be removed from Drupal\Tests\eca again and namespaces in use statements need to be updated.

jurgenhaas → created an issue.

I think it's essential that we keep the local endpoint. The changes they seem to have made are pretty significant (https://developer.friendlycaptcha.com/docs/v2/versions) but the server repository (https://github.com/FriendlyCaptcha/friendly-lite-server) hasn't been changed recently.

Also, v2 is not available for all of their customers yet. User would have to find out if they're eligible or not.

All in all, it is probably a bit too early to jump. But if we do, it should not be disruptive. Unfortunately, I don't have time at the moment to help, maybe that changes later this year.

I was looking around but couldn't find a way to get the entity type from the config prefix. Examples: config name views.view.test1 is a views config entity. But the config name modeler_api.data_model.test1 is a DataModel entity from the modeler API with the entity type ID modeler_api_data_model.

@phenaproxima do you have something in mind on how we could find out if a config name represents a config entity?

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Great report, and thanks for finding the reference to 🐛 ECA Add Ajax Handler keep refreshing and scroll page back to Ajax area Fixed . That was exactly the same issue and back then it only handled some specific text fields, but not number fields.

As we may not know exactly what other input fields may come along with similar issues, I've tested this to disable re-focus on all fields and that seems to be resolving it in general.

Fixed in 3.0.x, will be back porting this to 2.1.x next

Thank you.

I'm not entirely sure where the problem is coming from, therefore I can't recommend any next steps. I guess, this requires some research to find out what exactly is causing this.

Merged this and also ported to 4.1.x and 6.0.x

Thanks @pameeela for opening the new issue 🐛 Dialog styles are not loading correctly in Canvas Active , merging there right now.

This is a great write-up, thanks @rclemings

Here a few comments:

I don't think that hook_simplenews_issue_operations and hook_simplenews_subscription_operations are legitimate hooks. What they do is covered by the hook_entity_operation hook provided by Drupal core. And that's already implemented in ECA.

Other hooks like hook_simplenews_subscribe, hook_simplenews_unsubscribe, and hook_simplenews_mail_result_alter are probably useful and could be turned into events that ECA can leverage.

When it comes to action plugins in simplenews, they look as if they can be leveraged by ECA.

Everything that's a content entity can already be used by ECA as well, just like you already stated above.

1. An "activate subscription" action. Simplenews has "hook_simplenews_subscription_operations," which includes both activate and deactivate. Can ECA use those? If not, how can they be converted to ECA actions?

The operation hooks are not doing anything. Action plugins to add or remove a subscription may be helpful, but maybe that can also be achieved by creating or updating a subscription entity? It depends on what will have to be stored in the database to reflect the subscribe/unsubscribe action.

2. A "deactivate subscription" action. Same comment.

See before, could probably be done by editing the subscription entity.

3. Some way to parse the Simplenews subscriber history entity and identify subscribers who have previously unsubscribed (and therefore should not be resubscribed). Still digging into how that works.

I'd assume that this should be possible by parsing the field values of the subscriber history entity. Maybe the token browser helps a bit with understanding the data structure?

When it comes to implementation, it's probably best to create a separate module eca_simplenews for this, so that the simplenews maintainers don't have to worry about it.

There is no difference between dark and light mode when it comes to button placement.

What I found is that Drupal core is trying to be smart here. Have a look at \Drupal\locale\Form\TranslationStatusForm::buildForm: the action button is only shown when there are actually updates available. And I found out on a different site, that the button was originally not there, then I checked for updates manually. After that, the button was still missing although there was at least one language with available updates. What fixed it was a clear cache, after that the button shows up as expected.

I can't reproduce this. The button is available in the sticky action bar (top-right corner) like all action buttons:

#32 yes, I was just uncertain if it's the MR!53 is the right thing to merge and if that's all that's needed to fix the bug. @pameeela could you confirm this and RTBC?

I've reviewed the execution of an agent in Drupal when called by AP. It does respond with an array that contains the success flag and either the result data or an error message:

[
  "success": true,
  "result": ... some data ...
]

[
  "success": false,
  "error": "... an error message ..."
]

However, the response code is always 200. Question: should the response code be changed to 400 in case of not being successful?

This is an interesting one because the role "authenticated" is not assigned to any user. It is only used to determine permissions, i.e. you can give that role specific permissions and a user has those permissions when they're logged in (=authenticated) but not when they are logged out. But in the database, there is no user who has that role assigned to them.

What may be interesting selection criteria may be these:

• All user accounts
• All active accounts
• All blocked accounts
• All accounts with active sessions

I guess, the second or fourth may be what you're looking for?

Thank you @megachriz for creating this issue and being proactive in getting ECA tamper up to speed. I didn't notice the meaning of the tamperable item before. Now that I looked into it, I probably would have implemented that with a separate method setTamperableItem() instead of using an optional argument with the tamper() method, but that's probably too late by now.

The implementation in 📌 Make Tamper plugins tell if they require a tamperable item Active looks good to me.

I think we could already implement your suggested solution here in eca_tamper without having to rely on the upstream implementation. Please have a look, I think the MR should work.

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

Correcting the state.

Would be amazing if a patch release with this schema could be made available.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

I've created the second MR!623 based on the previous MR!619, removed the ECA code, and updated the ECA model to use the plugins with the correct IDs from the ECA module.

Thank you @phenaproxima for the MR, I've enhanced it a bit to match the ECA common practices with naming and namespaces.

Regarding functionality, I've made 2 enhancements:

• The action plugin has an access callback which only grants access if the event is correct and the YAML definition is valid. The execute callback then can take those 2 facts as given and set the definition is the event with the decoded YAML, so that the event doesn't have to do that. This way, we won't get any exceptions at runtime, and we also get a log entry with information if the access to the action is not granted.
• In the event, when setting the definition, we also have to set the group so that in case of a new breakpoint, this value is present as well, otherwise the responsive image form fails.

Do you want to have another look at this?

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is looking great, well done @phenaproxima. I only left a couple of suggestions.

IN addition to that, we could add token support to the action plugin, but that's not a dealbreaker at all.

Rebased and re-rolled the MR for OO hooks.

I don't know what you mean by that. The message is created as a regular Drupal regular message, which is delivered in response to an Ajax request. There is nothing special implemented by BPMN.io other than the extra comfort that the message can be clicked to be removed.

Thank you @dan_metille for proposing this enhancement.

However, I'm not convinced that such an extra component is required. As you refer to BPMN, there it is the case that the component type LINK can optionally have a condition assigned to it.

If you're working with another tool as a front end, then this should still be possible by having something like this:

In the first example, the condition is attached to the link between task 1 and task 2.

In the second example of that screenshot, the condition is a complex object constructed of 2 links and a condition element in between.

Both variations can be built in the UI, I believe. Is that something that you could work with?

Well, it's probably time to move away from Claro and use Gin, that's what I wanted to indicate in my previous comment. Just note, that Claro is going away, just saying.

bpmn_io comes with a JS script that prepares incoming messages with this:

  Drupal.bpmn_io.prepareMessages = function () {
    $('.messages-list:not(.bpmn-io-processed)')
      .addClass('bpmn-io-processed')
      .click(function () {
        $(this).empty();
      });
  };

Maybe Claro has a different markup for messages meanwhile, but it used to be the same previously. But if the messages have indeed a different markup, then you may want to extend the selector to cover those from Claro too, if you still can't switch to Gin.

I expected the common UX where one is redirected to the clone form with machine name set to [agent_id]_clone and a chance to edit it before saving the new agent.

Such a form doesn't exist, unfortunately. Other than Modeler API there is nothing in Drupal that offers the clone of a config entity, AFAIK.

Would having to deal with many funky machine names not eventually lead to some unfortunate confusion?

So far, my impression was that if people are obviously working in the UI that the likelihood for them being confronted with machine names is pretty minimal. Therefore, I didn't pay any attention to making that ID more meaningful. Where do you expect the machine name to become relevant?

If we come to the conclusion that we have to ensure user controlled machine names, then we can open a new issue as a feature request.

That's looking good to me now.

There have been follow-up issues in modeler API 📌 Extend field validation to also allow for arrays Active and ECA 🐛 Fix schema for eca_entity_diff action and condition Active for this. I've fixed them both and will therefore now also merge the fix here and close the issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Looks like nobody has an opinion on whether this needs to be back ported. Marking as fixed for that reason.

This is done on purpose. If you click on that message is should disappear.

In Gin, this is optimized already, and as Gin becomes the default in Drupal core and Claro will be deprecated, there is probably not much sense in trying to do much more on this right now. If you think otherwise, please feel free to provide a merge request.

jurgenhaas → created an issue.

The AiAgentForm is now refactored such that the agent's "metadata", i.e. all config fields that are not tools related, is being moved to a new method buildFormMetadata which is called from both \Drupal\ai_agents\Form\AiAgentForm::form and from \Drupal\ai_agents\Plugin\ModelerApiModelOwner\Agent::buildConfigurationForm. So, all changes that will be made in buildFormMetadata are effective for the traditional edit form and the modeler.

I've also introduced \Drupal\ai_agents\Form\AiAgentForm::defaultConfigMetadata which provides the default config values for those fields. That's useful at many places, we should keep that maintained and in sync with the metadata form.

That's great, thank you @dr-gubó

Well, there must be some information somewhere about the cause of the 500 error. It's just about finding out in which log at what location.

This has been resolved. The new machine name is a random string which is necessary as we can't just use the same ID as before.

You can ignore those errors, they are just telling you that a plugin uses a field widget that's not supported in the bpmn_io UI. I looked into the "Moderated Content Bulk Publish" module but I can't find a field called current_translation_only there, so that must be from some other module.

However, I found an issue 🐛 Argument #3 ($currentTranslationOnly) must be of type bool, null given Active where you provide a patch which introduces such a config value, but there is no config form field available for it. Maybe that's why this is causing an error message.

Thank you @bisonbleu for reporting this. It's a modeler_api issue, I guess. I'll have a look.

I've also updated the readme and the project's main page.

Thanks for your feedback @svendecabooter

MR targets 2.0.x and I've tagged this such that it should probably be back ported to 1.2.x as well, I guess.

jurgenhaas → created an issue.

I've fixed the tests and merged the MR.

That sounds very sensible yeah - and at least to me makes sense to have a very minimal page, e.g. like cloudflare managed challenge is.

Yes, something like that is what I had in mind.

Would you imagine a plugin system where we contribute plugins to the various captcha modules?

Not sure we even need new plugins, I would hope that we could just use which ever captcha configuration is already available.

(And of course also contribute to them sending signals as you mentioned)

For that to happen, we first need to provide an API in the crowdsec module, and then it should be very easy for other modules to send signals.

There must be some logs somewhere. Without more details, we can't do anything about this. It's working elsewhere, so there must be something wrong with either the Drupal or the PHP installation where this is happening. It could be Windows, I've seen strange things happening there. Please find more logging information somewhere in your system, only then can we have a look and help.

Started with a simple approach, it's working but we can improve on it.