🇩🇪Germany @mxh

So, I tend to lean towards option 2, combined with a warning in the release notes that this may break existing models.

Agreed

Should we probably introduce a flag in \Drupal\eca\Plugin\DataType\DataTransferObject to optionally turn off rekeying which should be called right before calling $list->set($index, $item); in the action plugin?

Either this, or initialize a DTO with a rekey option, or do both. Maybe then let the user decide whether rekey should happen, by adding a corresponding option to the "List: add item" action? There is another place where someone can add an item to a list, for example with "Token: set value" using token name [mylist:+]. But I think that one is probably never used by anyone. Still there may be other places where an item is being added to a list (other actions, such as merging).

Sorry for the noise, just want to briefly tell you this reconsideration and turnaround is highly appreciated. Thanks!

Took a look into the current MR and leaving some notes here.

#[HookDocumentation] is a confusing attribute. Its primary purpose is outside of what is relevant for the runtime of basically all running sites, yet it will demand computation resources from all of them, because attributes are part of reflection-based class discovery. Those *.api.php files were not really demanding such amount of computation resources on every site, as opposed to this concept.

Looking at the currently suggested implementation of #[HookDocumentation], the constructor argument callable $documentation is confusing. It would indicate one may pass along whatever is callable (first assumption was maybe a function returning documentation?). But what is supposed to be finally passed along, is not even callable. There is no real function like "hook_form_FORM_ID_alter".

What about #[HookInvolved]?

Regardless, AccountEvents::SET_USER seems the better approach, as it will register the session user right when the current user session is being set by core.

Whichever component in the system comes first, would "win". We have no guaranteed way that this is always the very first thing happening when setting the current user.

And that's what documentation tells, what this token is supposed to provide. Or do you disagree with that statement?

I don't necessarily disagree with it. I'd also like to have the "real session user" all the time if we could get it in a guaranteed manner. But we simply don't have it as described above.

Any other component could decide else what is supposed to be the "session user". Example: The Masquerade module

I think ECA should do what's possible within its scope, and I think the described way in #16 is still the "most" safe one I can currently think of. To recap it's the following way:

When ECA is being executed on a root-level process (= execution is not a nested subprocess), it sets the current user's entity object as session_user token. When the last ECA process on a root level finished execution, the session_user token may get removed from the token variables environment to avoid further confusion.

But I'd be happy to discover better available options.

mxh → created an issue.

This issue is clearly a bug, I'm wondering why it got changed to a feature request.

Because that's what the documented purpose of the session_user token is: knowing the original session user. Does Access Policy API expose that?

This may be more reliable to set the static variable \Drupal\eca\EventSubscriber\EcaExecutionSwitchAccountSubscriber::$sessionUser to the real authenticated user, which is what this variable was designed to be there for.

The only thing ECA can ever know, is the currently set user before ECA itself is executing anything. Which includes switching the user inside of ECA process execution. There is no safe way to always guarantee that we have the "real session user" at that point.

For continuation I try to summarize this issue so far:

The current behavior of ECA is:
- When ECA is being executed the very first time, it sets the current user's entity object as session_user token. On every ECA execution at the same runtime later on, the same instance of that initially determined session_user is being used.
This behavior is wrong as described in #9.

To resolve this, my previous assumption to just use the AccountProxy object was wrong. Because the AccountProxy may then of course hold the "current user" set by ECA, which is not what we want to be stored there.

I think the correct behavior would be:
- When ECA is being executed on a root-level process (= execution is not a nested subprocess), it sets the current user's entity object as session_user token. When the last ECA process on a root level finished execution, the session_user token may get removed from the token variables environment to avoid further confusion.

mxh → created an issue.

What "else" would switch the user account on purpose?

Access Policy API exactly does this within Drupal\Core\Session\AccessPolicyProcessor::processAccessPolicies:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Maybe the following code part makes it more obvious: https://git.drupalcode.org/project/eca/-/blob/2.1.x/src/EventSubscriber/...

public function initializeUser(ConfigFactoryInterface $config_factory, AccountInterface $current_user): void {
    if (self::$initialized) {
      // Already initialized.
      return;
    }
    self::$initialized = TRUE;

// ...
}

