Gottmadingen
Account created on 1 August 2007, almost 18 years ago
  • Founder, Solution Architect, Senior Developer at LakeDrops 
#

Merge Requests

More

Recent comments

🇩🇪Germany jurgenhaas Gottmadingen

Here is a test model with 2 tools that can be tested with the Tools Explorer from the AI module.

🇩🇪Germany jurgenhaas Gottmadingen

Instead of "mis-using" the custom event, let's introduce a tools event for this.

🇩🇪Germany jurgenhaas Gottmadingen

Oh, sorry, I must have picked the wrong status. Thank you for correcting this.

🇩🇪Germany jurgenhaas Gottmadingen

I've turned this into an MR with the idea to replace the call to \Drupal::service() and to $_REQUEST with proper function calls. While that works for the router, it doesn't for the request parameter. I wasn't able to figure out why and commented that out in line 124. Maybe somebody knows why this isn't working?

🇩🇪Germany jurgenhaas Gottmadingen

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

🇩🇪Germany jurgenhaas Gottmadingen

Those are not use cases, they are only being used to work around bugs that haven't been fixed yet, or features that aren't released yet. And decoration is not the right way to do that.

Fixing the bugs and finalizing features so that they can be published is what's needed. And then, no decoration is needed anymore.

If we allow decoration for a service that we need to have control over, we will end up with support requests when something is doing it wrong. But if we publish proper code, that's never required.

If you still have a use case even under the circumstances that bugs are fixed and features completed, please let us know. The above given examples are not.

So, what are the next steps? Please help with the 2 issues Support Memcached for local caching Active and Support TLS for Predis Needs review to get them over the finish line. In the meantime, you're able to patch the code so that you can continue using this in the unfixed environment.

🇩🇪Germany jurgenhaas Gottmadingen

The message entities do support bundles, but you have to create them at /admin/structure/message yourself, the module doesn't come with any ootb. When I create a template there, this will show up as a bundle in the list. Setting back to NR.

🇩🇪Germany jurgenhaas Gottmadingen

As for the comments in #9 please see my comments in Could Drupal\crowdsec\Client::cache be protected instead of private? Active .

To help finding the best proposal and solution, could you please give it a try if your Redis module would also work if you used rediss:// as the schema for it?

🇩🇪Germany jurgenhaas Gottmadingen

We don't need that level of flexibility. We're implementing required changes such that this service remains fully controlled but still serves all the needs. Then, there is no alter or decoration required.

With Redis, we're now in an awkward situation because the issue at Support TLS for Predis Needs review is not finalized yet. Therefore, there are 2 steps to take in the meantime:

  • Help the other issue getting across the finish line, including the request that it should also support the rediss:// schema.
  • Write a patch and apply that to the Crowdsec module. This gets you into a working solution until we're able to provide an official solution.

Unfortunately, we have to wait for the Redis module to decide which way they want to go. I'll post over there as well, hopefully that helps to get it done. Please do the same as well, so that maintainers recognize the importance.

🇩🇪Germany jurgenhaas Gottmadingen

Merged and back ported. Thank you @pcambra for your contribution.

🇩🇪Germany jurgenhaas Gottmadingen

The design of the Redis cache handling in this module is to allow zero-config when the Redis module is available.

Therefore, before we can move this issue forward, we need to wait for the final implementation of TLS support in the Redis module. And there it would be important to get the Redis maintainers to also support `rediss://`, if that's the only schema that is supported by Symfony cache in this context. Then this should also work for the Redis module.

🇩🇪Germany jurgenhaas Gottmadingen

I'm not keen to do that. The Client is a service, but a critical one and it has been deliberately designed to be controlled by the module itself only. Required changes can be done in the module itself, but I don't want to open cache setup up to being altered in any way, because if that fails, it will have consequences that we don't want to support.

🇩🇪Germany jurgenhaas Gottmadingen

It's better, just left 1 suggestion on how this could be easier to read.

🇩🇪Germany jurgenhaas Gottmadingen

