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

Merge Requests

More

Recent comments

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This is great, thank you very much for your contribution.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

@aoturoa that's correct. However, we should not assume that each core on a host has the same credentials. It should be working with its individual credentials, if present.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Webforms are not using Drupal's form API, that's why the form events in eca_form are never dispatched when working with a webform.

The eca_webform modules comes with some webform specific event. Maybe the Alter submission form event is working in this scenario? You should then also add some message actions into your model so that you can test if the event is being dispatched and also after the action plugin to see if that action got executed successfully as well.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I had applied a small change when executing the command, so that it shows in the console the URL that needs to be loaded in DRD to finish configuring the agent.

That's a good idea, but it needs a bit more work. Could you please rebase the MR so that it contains the changes from the dev branch and then make sure that it tests first that $values['redirect'] exists? If it doesn't, an error message should be reported, otherwise the success message should explain what to do with the printer redirect URL.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I tried testing drush eca:update

That drush command is not involved in any of the update hooks. That's for a different purpose.

What you need to test is to run drush updatedb and you should see, that it will execute the eca_form_update_8003 update hook. For that to work, you don't need to manually edit any yaml file, that should all be done by that hook.

Also, please fix the pipeline error before setting to "Needs review".

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This is looking good except for one minor issue. Also, let's do bug fixes against the latest dev release and then back port.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Thanks a lot for your feedback.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

The event should implement a setValidationErrorMessage method which can be called by an action plugin called Set validation error or something like that.

The validation plugin, which dispatched the event, should afterwards receive that error message from the event and signal that upstream, if there is any.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This doesn't like an ECA issue. Something accesses the cache collector from Drupal core. That's a core service that receives the cache backend when being initialized. That cache backend is supposed to be NULL according to the error message you receive.

I can only think of some misconfiguration on that Drupal site.

If you doubt that's the case, please provide detailed steps on how to reproduce this issue on a fresh Drupal site.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

@cbccharlie the usage of the drushsevices.yml file is deprecated and not required by Drush 12 and later.

However, I just noticed there has been a different issue that prevented Drush from discovering the commands: the class name must end with Commands. I've fixed that in the dev branch. Please have a look.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Do you need to store them? The problem is not about storing them but the fact that the message needs them as a string, when you want to display the list.

If you take the list and then loop over that list, without displaying the list on screen, then you should be fine already, aren't you?

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Thanks for the clarification @mxh I was really off-track here.

In order to hook into the validation of TypedData and Entities, we should be able to provide validation constraint plugins for each data type. They should be discovered automatically according to https://www.drupal.org/docs/drupal-apis/entity-api/entity-validation-api... β†’ , and we could dispatch an ECA event, from which a model can take over and do whatever is needed.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This is something that needs to be addressed with a module that provides the email action plugin.

If you're using the email plugin from Drupal core, that only support very simple email functionality. And recipients can only be provided in the To field. There you could send individual emails per recipient, where they don't see their email addresses either.

For more complex email functionality, the easy email module β†’ is recommended. There you can define email templates with almost all supported fields for emails.

Other email modules (e.g.Symfony Mailer) are available too, but I don't have personal experiences with them.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I've committed that to the dev release. Please test and review.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Moving this over to the bpmn_io project since the route bpmn_io.add is defined there and requires the implementation of the title property. The title could be static, e.g. "Create new ECA model with BPMN.io". This could be added as bpmn_io.add.defaults._title

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This is caused by the usage of the message list-related = [list-related] which has to Yaml-encode the complex content of the list-related token into a string. As this token contains objects that can't be encoded, this exception is being thrown.

The Yaml encoder comes from Drupal core which uses the Yaml dumper from Symfony. I am not sure if and what they changed in that regard, but serializing a list of entity references is probably something that can and should be avoided.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I've updated the MR. The module name is now determined from the info.yml file name. It also verifies that there is exactly 1 info.yml file available, and aborts otherwise.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

we could use code to find the name of the root module from the info.yml

Fair enough, I'll give that a try.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Some modules may have multiple submodules.

