🇩🇪Germany @jurgenhaas

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.

jurgenhaas → created an issue.

Multiple things you should test to find out where exactly it goes wrong:

• Make sure that [entity:id] really exists by using the "Output message to user" action right before you enqueue the task
• Just to be sure, you can also add the [cpbnid] token in that message to verify that it has the expected value
• In the queue worker, you can also add a message output to see if that token exists
• And then, you should note that the queue worker is most likely executed as the anonymous user. So loading a node can easily be prevented due to missing permissions, e.g. if that node is not published yet-

If nothing helps, you may want to use the debugging techniques documented in the ECA Guide to find out which tokens with which values are available for each step in your workflows.

This is looking great, thanks again for your contribution, @ipumpkin

Hey @scott_euser great to see you here, and yes, that's probably something we should discuss together with the Captcha maintainers. I actually see 2 directions here: like you mentioned, using a captcha instead of a block may be something that people would prefer. To keep the server load low in case of a crawler, I guess this should be a blank page with a captcha only, not the fully rendered page. But maybe even that could be configurable.

The second direction I'd like to discuss with Captcha maintainers is that they signal the crowdsec module in case a captcha was not solved successfully. That could just be another pattern to identify malicious actors. Not for the first mistake, but if there are x failures in y minutes, then we may want to block them.

jurgenhaas → created an issue.

Hi @deviantintegral, great to see you around here. Happy you picked up on that blog post and I'll answer your questions as best I can. If you want to go deeper, we could even follow up in a call, or chatting on Slack.

What we're seeing with most of our clients is not that sites are going down, or are offline, due to bot traffic, but that their hosting costs are growing significantly without any real benefit. Since the two biggest Drupal PaaS providers (Acquia and Pantheon) usually charge by the page view or visitor, hosting costs are going up even if the site is quick to return responses.

Both parts can be a concern. Sites going down is something that I've heard more often then not at meetups, dev days, or DrupalCon since LLMs are crawling sites without limits. This can be especially tough when site provide search facets or otherwise lots of valid routes. However, this is maybe not as much of a concern on those PaaS providers but more on mid to low level hosting.

The other concern about growing hosting costs depending on pricing models is something that CrowdSec probably doesn't have an answer for.

I'm curious about this too. If I understand this right:

Your 2 items are spot on, that's exactly it.

Doesn't this mean that we're not getting the benefit of any caches or CDNs in front of Drupal? And, if a hosting provider charges per request, wouldn't the only benefit be reducing CPU usage for responding to long-tail requests where caches don't help?

If a CDN is in place, the request wouldn't get to the Drupal site anyway, in that case you don't need to worry about malicious behaviour.

I would have expected that CrowdSec would be additionally integrated at the CDN level so blocklists could be used to abort requests as quickly as possible (and hopefully reduce origin-server hosting costs).

It is, CrowdSec can be installed on many different levels. And that's what I mentioned in the blog post as well: the earlier in your tech stack where you can block bad actors, the better. However, to learn that some requests (or series of requests) are from such a bad actor, can only be determined at the application level, i.e. in Drupal in this case. Examples are brute force login attacks, crawling non-existing routes, etc.

So, being on the defending side of the operation, a multi-layer approach is what works best if you can afford it.

Anyways, I'd love to understand more how this module is being used to reduce the impact of bot traffic, and see if we can get that included on the project page. Thank you!

Regarding the bot traffic, CrowdSec has a number of specific block lists for bots, but they are not available in their free tier. Except for non-profits, they also provide them in the free plan.

Apart from that, we've seen and heard from others that they had great success in silencing the traffic logs a lot, and as a side effect that didn't just reduce CPU cost but also avoided a lot of error messages in the logs that were caused by invalid requests.

In one case, that was a very popular political forum, they really suffered from bots and crawlers, and the CrowdSec module came to their rescue. However, they then faced another issue that we haven't found an answer for yet: a lot of their visitors come through Tor exit nodes, mainly for privacy reasons. But some bots or malicious actors did so too, and as soon as an exit node was blocked, that also impacted their legitimate users. I guess, that's what the limits are for IP based block lists.

When it comes to improving the project page with more information, please feel free to submit merge requests to the readme.md file in the repository, as that is what we're using for the content on the project page.