So this logic is applied only once for the whole runtime. Given my possible case in #9 this may even be dangerous as the current user at the very first execution may be a privileged user and this would remain the same one on all subsequent processes on the same runtime.

I wouldn't have created an issue if I wouldn't have a problem. I took the time to analyze this, give an explanation and re-reading my explanations I think they makes it obvious to make sense what the problem is about.

Also, the very first ECA execution on runtime may happen within a place where a component may have switched to a different current user on purpose. Creating an overview for access reports is a good example for that where you may check access for a privileged user and another time for a non-privileged user.

it would probably make more sense to provide the "Available and used plugins" overview by the Modeler API.

The plugins being used are defined and resided within eca.eca.* configuration entities. Modelers may not necessarily have anything to do with the plugins. Why would the modeler API to be concerned about ECA plugins? Feels more like a report sub-module to me. In the future we also might want to provide more debugging techniques that make it easier to debug running ECA processes.

My explanations are correct, especially as analyzed in #3: It's a static property, being initialized once for the whole runtime. When switching the current user with account_switcher service, an AccountProxyInterface object automatically refers to the correct current user.

The "session user" is a representation of the current state of the "current user". And the "current user" can be switched using core's account_switcher service. That may happen, otherwise that service wouldn't exist. I'm using it for a access report page, where I check access for various user accounts and show the results on one page. But that report currently does not produce correct results when ECA is involved in any way.

The code part I linked in #50 is referring to this issue. The code change was applied in the scope of this issue. Therefore, this issue is about clarifying that code part.

I now take the time to explain what the code part does that was disabled within the scope of this issue. Here is a comment that describes what it does: https://git.drupalcode.org/project/eca/-/blob/2.1.x/modules/form/src/Hoo...

Only needed when coming from the inline_entity_form module, because the embedded entity does not automatically hold most recent form input.

That means, the embedded entity is not automatically updated on its values when further Ajax is happening on the form. That code part has made sure that the entity values were updated in order to properly work with most recent submitted input used on the entity.

Here is the comment that refers to this issue: https://git.drupalcode.org/project/eca/-/blob/2.1.x/modules/form/src/Hoo...

So as long as that code part has not been clarified and properly fixed, this issue is not fixed.

This is setting a static property once, using an entity object that won't get exchanged once the current user is being switched:

self::$sessionUser = $storage->load($current_user->id());

On the other hand, the purpose of an AccountProxyInterface object would automatically use the currently logged in user.

Please see #51 🐛 ECA Form breaks complex IEF widget Active

Just create your own module that integrates the provider of your choice. Similar project: https://www.drupal.org/project/ad_entity_smart →

Also note that the maintenance status of this module is not set to be actively maintained anymore for new features.
Maybe lookout to integrate with a modern alternative instead. Example: https://www.drupal.org/project/ad → .

mxh → created an issue.

Maybe I got this wrong, but I understood option 1 to be an additional "Method" option in the existing "Entity: set field value" action, not about defining a new action.

There are some PHPCS complaints that should be addressed.

Thanks for addressing the feedback.

The logic is still a bit different than before when we don't use the option. For example the $path parts are being split using the explode function and if we have not specified any path position, the following is being returned now: return '/' . implode('/', $parts);