What's the use case for that? Why should we allow to decorate this? In fact, the whole class should be internal, and nothing should be altered really, unless there is a compelling reason.

🇩🇪Germany jurgenhaas Gottmadingen

Doesn't matter. As I mentioned before, we can back port to older versions. But development always works on the latest branch.

🇩🇪Germany jurgenhaas Gottmadingen

The idea is great, but the implementation should be doing it such that we don't have duplicate lines of code just next to each other. The existing fallback should be used if the Redis approach fails.

Please update the MR so that it works against the 1.2.x branch. Development always works in the latest branch and if it's a bug fix, it will then be back ported to previous supported versions.

🇩🇪Germany jurgenhaas Gottmadingen

Thank you @coaston for taking the time to test this explicitly. What's required next is a bug report in the Drupal core issue queue. Don't be afraid, just describe what the problem is, maybe on the level of your manual test only because that demonstrates that it's definitely a core issue. Providing the steps on how to reproduce it will help a lot. Maybe you can then link that new issue here so that folks from here can follow along what's happening there.

🇩🇪Germany jurgenhaas Gottmadingen

I think the scope of this issue is probably not for Drupal core. It feels like this has been addressed for Drupal CMS with the Privacy Track, which resulted in the Drupal CMS Basic Privacy recipe.

In other words, there is simple technical solution to this topic, it requires a fine-tuned setup of the Drupal site, initially and ongoing.

🇩🇪Germany jurgenhaas Gottmadingen

Then this must be a core bug. Because what's happening here, if it were implemented in PHP, is this:

/**
 * Implements hook_entity_view_mode_alter().
 */
function eca_content_entity_view_mode_alter(string &$view_mode, EntityInterface $entity): void {
  $view_mode = 'teaser';
}

If that causes Drupal to render the entity with a title, then that's an issue in core.

🇩🇪Germany jurgenhaas Gottmadingen

@jfeltkamp that's exactly what I was talking about in #3. To render notifications, we've built this to use an entity with a title and a body. And the assumption has been made, that the node bundle page would be providing that, which I still assume is the case is the vast majority of Drupal sites.

I understand that you have a site which doesn't have that, and that's unfortunate. If somebody has a proposal on how we could do rendering otherwise, I'm happy to discuss them and review MRs for this.

🇩🇪Germany jurgenhaas Gottmadingen

Just released 4.1.4

🇩🇪Germany jurgenhaas Gottmadingen

This is either an issue with the view mode you're setting, i.e. if that also contains the title, but the default view mode doesn't, then that's where the difference comes from.

What ECA does with your model is just responding to Drupal core's hook_entity_view_mode_alter(). Everything else is down to default Drupal functionality and configuration of your site.

🇩🇪Germany jurgenhaas Gottmadingen

Those look great, thanks @benjifisher for providing them. I've just reviewed and merged them.

🇩🇪Germany jurgenhaas Gottmadingen

@marcus_johansson this is now ready for review, as discussed in our call. It keeps the original functionality and adds the alternative agent form only if the ai_agents_modeler_api submodule is enabled. That gives people a chance to play around with this new stuff, but it also keeps the functionality untouched for productions systems.

🇩🇪Germany jurgenhaas Gottmadingen

Sounds great, you explained it in a much better way, but that's what I had in mind ;-)

🇩🇪Germany jurgenhaas Gottmadingen

The intention back then has been to provide a kill switch that disables all of ECA as early as possible. And that's in the subscriber, where it comes with no overhead except for that 1 line to check if the switch is set or not.

🇩🇪Germany jurgenhaas Gottmadingen

It was indeed meant as a response to #21. In my view, individual config forms from plugins (components) don't require any enhancements that the modeler requires, e.g. parent. As of the described separation between model owner (ECA) and modeler (bpmn_io or cm), the modeler can add stuff that they require without having a plugin to be bothered with enhancements that are not relevant on the component level.

🇩🇪Germany jurgenhaas Gottmadingen