That's a great point. However, the situation might be more complex. Some submodules are optional, i.e. they can only be enabled if a certain dependency is available, where that is deliberately not a hard dependency. In such cases, the test can't just enable all submodules. I think, this should be addressed in a separate issue and carefully thought through.

Should this be a hidden variable?

Good question. I'd be OK to make this hidden if that's what we want to go for.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

@jsutta thank you for your input. Contributions to this project are very welcome.

In this particular case, the extra feature is not required, as the functionality is already available: for every condition, not only the arbitrary comparison, there is a flag Negate condition. If you set that to Yes, then you already get the "Not equal" condition. That is very flexible because you can use it with all the other comparison types as well, and of course with all other conditions in ECA.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This module doesn't implement any action plugins itself. It makes all the tamper plugins from the tamper module β†’ available as action plugins to ECA. So, if such a plugin you're asking for is available in tamper, it will then automatically also be available to ECA. If I don't misunderstand, there is nothing to do for the eca_tamper module.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Hmm Drush 11 is EOL since November last year. Why not upgrading to Drush 12?

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Turns out, my workaround only works on PHP 8.1, it fails on PHP 8.2 with reflection class exceptions.

However, that leads towards a proper solution, that I didn't see before: for Drupal core <10.3 we need to remove the attribute usage from the validation plugin and bring back the annotations. That needs to be done both in Drupal core and in the contrib book module.

Then we need the following releases:

  • Drupal core: we need 9.2.8 with that being fixed
  • Book contrib module: 1.0.1 with that being fixed and a version constraint on Drupal core for >= 10 and <= 10.3
  • Book contrib module: 1.1.0 and 2.0.0 with the attributes left in but a version constraint on Drupal core for >= 10.3

As a work around for site owners: use the BookOutlineConstraint.php file from #3 and add the 2 scripts to the root composer.json:

{
  "scripts": {
    "post-update-cmd": "cp config/BookOutlineConstraint.php web/core/modules/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php && cp config/BookOutlineConstraint.php web/modules/contrib/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php",
    "post-install-cmd": "cp config/BookOutlineConstraint.php web/core/modules/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php && cp config/BookOutlineConstraint.php web/modules/contrib/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php"
  }
}

This works for Drupal core < 10.3

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

jurgenhaas β†’ created an issue.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

jurgenhaas β†’ created an issue.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

WRT the real solution, the version constraint for this module should have been that it only works with Drupal core >= 10.3 because the used constraint plugin uses an attribute which isn't available in earlier Drupal core versions. Problem is that this can't be corrected now. Even if there were a new release with that constraint, the 1.0.0 release would still be found by composer and then used for the older Drupal core versions.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

For anyone who wants to use the same workaround, attached is a copy of the working constraint plugin, remove the .txt extension after downloading it. Then copy that into the config directory of the project root. In composer.json add 2 scripts:

{
  "scripts": {
    "post-update-cmd": "rm -rf web/core/modules/book && cp config/BookOutlineConstraint.php web/modules/contrib/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php",
    "post-install-cmd": "rm -rf web/core/modules/book && cp config/BookOutlineConstraint.php web/modules/contrib/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php"
  }
}

That way it works for now, but it's not a solution to the general problem.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

We're running into the same issue on Drupal core 10.2.7. Here is what happens:

  • in D10.2.7, the book module is still in core and uses annotations for validation plugins
  • there is now also a public release of the book module, which works with all Drupal versions >= 10
  • the json_api_book module requires the book module which came from core in the past and should be coming from the contrib module in the future
  • because of json_api_book, composer now also installs the book module from contrib
  • as a result, we have the book module with the same namespace twice: one time in core and another time in contrib
  • the contrib version uses attributes instead of annotations, and that's why Drupal doesn't find the plugins

Workaround, until we can upgrade to Drupal 10.3:

  • delete web/core/modules/book after composer update or composer install
  • apply a patch to bring back annotations into the validation plugin in contrib
πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

@mlncn that sounds great, let me try getting you going.