Despite the logic not being the same anymore we may set this to NR for now as the end result may still be the same (but I'm not sure).

Yes, I think we can extend eca_form_field_set_value that we change the '#value' (similar logic as the eca_form_field_default_value action) of the target $element. Meaning when $element = &$this->getTargetElement() returns a non-empty array. Maybe not even necessary to check the source event, rather just set the key on the $element if it's available might be the most flexible option.

mxh → created an issue.

I also tried the set submitted value action, but couldn't find the right event for it to do what I was looking for.

You're right, it doesn't seem to work for the rebuilt form to show the newly set value. So we might need to look for a way to provide a more convenient way to achieve it.

Thanks, now when using the internal path, we may want to use a proper naming of the option for that. Instead of "Skip language prefix" it may be called "Use internal path" as option, with a description something like "When using the internal path, added prefixes such as language code will be ignored. For example, both "/en/node/1" and "/node/1" would return "1" when index is set to 1.". This should make it more clear and also be understood for other cases than language code prefix.

Tests and code checks are currently all red, but that doesn't seem to come from the MR itself, rather from the current state of the 3.0.x branch. Once the section above got addressed we may set this again to Needs review.

mxh → created an issue.

Thanks. You've removed the option "skip_language_prefix" in the last commit, but this option is needed - could it be re-added?

The new logic should also only apply when this new option is in use, otherwise the previously implemented logic should just be applied as it was before, so we don't break existing configurations.

Thanks for addressing this issue.

I briefly looked into your current MR and suggest considering a different solution path. Instead of brute-force checking whether we might have a language code as prefix, may only solve this particular use case but not anything else that may add a certain prefix to a given system path. Also, there even may be routes that could actually start with a language code as prefix but were not added by the translation system...

Here is a suggested way to get the "correct" path we want to finally work with:

$current_url = Url::fromRoute('<current>');

// We want to get the path without language prefix, therefore use internal.
$path = $current_url->getInternalPath();
$path_args = explode('/', $path);
// ...

Thank you, this works perfectly for my implementation.

The action is called "Form field: set default value" and is not supposed to overwrite an already submitted value. We're using this action a lot and changing it in the suggested way will for sure break our existing configurations.

When you want to manipulate from what was submitted, then "Form field: set submitted value" is more likely the proper action you're looking for. However it might need to be executed in the right form event before values are being set in the form elements by the Form API.

Thank you for addressing this issue. I came to the same findings as you mentioned in #4 that this was changed as a part of a major refactoring without considering the consequences of this new explicit return type.

So let's change the return type here, but not the implementation.

Yes, that's totally sufficient for being able to implement a "real lazy" plugin collection.

mxh → created an issue.

mxh → created an issue.

The most important information for you on the module description page is the following:

this module is not recommended anymore

Practical usage has shown this module is causing slow PHP processes and too much memory.

#3303624: Seeing ContextStack show up in the slow php-fpm logger →
#3283297: Maximum function nesting level reached →

Maintenance status may be changed to unsupported entirely in the near future.

Until now, the starting point as mentioned in #47 is always:

Drupal with standard installation profile

Just using the current stable versions of Drupal core and every most recent version of involved contrib module at this time.

Maybe there's a difference in the combination of the theme being used, or even other contrib modules involved since Drupal CMS involves a lot more. So I recommend using Drupal core with standard installation profile as a common basis for this issue.

With my patch, you get the correct case study loaded when you edit, and if you add a new entity it gives you a fresh blank form.

Since the previous state of the MR didn't work for me as described in the previous comments, and I took the time to make it work for the standard installation profile (as mentioned), taking care that it doesn't break existing test coverage, the more interesting question to answer now is, whether the current state of the MR also fixes the problem with the steps to reproduce as you described in #55.

mxh → created an issue.

I'm wondering what scenario leads to that being correct

When either using the simple widget or possibly when using any other widget implementation for IEF.

I thought I saw some of those lazy-loaded, so I thought $context['items'] seemed more reliable... is that where you're getting null instead? I'm thinking this might be due to a difference in the field or widget config...

I got this error with the steps described in #47. Letting the user create a new node was enabled, but that should've been enabled by default anyway. So I basically changed nothing in the default widget config. Are you using different settings there?

So far I haven't seem much Symfony UX Live Components in the wild. From my perception the adoption of that solution has not been so wide (as opposed to Livewire and Phoenix LiveView). But I could be totally wrong about this perception.

Don't these technologies rely more on Websocket connections in order to be fully leveraged? To me Ajax seems to provide some commands (MessageCommand, ReplaceCommand etc.) that more feel like out-of-band swaps rather than components that require live updates with some "magic" wiring.

Some time ago I integrated Webpack Encore into Drupal → . Maybe it still works with current D10. Here's the Readme of that module. Not sure whether it's helpful regarding the JS dependencies but maybe so I thought I just post a reference to the module here.

When the current state of the MR487 doesn't work on your site, please provide steps to reproduce so we can have a look at it together and see for a possible solution. Best starting point would be Drupal with a standard installation profile and list additional modules you've installed. I have provided steps in #47 to reproduce the problem I had with the previous state of the MR and pointed to the place where the error is happening in #49.

As for comment #50 I don't know what the goal of that is

I won't lookup the code now but I am certain it was used, otherwise the code wouldn't exist. So we need to clarify in what way we need to address it, as that particular one was originally changed within the scope of this issue. It seems we've been looking at a different problem with MR487 and should have addressed it in a separate issue. Probably one of the reasons we have some confusion here.

I have tried to write a test for this, but it seems I cannot reproduce it anymore for now. I also currently don't work on the site anymore where I encountered this problem. I've also tried to reproduce this on a fresh Drupal installation with standard profile, writing a hook_form_alter implementation in a module and switch to a theme that is using this theme as base. But still no success.

The only thing that I can think of is that maybe the composer autoloading settings are different. But unfortunately I have no more time to spend on trying to reproduce this.

So maybe can be closed as "cannot reproduce" for now?

I have committed into the MR my suggested logic which works on my test site now.

Even when that would seem to work now, still this issue needs to stay on NW, because of what I mentioned in #50. The code logic there is not enabled for complex widgets. The linked commit belongs to this issue: https://git.drupalcode.org/project/eca/-/commit/6e183c9ebefb8f8b7331b8e9...

So we need to clarify what we need to do with that code part.

Also the main logic for rebuilding the entity for complex IEF widget hasn't been addressed at all, but that was the original changed part in scope of this issue: https://git.drupalcode.org/project/eca/-/blob/2.1.x/modules/form/src/Hoo...

So from the code logic itself the complex widget is being excluded, I wonder why the delta fix would suddenly resolve that part. It doesn't make sense.

Are you sure you're applying the correct MR (487), and not the old one that was already merged?

Warning: Trying to access array offset on null in Drupal\eca_form\HookHandler->fieldWidgetSingleElementInlineEntityFormComplexFormAlter() (line 275 of modules/contrib/eca/modules/form/src/HookHandler.php).

https://git.drupalcode.org/project/eca/-/merge_requests/487/diffs#bd67f3...

The current merge requests prints out following error on my test site (using the steps to reproduce as described in the IS):

Warning: Trying to access array offset on null in Drupal\eca_form\HookHandler->fieldWidgetSingleElementInlineEntityFormComplexFormAlter() (line 275 of modules/contrib/eca/modules/form/src/HookHandler.php).

Also when applying the merge request, the error is still the same on my site. My steps to reproduce

• Drupal with standard installation profile
• Enabled modules eca_form, inline_entity_form, bpmn_io, eca_user, eca_base, eca_content
• Added entity reference field on basic page to Article content type with cardinality = 6
• Configured using complex IEF widget on that entity reference field in basic page's form display configuration, allowing users to create new content.
• Wen editing an existing basic page and now try to create new article within that basic page form -> the first one succeeds
• Now try to create another article in there -> you will see that field values from the first submitted one are still there - this is not happening when eca_form is uninstalled
• Anyways no error shown yet, but try to save the whole basic page form now -> you will see integrity constraint violation as fatal error.

Getting this same error either when running on 2.1.7 or when applying the merge request.

Basically this part needs to be changed from

$form['token_name'] = [
      '#type' => 'machine_name',
      '#machine_name' => [
        'exists' => [$this, 'alwaysFalse'],
      ],
      '#title' => $this->t('Token name'),
      '#description' => $this->t('Optionally define a token name of this render element. It will be made available under that token name for later usage.'),
      '#default_value' => $this->configuration['token_name'],
      '#required' => FALSE,
      '#weight' => -28,
      '#eca_token_reference' => TRUE,
    ];

into

$form['token_name'] = [
      '#type' => 'textfield',
      '#maxlength' => 1024,
      '#title' => $this->t('Token name'),
      '#description' => $this->t('Optionally define a token name of this render element. It will be made available under that token name for later usage.'),
      '#default_value' => $this->configuration['token_name'],
      '#required' => FALSE,
      '#weight' => -28,
      '#eca_token_reference' => TRUE,
    ];

mxh → created an issue.

We don't need to restrict by plugin IDs. See my comment #42 first two sections for details. tldr:

• We don't need to check for the plugin IDs. We basically just need to check for the existence of that special IEF key and otherwise fall back to the delta from the $context variable.
• When we still don't get any delta for some reason we should log it as an error in the same way it was done before.

So, users who're worried about the size of the XML data will be able to turn their storage off.

Our clients are using the modeler UI in order to better understand it. And one particular detail is the way *they* have arranged & placed the items in that modeler, not how the system would do this automatically. It is a visual helper which brings massive value for better understanding. So I don't think this is an acceptable compromise.

mxh → created an issue.

It's now better since it's not breaking the existing test anymore. I don't think we need to specifically check for the plugin being used. It's probably fine to just do this: "So the code should maybe check whether that special IEF key exists - if yes use it, if not then fallback to that delta key set by the context variable."

We could additionally preliminary return and/or log an error like it was done before when even the fallback delta key from the context variable doesn't exist.

Setting to NW because of some PHPCS complaints. We should also have a test covering this, but since it's a bug report the can see first whether the code change fixes the bug and doesn't break existing tests.

But what happens, if we create a batch in different events where e.g. Drupal is about to save an entity. This may interrupt the current HTTP request and the expected entity save will not complete. Note, I'm using this just as an example, there could probably be many other events where such an interruption might be equally problematic.

Same argument would go for core's existing "action_goto_action" that performs a redirect. But what actually happens there is that the page contents of the current route are still being fully processed, but then at the end of the request a redirect response will be set to be returned (by adding an event listener on the fly).

The same mechanism can be applied to perform the redirect to the batch UI route, i.e. everything is still being processed in the backend, and at the end of the request the redirect response will be returned. One negative side effect that is also happening on the redirect action is that whenever you render a message on the current page, it won't be shown to the user because it got processed before being redirected.

Thanks for the link. I have checked the linked callback and it turns out this is exclusively added as a submit handler to 3 submit buttons inside of IEF. Example:

$row['actions']['ief_entity_edit'] = [
            '#type' => 'submit',
            '#value' => $this->t('Edit'),
            '#name' => 'ief-' . $this->getIefId() . '-entity-edit-' . $key,
            '#limit_validation_errors' => [],
            '#ajax' => [
              'callback' => 'inline_entity_form_get_element',
              'wrapper' => $wrapper,
            ],
            '#submit' => ['inline_entity_form_open_row_form'],
            '#ief_row_delta' => $key,
            '#ief_row_form' => 'edit',
          ];

So it would be only available when one of the three submit buttons was clicked. I don't see any Ajax or JS behavior that automatically submits one of these buttons so I suspect that this key is not available when the form is not yet submitted. And in fact, the existing test does confirm this. It's only green when the key is manually added as highlighted in #35.

If there is no place that shows that this is triggered automatically, then the test definitively remains valid as it was before and we need to find a different way.

I recommend once more to revert the change in the test and instead try to adjust the suggested code change so that it also works for the simple IEF. I think it's worth trying what I've mentioned in #32: So the code should maybe check whether that special IEF key exists - if yes use it, if not then fallback to that delta key set by the context variable.

The "complex" widget uses Ajax to load the inner form, and the triggering event to determine which delta to load

I gave an explanation above why it doesn't make sense to have a triggering element when a form is not submitted yet. You could be right, that on IEF complex widgets there is an Ajax submit handler involved. If so, do you know the place in IEF where exactly this happens?

We could then simulate that behavior for a complex IEF, but as said, it would need to be a separate new test besides the test for the simple widget.

The "complex" widget uses Ajax to load the inner form, and the triggering event to determine which delta to load

The testing module is using the simple IEF widget and not the complex one. So the test was properly covering the use case for simple IEF and now breaks with the current suggested changes.

Here is the part that contains the simple IEF widget configuration: https://git.drupalcode.org/project/eca/-/blob/2.1.x/tests/modules/inline...

Keeping the scope of this issue, you could create a separate new test for complex IEF widget instead of changing the existing one.

Even when split, I don't understand why this is a valid change in the existing test:

// IEF delta is found using the triggeringElement --
// without this, no default entity is loaded.
$form_state->setTriggeringElement(['#ief_row_delta' => 0]);

It doesn't make sense because a triggering element does not logically exist when a form is not submitted. Can you please explain?

The raw data from the modeler will either be ignored or stored in thirdparty settings of the eca.eca config entity.

You want to store the BPMN XML data in third party settings of eca.eca.* configuration entities? This will mean a significant load overhead when loading an ECA process configuration just for execution. Until now by separating this, loading that data was only done when needed (i.e. when configuring ECA via UI).

See example raised in #40: In order to make the third-party component work, one needs to properly pass along credentials. That example is using global credentials, so the required logic to achieve it could in theory just be "hidden" and only be taken care of when actually interested.

I guess one could also think of RenderElement and FormElement plugins and theme functions being transformed into SDCs. In these existing implementations we see various preprocess methods being involved before finally a Twig template will be used to render the element itself.

It makes sense to do the "dirty work" before instead of trying to do it inside of the Twig template, which is currently the only place in a component and I guess even more site owners will end up installing twig_tweak trying to achieve something similar in a SDC Twig file.

Those "batteries-included" components looks like Drupal plugins (blocks, layout, formatters, text filters...). Are they?

Components could have solved a common problem of fragmented code that we regularly encounter when working with RenderElement plugins, theme functions and template_preprocess functions. They could have provided a common way for reusable components across these different sorts of plugins.

Without batteries being included, a component on its own may not work at all. Some frontend devs then may wonder, why do they need to take care putting the batteries in? At least it can be simplified by providing helper classes, comments in the definition file or any other sort of documentation. But it's still fragmented but more importantly there is no standard way of doing this, as we're not able to just put that layer into the component as well.

mxh → created an issue.

Ok, re-reading once more to make sure I've not missed something and I clearly see that we have different arguments why the test was failing. The main problem I see is the current suggested code. It must be changed, otherwise it will not be compatible with Paragraphs anymore. See my comment in #32 why.

Here is the current suggested code:

$delta = $form_state->getTriggeringElement()['#ief_row_delta'];
    if (is_null($delta)) {
      return;
    }

Before, it was using $context['delta'], which is what at least I know for sure this worked with Paragraphs. And it least worked for the first item of an IEF element.

Now it's just using that IEF key and that will for sure not be available with Paragraphs, and for sure it won't be available when there is no triggering element. $form_state->getTriggeringElement() may return NULL, especially when the form is not submitted.

Let's first try to make the suggested code change not break Paragraphs and still work when form is not submitted. Then let's see whether the test needs a possible adjustment. I'm quite sure the test was fine, because it rightly tested whether both fields were disabled by the ECA configuration.

Ok, the test is testing for both and yes the naming of the test could have been improved in that regard.

But there is no triggering element when a form is not submitted. The model is disabling the field when the form is being built. That's why the suggested change in the test doesn't make sense and the current suggested change in the code is incompatible with Paragraphs. The test is only green because the triggering element with that special IEF key was manually set by the test itself. That's not what's happening when building a form though.

Once again setting to NW as we still need to get the test green without changing it. The test itself was fine.

So the code should maybe check whether that special IEF key exists - if yes use it, if not then fallback to that delta key set by the context variable.

Regarding the mentioned test: reading from the code it's using a paragraph provided by the Paragraphs → module in an article node and it asserts whether the title field got disabled. You may have missed that the test class is installing the eca_test_inline_entity_form which includes an ECA configuration that disables the title field of the paragraph: https://git.drupalcode.org/project/eca/-/blob/2.1.x/tests/modules/inline...

... this reflects a pretty often used scenario, which is to disable fields in forms.

The test failure proved that the suggested changes in the code break currently existing functionality. The test is using Paragraphs as module, and not Inline Entity Form. The Paragraphs module does not use IEF at all, it has its own implementation of "inline forms". It doesn't make sense to change the test in the suggested way because then it would be "tricked" to have some IEF logic in there.

I recommend to remove the suggested change in the test itself and try to make it green again without changing the test itself.

Thanks for having another look at this. Tests are not passing yet, thus setting to NW for now.

Not sure if it breaks any ECA functionality.

Given from what we've tried in the past at this place and given that the $context variable once held a delta that worked on our sites (but not anymore?) I guess either way something will change or break at a different place.

I just wouldn't want to have a single magic custom event that handles almost everything.

ECA doesn't really have a contextual validation logic anyway, so I'm wondering why we'd suddenly want to do this for custom events.

The difference between a "Custom Event" and "Custom Event (entity-aware)" is still a confusing concept. In that particular example the entity-aware even makes barely sense anymore since we support forwarding tokens anyway. We'd also then need to implement one action for each for being able to trigger it. I've had received many problem reports as the configuration was using an entity-aware custom event but was using eca_base's "Trigger custom event" (->not entity-aware) action to trigger it, which doesn't work.

I tried to fix this several times with no success. Complexity of Form API plus complexity of IEF makes this impossible for me to find a way to properly resolve this. Also it was a terrible mistake trying to manually rebuild an entity from a simulated form submission in "Form: build entity". Since it looks like no one is able to fix this critical bug, we may want to consider dropping support for IEF (and Paragraphs) entirely from ECA 3.x, and only stick with core's Form API events?

@pdureau Thank you for providing this detailed answer, this makes it clear now. You've set it to postponed maybe for some good reason, for me it's fixed now since this has been clarified.

Sure. Weird that this doesn't get automatically detected. I now checked you in the credit list, let me know if that's not the right way or how to do it.

Makes sense, thanks for reporting and providing an immediate fix.

Thanks for taking your time to explain, it gives a good understanding what's being tried to achieved.

When being shared across projects I now need to separately ship the logic for getting the credentials. So I'm basically not talking about the ability to call an external preprocess hook function, instead being able to call PHP logic that is resided inside the component itself. It's possible in SFC → and it makes sense.

It would have been a perfect fit to have the logic resided within the component in order to reduce complexity of understanding the component definition. For frontend developers (I mean people who also create field formatters, block plugins etc.) they wouldn't be interested in "When I want to use this component, how do I now properly pass the credentials to it (everytime I need to use it)?".

In my opinion this is not a good decision of being that restrictive. However I understand the arguments put in #48 and I wish good luck achieving these set goals. For me components with that current state are not that much useful because of this limitation as I cannot hide these things to reduce complexity. For my particular example, I think I'll just stick with the comment approach for now (just describing how to get the credentials in the component definition file).

Just read again through this issue and documentation → . It seems the concept of SDCs include that it should not call any Drupal API whatsoever. But I'm a bit confused about the usefulness of an SDC when I cannot put everything in there that is needed to work properly. I understand SDC are meant for frontend concerns only, which generally are not supposed to include things like PHP classes that contain business logic. But whenever such things are needed, duplication and fragmentation of such components start again. I wish this concept could've been a bit more "permissive", letting developers themselves decide what is allowed to put in a component.

One point I noticed in the documentation is the following sentence:

Within SDC, all files necessary to render the component are grouped together in a single directory (hence the name). This includes Twig, YAML, and optional CSS, JavaScript, etc.

- That "etc." kinda makes me curious, whether that may also include a PHP helper class for example ;)