We already have a kill switch for ECA, which is implemented in \Drupal\eca\EventSubscriber\DynamicSubscriber::onEvent. Would it be possible to move the suggested kill-switch from this MR into that context as well? That way, we have the same purpose combined in the same location, certainly better for long term maintainability.

🇩🇪Germany jurgenhaas Gottmadingen

This may be related to Make custom events re-usable for rendering and other event types Active .

How about taking this even a step further: instead of providing a list of token names, we could also allow for a yaml style mapping of destination token names with values from the current context. That would allow models to use their own token names, but when it comes to re-use custom events or sub-processes, they can map their own tokens names to those that are used by the custom event.

🇩🇪Germany jurgenhaas Gottmadingen

I ran into this before as well, especially when building URLs with a lot of tokens. As a workaround, I used the "Token: set value" action to build the URL and then use that token in the render link action. But supporting longer text strings directly would certainly make sense. I read somewhere that core is actually doing something similar, or maybe the default max length for text fields will even be increased in the field widget.

However, we should define the max length of most if not all text fields to 1024, as @mxh suggested. Perhaps we could do that right here in this issue, without a follow-up later on.

🇩🇪Germany jurgenhaas Gottmadingen

Good catch @boromino, I missed that ID issue completely. Just updated it.

🇩🇪Germany jurgenhaas Gottmadingen

That makes perfect sense. I agree, we should provide that new feature.

🇩🇪Germany jurgenhaas Gottmadingen

We've had an issue at 💬 An option to enable tokens support Fixed where we ended up using 2 tokens that contain the opening and closing brackets as their content. Maybe not the most ideal solution, but it is working.

🇩🇪Germany jurgenhaas Gottmadingen

The architecture of the framework has meanwhile changed a lot. ECA and bpmn_io don't know each other anymore. They "communicate" through the modeler API such that ECA (or any other complex model owner) tells the modeler API which components and component types they have. Modelers, on the other end, also tell the modeler API what component types they provide, and the modeler API then maps the model owner component types to those of the used modeler.

The modeler receives the used components and a config form for each of them as individual pieces. That way the modeler can do with them whatever is needed by the modeler. At that point, the modeler can e.g. inject a parent ID, if that's what the modeler requires.

On the way back, i.e. when a model gets saved, the modeler breaks the model apart and delivers a list of used components and their config to the modeler API, who then maps that back to the model owner, who receives that data and stores it in its config format.

In that architecture, the modeler doesn't store a model as a complete entity. It just passes components, config and context to the modeler API. The modeler has no knowledge about the meaning or the components or the structure and format of the model as a config entity. That's only known and under control by the model owner, i.e. ECA or AI Agents, etc.

This decoupled architecture allows every model owner to work with every modeler, as long as both sides support the modeler API.

Of course, if a modeler only wants to work for e.g. ECA, they can still avoid using the Modeler API, but then they are on their own and need to implement everything themselves.

🇩🇪Germany jurgenhaas Gottmadingen

This is a composer support request, not an ECA bug report.

If composer can't find something, or runs into constraint issues, what often helps is to use composer why-not [PACKAGE] [VERSION], it should tell you why something can't be installed.

I also wonder why you would require the dev release?

And the message in the IS states, that ECA 2.1 is already installed. The lock file has it as version 2.1.7, and now you want to require the dev release. Composer may be of the opinion that 2.1.7 and 2.1.x-dev are in conflict. Or, there might be something else in the dependency tree that requires 2.1.7, which would then prevent 2.1.x-dev to be installed.

Again, all about composer constraints and your project structure.

🇩🇪Germany jurgenhaas Gottmadingen

I've re-rolled the MR, please review again.

🇩🇪Germany jurgenhaas Gottmadingen

Just to clarify, i don't want to collapse a pager. This happens for blocks with a view and a pager, not a pager on their own.

Thanks for merging.

🇩🇪Germany jurgenhaas Gottmadingen

