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
$contextvariable. - 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.
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.
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.
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.
There is a listener Drupal\Core\EventSubscriber\MainContentViewSubscriber::onViewRenderArray on event Symfony\Component\HttpKernel\KernelEvents::VIEW. Within that listener method, following happens:
// For main page content controller methods, this can (but is not forced to be) returning a renderable array.
$result = $event->getControllerResult();
// ...
$renderer = $this->classResolver->getInstanceFromDefinition($this->mainContentRenderers[$wrapper]);
// $wrapper == 'html' -> will be calling \Drupal\Core\Render\MainContent\HtmlRenderer::renderResponse
$response = $renderer->renderResponse($result, $request, $this->routeMatch);
// ...
There is an already existing "kernel:view" event plugin in eca_misc. It could be extended to reckognize $result = $event->getControllerResult(); so in case of a renderable array, it could be instantiated as a Drupal\eca\Event\RenderEventInterface.
There is a "hidden" layer of how a page title can be set.
There is an anoymous function in Drupal\Core\Render\MainContent\HtmlRenderer::prepare:
<?php
$get_title = function (array $main_content) use ($request, $route_match) {
return $main_content['#title'] ?? $this->titleResolver->getTitle($request, $route_match->getRouteObject());
};
?>
It fetches the #title key from the main page content render array and uses it when set.
For example, within Drupal\node\NodeForm::form the page title of the node edit page is being set as follows:
<?php
if ($this->operation == 'edit') {
$form['#title'] = $this->t('<em>Edit @type</em> @title', [
'@type' => node_get_type_label($node),
'@title' => $node->label(),
]);
}
?>We could make use of this mechanism, by implementing "Set Title" as a render action so that it is able to get the current render array. Then we are probably able to set the title for example on a "Build form" event.
I took a second look for verification and it seems my problem is somewhere else and is probably unrelated to this. I'm not having more than one group content plugin involved for one particular entity, which seems to be in your case (plugin IDs "group_contract__seller:default", "group_contract__buyer:default")?
As nodes usually have one plugin ID in use on a group type, for example "group_node:article" this issue may only show up when dealing with multiple plugins. So even when there are multiple roles defined in one group type, still only one plugin ID is in use.
If the query access logic acts like "if one handler allows access, then access is granted" then yes the loop in group_entity_access needs to be adjusted so that is behaves the same. I took a look into your MR and it might work, just a bit hard to read. Maybe a test that verifies the result between query and runtime access when having multiple plugins involved can make this clear.
Experiencing the same issue, and wondering at the same time why this seems not to happen frequently for other sites. Are most sites only using one relationship on an entity?
But the fix in MR!29 should still be added to the flag module, as it fixes the action plugin in this module
Maybe I'm wrong as this is long time ago. But since #3322747: Action plugins cannot be instantiated in a generic way → is in for a while now, which is supposed to have the problem fixed already, I don't think MR29 is necessary anymore.
Note that if you're just using a flag as a UI to trigger the ECA action and you don't care about the flagging entity that doing that creates, you'd be better off using https://www.drupal.org/project/action_link → instead.
Interesting module. Just a side note, ECA already has anything included to achieve the same thing, e.g. using "Render custom form" with a submit button and adding an Ajax handler to it. Or create a regular link with "Render: link" and build an ECA endpoint for it.
Needs review over two years, maybe this is a "won't fix"?
mxh → created an issue. See original summary → .
This issue is rather old but interestingly I also ran into this issue the last couple of days. In my case I'm just returning a MessageCommand as Ajax response. When no message is already existing on the page, this bug occurs.
I don't see anything else I can do to make PHPStan happy. Writing a test that tries to serialize and de-serialize an ECA config entity might help but I don't have time to do it. Using the currently provided fix as a patch and will see how it goes. Seems to be sufficient being addressed later on.
mxh → created an issue.
The Parameters → module makes parameters available as tokens and are completely configuration-based. Major difference that a parameter token always starts with [parameter:*] whereas this module allows to specify a token type on root level.
Usually I close or update issues accordingly when I find a solution or it had been fixed in the meantime. This issue is too long ago to remember why I encountered it, so I probably just found yet another workaround and moved on. And since no one else has encountered the same issue so far, this may be closed as outdated.
Running the reflection scan on possibly thousands of plugin definitions where only a fraction of them make use of it looks like a disproportionate resource overhead. Why not handling this similar to Annotations -> Attributes transition? For sure once autowiring is possible, more and more plugin implementations will adapt, but it's a matter of months if not years to come. I'd introduce an Autowire interface for plugins similar as ContainerFactoryPluginInterface to quickly identify which plugin is making use of it. For comparison, services also need an additional autowire: true declaration in order to make use of it. And there may be way more plugin definitions than service definitions around.