Is it possible to express this validation logic only with JSON schema and (native, no custom extension) Twig templating?

The first question is whether I can hide this detail in the component definition. I guess I'm not able to read out the credentials from where it's provided without a custom Twig extension. And as long I don't want to go that route, I'd then need to have a mandatory component prop to pass along the credentials. Which then means every consuming instance needs to first take care of properly receiving the right credentials.

So, this is the place were the validation logic is expected, isn't it?

Yes, in the moment credentials are being entered, validation is happening where possible.

So, where are you calling the component renderable?

In my case it's in at least two different places, a field formatter plugin plus as an ECA render action plugin. Without the credentials provided in a certain expected format, the whole component is basically unusable. Yes I could call the validation logic outside of the component, but ideally the code should not be duplicated. It would be great being able to just instruct "At this place render this component", as the field formatter and ECA plugin don't really represent business logic as well, because they just exist to finally render the component. Also the settings to provide are defined site-wide, not per plugin configuration.

I'm just worried that the batch API only works in a fairly limited context and not across the board.

Sorry I currently don't understand this, could you maybe provide an example?

There we can ensure the proper context and let it work with multiple http requests in a row that make sure that none of the individual requests runs into a timeout.

Coincidentally, I recently experimented a bit with the endpoint as I had a timeout issue on a large operation on a magnitude of records.
At the end of an endpoint request, I used the Redirect action to redirect back to the same endpoint, continuing the next chunk of records. Surprisingly that already worked. Just wondering when a browser may complain about too many redirects...
So I implemented another experimental action that renders the contents of an (ECA) endpoint lazily via Javascript. A bit similar to HTMX lazy loading feature. And that works for a "poor man's batch" quite well, it even can output arbitrary status information and it even supports "parallel" processing since I can put in multiple lazy loading items on a page at the same time. If interested I could provide the action, however it needs a small Javascript file to be included in order to work.