Yeah, performance was a concern to me for a long time. IT's surprising how quick the form loads, though. And I'm even considering to keep forms in the browser, so that we only have to load each form once.

However, the BPMN Property Panel doesn't come with any maintenance effort for us, so we can keep it until we're really certain that everyone will have switched.

For now, it's just a nice infrastructure for development. We can keep the 3.0.x branch functional with the native property panel and can build with the Form API without breaking any of the other development streams.

🇩🇪Germany jurgenhaas Gottmadingen

I've finalized the MR and merged it so that people can more easily play with this with the latest 3.0.x-dev release together with the modeler API. A quick video show how this looks like can be found at https://drupal.slack.com/files/U245N8LMC/F08TCR0ANT1/recording-2025-05-2...

🇩🇪Germany jurgenhaas Gottmadingen

Yeah, the MR needs to be rebased to cover the latest changes of the dev branch.

🇩🇪Germany jurgenhaas Gottmadingen

Ajax, autocomplete, states, and wide range of widget is all supported. Form validation is already in place, just not in a way that each individual form gets validated but the values for each individual plugin by using the plugin's validation. That's already in place for ECA 2, but that will have to be moved out of ECA into the modeler API so that all use cases of this framework will benefit from it.

When it comes to full Form API support, I would say we will get pretty close but maybe not 100%, like e.g. multi value fields could probably be a challenge. I guess, we may have to "wait and see" what the experiments with all of this will highlight.

🇩🇪Germany jurgenhaas Gottmadingen

I had considered that, but we don't need it as the forms will not sure back. We only receive the forms for data input, but the values will be collected in the frontend. Only the save route well write back to drupal, and that's already protected.

🇩🇪Germany jurgenhaas Gottmadingen

As part of the modeler API integration and the additional requirements coming from the AI Agents who will use this framework in the future as well, I went back and looked into this topic and got surprisingly far: I'll be able to replace the property panel with Drupal Form API components which will open an amazing set of new capabilities. I will make this optional, so that users can decide whether they want the legacy property panels or the new ones. Maybe we will deprecate the legacy panel eventually, but for now this seem reasonable.

🇩🇪Germany jurgenhaas Gottmadingen

Merged. I guess this shouldn't be back ported, though.

🇩🇪Germany jurgenhaas Gottmadingen

Merged and back ported into 2.1.x, 2.0.x, and 1.1.x

🇩🇪Germany jurgenhaas Gottmadingen

Another suggestion, since we already check for instanceof DTO: We could add a helper method to the DTO "setWithoutRekey" or "setWithRekeyOption" which takes care of properly setting the rekey state back-and-forth.

I wouldn't do that, because we're not calling writePropertyValue either, we're calling the set method in the interface which may do more things than just calling writePropertyValue. I think what we now have should be safe.

🇩🇪Germany jurgenhaas Gottmadingen

Addressed that in another commit and commented in the MR thread.

🇩🇪Germany jurgenhaas Gottmadingen

Yeah, just saw that and fixed the existing test as well as appended a new test, so that we can test both cases: if the case of the token is equal and if it's not.

🇩🇪Germany jurgenhaas Gottmadingen

For now, I've only implemented the explicit disabling of rekeying for a DTO in the context of this action plugin. Reason being that we don't know where the DTO is being initialized, that could be almost everywhere. And using a special token syntax to disable rekeying is something that won't be used, as @mxh already stated.

🇩🇪Germany jurgenhaas Gottmadingen

MR is ready for review. I've also changed the issue title so that the breaking change will be visible in the release notes.

🇩🇪Germany jurgenhaas Gottmadingen

I think, that's the wrong approach. You should design your model in a way that you are in control of what's happening, and not try something, wait for potential errors, and then make other decisions.

For your examples above that means that you shouldn't provide access to the actions that will lead to any of the errors. Only then, the ECA model should be considered completed. So, if a user doesn't have the permission to add a certain entity to a group, don't allow them to even try.

With ECA you can use conditions to try beforehand and only display the links or buttons to the user when you've made sure that they are allowed to execute those.