The OP talks about the validation handler of an entity type, while you're now mentioning validation plugins. Those are two different things, but we could probably leverage both.

With the validation handler, I could imagine that we implement an ECA condition which takes a content entity as an argument and runs it through its validation handler. It returns TRUE or FALSE, depending on the result of the handler. If it doesn't have one, it falls back to TRUE. Note, there is also always the negation flag for those result, so that should also apply to this result.

With validation plugins, they are applied to distinct values, e.g. field values. So, to utilize a validation plugin, one needs to select the plugin and pass it a value that should be validated. Please correct me if I'm wrong. So, if that is not too far off, then this would qualify for another ECA condition plugin that provides a drop-down list of available validation plugins and a value field. It would then process that combination like described for the handler before.

The entity handler validation plugin should be going into the eca_content sub-module. The other one with validation plugins would be better placed in eca_base, as it could be useful with arbitrary values, not necessarily related to content entities.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

For some reason, Drupal core doesn't provide a token for the user status. that's correct.

But you can use the Entity: get field value action to receive the field value and write that into a token. Then you can use that to compare it.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Ouch, missed that. Thanks for the heads-up.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Good point, I've updated the MR accordingly and will now test that in the SQRL module.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

jurgenhaas β†’ created an issue.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

jurgenhaas β†’ created an issue.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Closing due to missing feedback.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

jurgenhaas β†’ created an issue.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