While the render example was the first reason I thought of making custom events "more reusable", this actually applies also to other sorts of events, such as access. So currently I cannot properly execute "Set access result" within a custom event.

But it could be argued, whether a custom event should be only supporting one sort of "return type" which are currently tokens. At the same time some custom events may just fulfill for one specific purpose, such as rendering something or setting a specific result.

By the way, we're currently using an "advanced" implementation of the custom event to support renaming of tokens being passed along, by setting "from_name->to_name" pattern in the field for tokens to forward. We often have cases where for example a custom event accepts a "node" token, whereas the calling parent process is working with multiple nodes having different names in context. The renaming is convenient in such cases. If you're interested in that I can create another issue for it.

mxh → created an issue.

mxh → created an issue.

mxh → created an issue.

One use case I recently had where a preprocess function would have helped me: I created an SDC that renders a third party provider JS widget. In order to connect to the third party I had to provide some configurable global credential settings. Either I wanted to hide that from the component itself to reduce its complexit so it would have loaded them by itself from a global config object, or when I provide it as SDC prop for credentials I wish I could have a way to validate that these were valid credentials.

mxh → created an issue.

mxh → created an issue.

In the example of LoadEntity, the refactor may look something like this:

class LoadEntity extends ConfigurableActionBase ... {

  /**
   * {@inheritdoc}
   */
  public function executable(...$args): ExecutableResultInterface {
    if ($this->doLoadEntity($object)) {
      return ExecutableResult::executable();
    }
    return ExecutableResult::notExecutable('No entity available.');
  }

