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.
This is in my generated composer.lock file:
{
"name": "drupal/tour_ui",
"version": "2.0.0",
"source": {
"type": "git",
"url": "https://git.drupalcode.org/project/tour_ui.git",
"reference": "2.0.0"
},
"dist": {
// ...
},
"require": {
"drupal/core": "^9.1 || ^10",
"drupal/tour": "*"
},
"type": "drupal-module",
"extra": {
"drupal": {
"version": "2.0.0",
"datestamp": "1689967253",
"security-coverage": {
"status": "covered",
"message": "Covered by Drupal's security advisory policy"
}
}
},
"notification-url": "https://packages.drupal.org/8/downloads",
"license": [
"GPL-2.0-or-later"
],
"authors": [
// ..
],
"description": "Provides a UI to manage guided tours.",
"homepage": "https://www.drupal.org/project/tour_ui",
"support": {
"source": "https://git.drupalcode.org/project/tour_ui"
}
},
So tour_ui module depends on it according to composer.lock.
But the tour_ui module does not even have a composer.json file - neither in the Gitlab repository nor locally when downloaded.
So that definition must be (wrongly) generated from something else, maybe by the packagist repository.
Maybe this problem does not occur if tour_ui would have a composer.json defined on its own.
Moving this to tour_ui as it looks like the problem needs to be addressed from there.
Insert statements are being wrapped by a transaction as implemented in Drupal\Core\Database\Query\Insert::execute.
Thanks for the info, but it doesn't answer why Composer is installing this module.
Thanks for updating the merge request.
Setting to NW because the tests are failing: https://git.drupalcode.org/issue/rules-2816033/-/jobs/3962405#L1017
The same error is happening often times in the tests. The test failure is actually happening since the merge request was created, so it's not because of the rebase that just happened.
It turns out the merge request that was initially created in #160 does not 100% reflect the patch of #156. It is different, seems the contents were manually copied over and some parts were forgotten or copy-pasted wrong.
What I can see for example is that this line https://git.drupalcode.org/project/rules/-/merge_requests/21/diffs?commi...
still includes the event_subscriber tag definition whereas within the patch of #156 this definition was removed. And this is most probably the reason why the tests are currently failing in the merge request, as the main point of the patch is to let the listeners being subscribed by LazyGenericEventSubscriber.
We first need to make sure it 100% reflects those changes from #156, then rebase (and then make further changes if needed).
Appreciating the attempt to update the MR. However, as long a the MR is not mergeable and does not properly show the relevant code parts that are requested to get merged, this cannot be reviewed. Thus setting to NW.
When providing and updated version of the patch, please also provide an interdiff file from the last one that got mostly perceived as working, which was the one from #156. In that comment you can also find an example of such an interdiff file. It's an important piece of the patching workflow in order to efficiently notice the differences for anybody. I know that the patch is most probably only provided for patching the codebase and not as part of the review process here, but still it should be included.
Re-rolled against the latest release (1.0-beta12)
Merge request and patch need a reroll for the latest release.
When using basic_auth module, user authentication is happening on Drupal-level using HTTP basic authentication. So why would you want to login when you're already authenticated? It doesn't make sense.
Looking at the IS and since you're the only one who reported such a problem, the cause of the problem is/was elsewhere and thus it looks this issue got outdated.
This sound like a special requirement that needs the combination of some modules, such as ECA → and maybe ECA VBO → . But this module is not covering anything of your requirement and it's not supposed to do so. I guess this is something that you could have find out by yourself in the meantime and you could have reported your findings back here.
We are building Model where the PreSave Event of a GroupRelationship is relevant
This sounds like an ECA-specific requirement, which is not within the scope of this module.
mxh → created an issue.
Experiencing the same that the database table still contains group relationship items while the according group does not exist anymore.
We're having lots of concurrent requests going on, and one thing I could imagine is that the deletion process may take some time, especially when having many group relationship items. And within that time frame another request might have just added a new relationship to the group that is about to be deleted. But this is just a first guess on finding out what could have gone wrong.
mxh → created an issue.
There are still comments made from @avpaderno 10 months ago that have not yet been properly resolved. Example: https://git.drupalcode.org/project/parameters/-/merge_requests/5/diffs#6...
Patch attached that is the same as the current merge request.
Created a merge request containing a possible fix. It works for me locally but more testing is necessary.
Unfortunately this is certainly not fixed yet. See comment: https://www.drupal.org/project/views_entity_form_field/issues/3462770#co... 🐛 Field values get swapped among different entities in a view Active
Turns out this issue is not yet fixed with the linked issue from #4. This is actually a bug cuased by a different place in code.
Problem is located at Drupal\views_entity_form_field\Plugin\views\field\EntityFormField::buildEntities:
<?php
protected function buildEntities(array &$form, FormStateInterface $form_state, $validate = FALSE) {
$field_name = $this->definition['field_name'];
// Set this value back to it's relevant entity from each row.
foreach ($this->getView()->result as $row_index => $row) {
// Check to make sure that this entity has a relevant field.
$entity = $this->getEntity($row);
if ($entity && $entity->hasField($field_name) && $this->getBundleFieldDefinition($entity->bundle())->isDisplayConfigurable('form')) {
// Get current entity field values.
$items = $entity->get($field_name)->filterEmptyItems();
// Extract values.
$this->getPluginInstance($entity->bundle())->extractFormValues($items, $form[$this->options['id']][$row_index], $form_state);
// Validate entity and add violations to field widget.
if ($validate) {
$violations = $items->validate();
if ($violations->count() > 0) {
$this->getPluginInstance($entity->bundle())->flagErrors($items, $violations, $form[$this->options['id']][$row_index], $form_state);
}
}
}
}
}
?>Here the mapping for building the entity from submitted values is still done by the views's row index ($row_index variable). It can be related to a different entity when a new one got added in the meantime.
Additional note to reproduce this problem: It particularly appears when using an Ajax field, such as a file upload field:
1. Open a view using a file upload field as Views Entity Form Field. Upload a new file or remove an existing one. Don't click on save yet.
2. Open another tab, create new entity of same type and save it
3. Switch to the first tab (no refreshed page) and click on the save button of the Views form.
Great news, thank you!
Module should work fine with D11.
Another thought I had when looking at this: When any (imported/extracted/migrated?) icon is a plugin definition, then there may potentially be a lot of plugin definitions to be collected. From what I've seen on most sites, only a (very) small subset of icons of a library would be displayed on a website.
One example: many sites include Bootstrap icons or Google material icons as a whole library, but in the end they just use 10-20 of them in total, wheras the library itself comes with hundreds. There are other icon libraries around that may have thousands. If we're loading a plugin, then a default plugin manager will first load all existing plugin definitions (mostly from a cache backend) into memory and will keep them until the end of its service lifecycle. When only a small amount of the definitions are finally being used, this may be worth to be addressed for optimization.
"Extractors" remind me of plugin derivers. "Icon pack" reminds me of a base plugin ID. For me icons have characteristics of a (single-directory) component. Just a bit concerned that it might be over-complicated DX, especially when looking atalready existing core concepts. In the end it's just about icons.
I briefly tested with '#tags' enabled, however multiple groups are not yet working. It is currently only adding to one group.
I'm going to merge this one as it works on my testing and already gives some UX improvements. Support for multiple groups may be addressed by a separate issue.
Thank you @nielsonm for kicking this off and helping out.
mxh → changed the visibility of the branch 3481519-add-autocomplete-to to active.
False alarm, this is caused by some other code.
Investigating a bit more.
Resolved #6, setting back to NR.
Unfortunately this breaks the existing functionality for entering ID / UUID only.
Thanks for creating the issue and the merge request. Is it ready for "Needs review"?
Same slow query is happening within group_entity_access function so that part may be optimized as well using the cache.