According to the change record [#3382316] that property default_argument_skip_url has never worked anyway, so I'm just removing it.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This is a difficult one. Building ECA models does not always know about any context. Let's say you just create an action in the bpmn_io canvas, not being linked to any predecessor yet, i.e. not connected by any arrows. When building the config form for the property panel, the system doesn't know which tokens will be available, so it's impossible to provide a list of available tokens.

The same is true for later changes. The model building process is often such that plenty of changes are made, and each of those changes can have an impact on the context for each of the action contained elsewhere in the model.

Often, we only really know at runtime, i.e. when the model is executed, what tokens are available when processing an action. And that can even be different each time the same model gets executed.

With that in mind, while we can certainly see the wish for a more user-friendly UI in this situation, it's very hard and often times even impossible to do.

Instead of doing nothing, I can only see to improve the user experience by adding more detail and samples to the documentation.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Moving this to the ECA project, since the action plugins provide the configuration forms, so any changes to those forms need to be made there, not in bpmn_io.

The token field should absolutely be required. That's almost a bug that it's not.

Help text can also be improved, thanks for the suggestions. Not all of them are straightforward, though. The user field can be left empty, and saving the entity is not always required.

The description of the plugin already describes it as "Create a new content entity without saving it.".

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

When using the command drush uli there is no login happing at that point. This only creates a link which can be used as a one-time-login to then also reset the user's password. So, when using the created link in the browser, that's when the login happens and that does dispatch the user login event. we're using that in many places without any issue.

And Drupal can not distinguish how that link was created, either by drush, by the password reset form or otherwise. The link is always the same structure.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I'm not sure if I fully understand what you mean by an activity stream like classic social. An activity stream to me sounds like a number of content entities that are displayed in a list. So, content entities need to be created (either manually or automatically) and the list display is probably created by a view.

If that's, what you're asking about, I'm not sure which role ECA would play in that scenario other than automatically creating content entities following some events that occur on the site.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

That's ambitious, but makes total sense.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

However, what I did have was the core (deprecated) Actions UI module active and it seems that once I had uninstalled that and re-run a drush eca:update the same error message does NOT show so we can probably safely assume that the deprecated module was the root cause.

Agreed, that sounds about right.

So, I'm closing this one as fixed then.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

+1 for the dashboards module, we've started using this on a range of different sites types recently and it work extremely well.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Thank you so much @fjgarlin for working together on this and thanks @drumm for merging and deploying it. I've tested it on the ECA module page and it makes a huge difference. The description looks so much better now.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Agreed, if there's anything we can do about it, let's do it. I was just under the impression there's something wrong in core and I could find a solution or even workaround.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Oh, that's a great insight, thanks for sharing.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Thank you so much for your detailed description and your suggestions. This is great input altogether.

In detail:

The described step by step process sounds perfect.

Finally then, I then issue a drush eca:update and the result is...

Pretty much no feedback which is probably a good thing but I think at least 'done successfully' for each update might be useful.

That's correct, and I see that being not ideal. I've opened ✨ Add feedback to the eca:update drush command Active to address that.

What I do see is...

[error] The action plugin comment_unpublish_by_keyword_action can not be initialized. ECA is ignoring this action. The issue with this action: The "comment" entity type does not exist.

...and I'm going to assume there that that 'error' didn't stop proceedings but I I don't know that for sure.

Where and when do you see that output? I know where it comes from: Drupal has removed some action plugins in 10.3 and the message above means that at least one of your models seems to be using this action. You should find out which one and remove/replace that action with something else.

... my more complex ECA models and they appear to be working as normal but like I said, I cannot be certain of that ...

Here is what we've done to address that: for our business critical ECA models, we've write automated end-to-end tests for all scenarios in those models. We run them in our project related pipelines for every update, and that way we can be certain that nothing broke. I know, this is a lot of work to prepare, but it feels great and pays back quickly.

"Update existing ECA models with drush eca:update" - 'drush' is missing.

Thanks, I've fixed that right away. The same issue existed also in the release notes @ https://www.drupal.org/project/eca/releases/2.0.0 β†’ , of course as a result of copy & paste.

Question RE shared hosting

Great suggestion, we should follow up with that at ✨ Make eca:update available through the UI Active .

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Sounds good, thanks for your feedback. Marking as fixed for now.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

I've applied your suggestion, this is genius.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Looks like your test code is missing the first instruction:

// Remove whitespace between tags.
$html = preg_replace('~>\s+<~', '><', $html);

Maybe that's the reason?

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

That's strange, I ran this locally and received the expected output:

<h2><a href="#contents-of-this-file" aria-hidden="true" class="anchor" id="user-content-contents-of-this-file"></a>CONTENTS OF THIS FILE</h2><ul><li>Introduction</li>.....

Not sure where this went wrong.

The local script looks like this (3rd line starts the extra code):

// Html entities appearing in the markup are encoded.
$html = html_entity_decode($html);
// Remove data-sourcepos attributes.
$html = preg_replace('/(<[^>]+) data-sourcepos=".*?"/si', '$1', $html);

// Remove whitespace between tags.
$html = preg_replace('~>\s+<~', '><', $html);

// This code block replaces remaining linebreaks with spaces, except within
// code or pre elements, where we need to keep the linebreaks.
$html = str_replace("\n", '###TEMP###', $html);
$document = new \DomDocument('1.0', 'UTF-8');
$document->loadHTML($html, LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD);
foreach (['code', 'pre'] as $tagName) {
  /** @var \DOMElement $element */
  foreach ($document->getElementsByTagName($tagName) as $element) {
    $element->textContent = str_replace('###TEMP###', "\n", $element->textContent);
  }
}
$html = $document->saveHTML($document);
$html = str_replace('###TEMP###', ' ', $html);

Don't know how to reproduce this. Have I missed something og the code in the MR? I don't see any differences, do you?

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Ah, those are a problem? Better to be on the safe side, agreed. And thanks for the link. I've tested the usage of those 2 options and they're doing what it's saying.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Oops, worked on that but not sure I caught it yet. Left a comment in the MR as well.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

MR!265 seems to be doing the job, please have a look.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

This sounds like Drupal Symfony Mail provides their own form validation? If that's the case, I'd say this is not necessary. If the field type is email, i.e. '#type' => 'email', then validation is done by core validation and ECA is prepared for that, even if a token is being used.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Can you answer my questions from comment #2 please? I have no idea where this action plugin in coming from.

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

There you go, if I change the cardinality of the field_tags from 1 to unlimited, then the error goes away, as the $values = NestedArray::getValue($form_state->getValues(), $path, $key_exists); then returns an array as expected. Makes me think even more that this is an issue in core.

Production build 0.69.0 2024