  /**
   * {@inheritdoc}
   */
  public function access($object, ?AccountInterface $account = NULL, $return_as_object = FALSE) {
    $reason = NULL;
    $entity = $this->doLoadEntity($object);
    $access_result = $entity->access('view', $account, TRUE);
    if (!$access_result->isAllowed()) {
      $reason = 'No permission to view the entity.';
    }
    if ($reason !== NULL && $access_result instanceof AccessResultReasonInterface) {
      $access_result->setReason($reason);
    }
    return $return_as_object ? $access_result : $access_result->isAllowed();
  }

}

// Within ECA Processor:

if ($action->executable(...$args)->isExecutable() && $action->access(...$args)->isAllowed()) {
  $action->execute(...$args);
}

.. which would streamline the access check to its purpose, since it doesn't need to check anymore whether the entity exists as this was already checked by executable().

Yeah, the global user switch is a rabbit hole where it's hard to get out from later on.

And to accomplish that, the entity needs to be loaded, which inherently also checks that the entity even exists. What I'm saying here is that in this case there won't be any access control possible if this will be part of the executable check.

In this example, yes it obviously needs to load it. Maybe not the best example picked up. Still there may be an executable method that only checks whether it exists. If I don't care about the actual access check then, I could skip it. Loading the entity multiple times here could be optimized in case we see a potential performance issue.

But that setting should probably not be global or on a per action plugin basis, it should be configurable per ECA model.

Sounds good.

Just a question for the release notes: this may be breaking some styling in forms if somebody has some CSS which doesn't expect the container at this point, is that correct?

Yes that's correct, it may break existing custom styles.

Thanks so much for addressing this so quickly. Manually tested and it works as expected.

mxh → created an issue.

Needed for 2.0.x and 1.1.x

mxh → created an issue.