🇩🇪Germany jurgenhaas Gottmadingen

Well, you should compare the database records for that entity, and probably its body field in particular, before and after saving the entity from the UI. I wouldn't be surprised if there is a difference in the format column. When populating the body field with ECA, or in your case with webform content creator, then you don't just want to set the body value but also the format field.

With ECA you certainly cannot open something in a browser, save it and get back where you've been before. ECA operates as a black box in the backend, it has no control over your browser.

This question probably even relates to the webform content creator module.

🇩🇪Germany jurgenhaas Gottmadingen

Yes, finding the field names for all entity types anywhere can be tricky. A good place is the "Manage form display" page for the entity and bundle you're looking for, e.g. /admin/structure/types/manage/article/form-display for article nodes. That shows you the machine name of all the available fields.

🇩🇪Germany jurgenhaas Gottmadingen

Thanks @tim-diels for reporting this in the first place and then your follow-ups, this is much appreciated. There's always something to learn from these types of issues, for everybody.

🇩🇪Germany jurgenhaas Gottmadingen

If the showcase goers in the right direction, there are 2 blocks of topics that we need to discuss:

Integration decisions:

  • Is modeler API a fixed dependency?
  • Do we allow different modelers?
  • If no modeler is available, the agent can not be edited?

Tasks to do in bpmn_io:

  • Property panel too high, should be embedded in the canvas and have a scrollable container when it's too high
  • Property panel: replace their forms with Drupal form API components
  • Hide all non-editable components in the property panel
  • Hide non-supported components in the left components panel
  • Auto-fill task label when adding a new template
  • Adding a new tool should start with the template selector, not that the user has to trigger that as a second step
  • Each tool can only be added once
  • Add zoom in/out widgets
🇩🇪Germany jurgenhaas Gottmadingen

@alex.amtr you're using the Classic Modeler and you should ask that question over there, it is a different issue from this one.

🇩🇪Germany jurgenhaas Gottmadingen

Have you tried using dots or periods as separators for the key? That's often how these types of modules are separating the nested keys.

🇩🇪Germany jurgenhaas Gottmadingen

Sorry, I still don't understand. What is a "schema product offer fields of schema metatag"?

Are you able to set a meta tag of a normal Drupal page? How is that then different for what you're trying to do?

🇩🇪Germany jurgenhaas Gottmadingen

I'm not sure I understand the question. Can you please provide more details? I mean, metatags are always string values, aren't they?

🇩🇪Germany jurgenhaas Gottmadingen

I've further updated the MR to fixed the renamed Yaml class. This is now working on our D11 site.

🇩🇪Germany jurgenhaas Gottmadingen

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

🇩🇪Germany jurgenhaas Gottmadingen

This has been implemented and tested.

🇩🇪Germany jurgenhaas Gottmadingen

@rkoller never mind, I just had a couple of minutes and ran it through config inspector. It's all green now. Thank you so much for your contribution, this is great.

🇩🇪Germany jurgenhaas Gottmadingen

jurgenhaas created an issue.

🇩🇪Germany jurgenhaas Gottmadingen

I've rebased this for the upcoming 1.2.x release.

🇩🇪Germany jurgenhaas Gottmadingen

This seems to work. And all tests are green too.

@rkoller do you want to test this with config inspector? You would have to use Drupal 11.2, i.e. the latest 11.x

🇩🇪Germany jurgenhaas Gottmadingen

As I was working with schema validation for ECA recently, I came across the approach to writing a constraint validator in code which will be able to dynamically determine the list of allowed choices. I'll rebase the MR and have a look if I can get this implemented.

🇩🇪Germany jurgenhaas Gottmadingen

The expected enhancements don't seem to require a new major release, a new minor is sufficient for that. So, I changed the target version to 1.2 and created child issues for each of the 3 feature requests that should be implemented.

🇩🇪Germany jurgenhaas Gottmadingen

jurgenhaas created an issue.

Production build 0.71.5 2024