mxh → created an issue.
Added another small note regards the array lookup. Other than that the code changes look good. I have no time to actually test it though.
Some thoughts:
- When implementing this on one of the next major versions that require at least Drupal 11 then I guess we don't need to consider LegacyHook or any other sort of BC layer. Just going all-in for OOP hooks.
- I didn't look any more in detail, but one thing I noted was that hooks are being registered as listeners into the
event_dispatcher
service - something ECA does as well. Is it maybe possible that we merge some of our hook-related events entirely to OOP hooks, so we could reduce some class definition overhead?
Thank you for your kind words @mandclu, I really appreciate it 🙂
If, however, you would prefer to not even see the notifications for this module, then I will remove you a maintainer if that is your preference.
All good, we can leave it as is. Just wanted to let you know about my current availability for maintenance.
Likely this is yet another issue around ✨ [META] Expose Title and other base fields in Manage Display Active
If so, please consider closing this one as duplicate.
MR237 contains test and fix, whereas MR238 only contains the test. Running the test alone should give some fatal errors or exceptions, whereas the fix should pass the test.
I had to add another fix part into group_user_delete hook implementation, as the problem is also happening when deleting a user that is the owner of some groups.
Sorry I only had the time to work on top of 3.2.x branch, needs probably a reroll for 3.3.x (and other more recent versions).
About to write a test.
Is this automatically solved when 🐛 Node update hook is triggered upon group content create RTBC is fixed?
Thanks for the link to the other issue. Using A token for the opening bracket and another one for the closing one may indeed a valid trick to work with.
However, the "#parents" key is a common Form API concept. It's not something special. An implementation based on a certain API is supposed to incorporate its common concepts.
Yeah, this might work too. Or move the logic from the event into the Processor. DynamicSubscriber should be as "dump" as possible and only serve one purpose. Feels a bit odd to disable the processor there.
Is #21 a reply to #20 or a general information about modeler API? Maybe posted into the wrong issue (looks like an answer regards modeler API)?
mxh → created an issue.
As long as the #parents
key isn't incorporated into the states definitions, this may only work for now with the bpmn_io
modeler. But it wouldn't work for any other tool that embeds plugin forms "inline" such as eca_cm
does. The #parents
key is a common form API concept and thus may be worth to be incorporated. For details, see #12.
Ok. We may be more flexible when not supporting "BC" to the BPMN's property panel. Currently I see no reason why I'd use BPMN's property panel if I could have all the good stuff of Drupal's form API. Maybe not worth trying to cover both. But that's just an assumption around this, maybe there are good reasons to keep that compatibility I just have missed.
Do we get full form API support from this mechanism, including native form validation, submission and Ajax such as autocomplete?
Took a brief look into this and left two minor side notes.
That's great news and exciting!
Do we need to take care about CSRF token protection when opening up for a real form coming from the backend? So far my understanding is that it's needed when the request might involve some write operations. Maybe the CSRF token is already included in the rendered form, but might be worth a double-check to look into.
mxh → created an issue.
Setting to RTBC since tests are green and code looks like a working solution.
Looks great, thanks!
Not sure what you mean in #9 - I think calling a helper method once would be a bit more readable code rather than back-and-forth flipping of the boolean variable from the outside. Nevermind, the current state of the MR now looks also to be a working solution.
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.
public function setWithRekeyOption($property_name, $value, $notify = TRUE, $disableRekey = FALSE) {
$initial = $this->disableRekey;
$this->disableRekey = $disableRekey;
$this->set(...);
$this->disableRekey = $initial;
}
Thanks. While the code change looks exactly as described, there is a failing test which may require some attention: https://git.drupalcode.org/project/eca/-/jobs/5327402
Setting to NW due to the failed test.
Thanks for addressing this. Took a brief look and left one note.
Hope I extracted the right code part from \Drupal\htmx\Render\HtmxBlockView::build
which currently looks like this:
/**
* {@inheritdoc}
*/
public function build(HtmxBlock $block): array {
$build = [];
$currentRequest = $this->requestStack->getCurrentRequest();
// Verify that this is an HTMX request.
if (
$currentRequest instanceof Request
&& $currentRequest->headers->has('HX-Current-URL')) {
$simulatedContext = Request::create(
uri: $currentRequest->headers->get('HX-Current-URL'),
cookies: $currentRequest->cookies->all(),
server: $currentRequest->server->all(),
);
if ($currentRequest->hasSession()) {
$currentRequest->getSession()->save();
$simulatedContext->setSession($currentRequest->getSession());
}
// Add the simulated request to the top of the stack.
$parameters = $this->routeMatchingEnhancer->matchRequest($simulatedContext);
$simulatedContext->attributes->add($parameters);
unset($parameters['_route'], $parameters['_controller']);
$simulatedContext->attributes->set('_route_params', $parameters);
$this->requestStack->push($simulatedContext);
if ($block->access('view')) {
$build = $this->renderBlock($block);
}
// Remove the simulated context.
$this->requestStack->pop();
}
return $build;
}
Haven't verified, but maybe there is no cache context handling defined for the special HX-Current-URL
header?
Maybe add a cache context as provided by Drupal\Core\Cache\Context\HeadersCacheContext
like this:
// ...
$build['#cache']['contexts'][] = 'headers:HX-Current-URL';
return $build
Also be aware that Page Cache doesn't know of any cache contexts or headers 🐛 Page Cache must respect Vary, otherwise it might serve mismatched responses. Active .
I have checked whether VDE has something to do with this problem - it doesn't. The problem is within the bootstrap_barrio theme. Details can be found here: 🐛 Progress bar not styled correctly Active
I can see one possible problem here:
https://git.drupalcode.org/project/bootstrap_barrio/-/blob/5.x/css/compo...
This CSS file is empty, but the corresponding pcss file is not empty. So maybe the build process was wrong for compiling this file?
This problem exists for any Batch UI, it's not because of views_data_export. So wherever I start a batch, the progress bar looks unstyled.
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.
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.
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.
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.
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.