Thank you @ipumpkin for reporting this and your contribution. I must have missed this, therefore my late reply.

The MR looks pretty good, I've only changed the public method to protected, as nobody from outside should ever call this.

The test is somehow failing, though. Can you please have a look?

Closing due to missing feedback. The related issue just got merged.

Looks like there's not much interest in reviewing this. I've therefore now merged this and will document that change in the release notes as a potentially breaking change.

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

It does work, I've just tested it with the model above in #4. Every token should be possible to pass along, it just needs to be serializable, because Drupal stores a serialized version of it in the database.

And you use that token under the same name as in the creating model and access its properties just in the same way.

For further details, I recommend the Tokens and Debugging chapters in the ECA Guide.

jurgenhaas → created an issue.

As stated in the referenced issue #3325480: ECA Render: Call to a member function getMode() on null in eca_render_form_alter() → this was never perceived as a bug in ECA, it must have been an error elsewhere. And with the modern environment in Drupal 10 and 11, we need to rely on the method declaration of \Drupal\Core\Entity\ContentEntityFormInterface::getFormDisplay which says that it returns \Drupal\Core\Entity\Display\EntityFormDisplayInterface and never NULL. Therefore, checking for the type of $display is redundant and will never get through PHPStan any longer.

So, there is nothing that needed to be done here. This method will always return the correct value type. If some other module were to mess things up to return something else, this will cause an issue earlier in the process by failing with an invalid return type.

Values are passed forward as tokens. The field "Task value (optional)" is not required in this context.

So, if you add the prelim_nid into the field "Tokens to forward", just the name, not token syntax, then you will have that token under that name available in the queue worker.

Here is a simple example:

langcode: en
status: true
dependencies:
  module:
    - eca_base
    - eca_queue
    - modeler_api
third_party_settings:
  modeler_api:
    modeler_id: bpmn_io
    label: 'AAA Test Queue'
id: aaa_test_queue
weight: 0
events:
  eca_base__eca_custom_04a0t2g:
    plugin: 'eca_base:eca_custom'
    label: 'ECA custom event'
    configuration:
      event_id: q1
    successors:
      -
        id: eca_token_set_value_19bwtqq
        condition: ''
  eca_queue__processing_task_1:
    plugin: 'eca_queue:processing_task'
    label: 'ECA processing queued task'
    configuration:
      task_name: test_1
      task_value: ''
      distribute: false
      cron: null
    successors:
      -
        id: action_message_action_1bxq8zo
        condition: ''
conditions: {  }
gateways: {  }
actions:
  eca_enqueue_task_0l5bmom:
    label: 'Enqueue a task'
    plugin: eca_enqueue_task
    configuration:
      task_name: test_1
      task_value: ''
      tokens: xyz
    successors: {  }
  eca_token_set_value_19bwtqq:
    label: 'Token: set value'
    plugin: eca_token_set_value
    configuration:
      token_name: xyz
      token_value: 'Here is my test value'
      use_yaml: false
      validate_yaml: false
    successors:
      -
        id: eca_enqueue_task_0l5bmom
        condition: ''
  action_message_action_1bxq8zo:
    label: 'Display a message to the user'
    plugin: action_message_action
    configuration:
      message: 'The value is [xyz]'
    successors: {  }

You can add a task to the queue by running drush eca:trigger:custom_event q1 and then run the queue with drush queue:run eca_task.

Merged and back ported to 2.1

You're right, it's the updateLoadedRevisionId call causing the issue. I was just investigating why we've put that in originally, but I can't find any trace. It's been like that since before the first alpha. Can't remember what caused us doing it, but together with the comment there must have been a specific scenario that caused an issue that we solved with it.

On the other hand, we now know that after an entity update event, ECA models shouldn't even try and save that entity again. It would cause a loop, that logs an error, and things should be done differently, when entities need to be changed by ECA models.

That said, I wonder i we shouldn't even get rid of all the revision handling in that update hook, and instead leave it to the system to deal with all of that.

We should assume that such a change could break existing models, and if we publish that, this needs to be documented in a CR.

Thank you so much, this is now merged.

Tested this is 2 scenarios and both are fixed by this MR. Thank you @marcus_johansson

This is great, now I can also update the related implementation in ECA and merge the changes there too. Thank you so much for all the improvements.

Please test and review the MR.

Moving this to the bpmn_io project as this is an issue with upcasting config values that we receive from the canvas, which only always provides strings, but config values may be declared as arrays. So far, the upcasting in this scenario tries a JSON decode, but if that fails it needs to try exploding lines into an array.

No plans for further releases.

If you look into the composer.json of 2.0.4 it says clearly that it allows ECA 2 and 3.

ECA Tamper requires Tamper version 1, and as Tamper still only has a beta release, your composer message clearly states drupal/eca_tamper[2.0.3, ..., 2.0.4] require drupal/tamper ^1.0 -> found drupal/tamper[dev-1.x, 1.0.0-alpha1, ..., 1.x-dev (alias of dev-1.x)] but it does not match your minimum-stability., so your project doesn't allow for beta releases.

The ai_eca submodule will be removed from the AI module. That code went into the stand-alone module here. You can see that being done already in the 1.2 branch of the AI module.

The agents submodule here provides an agents that's able to create ECA models.

ECA 2.0.4 is already compatible with ECA 3.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Thank you @steve.elkins for reporting this. I've taken a different approach, though, so that future changes will not have any impact whatsoever. I've also fixed all other tests, too.

I think the best way forward would be to implement multilingual support into this module.

I assume this is OK by now.

jurgenhaas → created an issue.

Thanks for bringing this up. As flag 5 has no changes compared to the latest 4.x release, this is a no-brainer.

Commenting here just to test the default settings, according to https://drupal.slack.com/archives/C51GNJG91/p1757054651468009

Looking good.

Only one code suggestion, I've added the changed code block to the comment in the MR.

jurgenhaas → created an issue.

I've now added many type checks and added safety checks with messages all over the place. Creating ECA models should now work properly, or meaningful messages should be printed.

Please test and review with the lastest dev release.

OK, this was caused by an issue in the configuration form where the default value was an empty string and the drop down looked like the first item was selected. I've fixed that by adding an empty option to the drop down and force the user to select an item explicitly.

Please test this in the latest dev release.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Fixing issue status.

To test and review this, the latest dev release from modeler_api is required.

jurgenhaas → created an issue.

Thanks for clarifying this @a.dmitriiev, makes total sense now.

Great idea, I'll start working on this soon.

And within the action plugin, we should also directly unserialize the response if schema has been used and the response will therefore be a serialized string.

I wish I could reproduce this. I have Drupal 11.2 and around 100 contrib modules enabled. And this problem doesn't happen to me. So, my suspicion is that some (maybe exotic) contrib module is doing something that triggers this, and it only seems to be related to modeler API but may be caused by something else.

To further work on this, we would have to find the route cause of this. For that, you would have to start with a vanilla Drupal 11.2 installation, add Modeler API, ECA, and BPMN.io and enable them all. The problem should not be present. Then you may want to enable more of your modules one by one, and always run drush cr after enabling another new module. At some point, the circular exception may show up again. Then we would want to know which new module caused this. At that point we can try if we can reproduce this with that module.

This looks good to me, but I haven't tested the content suggestion part of it.

When it comes to the missing delta from the core API, wouldn't it still make sense to fall back to integer zero instead of a NULL? Not a blocking question, just curious.

Good for RTBC from here, leaving it at NR for @marcus_johansson

jurgenhaas → created an issue.

jurgenhaas → created an issue.

That's because DANSE has not implemented that feature yet. The events regarding content entities are about creating, updating, viewing, and deleting them. But translations have not yet been implemented.

Closing this as outdated, since there is no sub-module anymore.

Oops, that comment should have gone into 📌 Add PHP 8.1 tests Active which handles the same topic. So this one is probably a duplicate of the other one, isn't it?

I've just had another look and realized that we don't require the drupal/bpmn_io dev dependency any longer, after the latest refactoring. So, that can be removed from the require-dev section but should remain in the recommend section. After that, tests should be working just fine without any extras in phpstan.neon.

Sorry for the confusion here, I totally ignored the positive side-effect of that refactoring we did before it went into 1.2.x