- Issue created by @mxh
- 🇩🇪Germany jurgenhaas Gottmadingen
That's very reasonable and raises a couple of follow-up questions:
- What is the default state for new plugins: enabled or disabled? In other words, do we maintain an allow list or a block list?
- What if a plugin is disabled and an ECA config entity gets imported that uses one of those disabled plugins?
- What if an existing ECA config entity uses a plugin that gets later disabled? This is similar to the question above, but comes from a different use case.
With all that in mind, I wonder if we should maintain such a list in the ECA settings or rather use the permission system to control, which role is allowed to see and execute which plugin. This could rather open up for some more interesting scenarios then.
- 🇩🇪Germany mxh Offenburg
do we maintain an allow list or a block list?
We should go for a whitelist, because this guarantees a "stable" subset of available plugins. There may be dynamic derivers that will add more plugins, for example when a new content type got created. The admin of ECA would otherwise need to always check for the blacklist, but won't need to when having a whitelist.
What if a plugin is disabled and an ECA config entity gets imported that uses one of those disabled plugins?
What if an existing ECA config entity uses a plugin that gets later disabled?
It should still work, i.e. it allows to use it. The aim of this issue is not to disable / remove / block plugins, instead I think it should mainly aim to hide unnecessary "clutter" from the UI.
I would not like to involve permission management here, as this may be a different issue and lead to other problems, such as not being able to use a plugin for an exceptional case. It would also raise the complexity of the execution engine. I think it's sufficient to look at this issue just from a UI perspective.
- 🇩🇪Germany jurgenhaas Gottmadingen
We should go for a whitelist, because this guarantees a "stable" subset of available plugins. There may be dynamic derivers that will add more plugins, for example when a new content type got created. The admin of ECA would otherwise need to always check for the blacklist, but won't need to when having a whitelist.
If I'm not totally wrong, it's just the other way round. When working with an allow list (formerly known as whitelist), new items only work when they get explicitly allowed. What you describe is more like a block list, i.e. everything is enabled, unless it gets added to the block list.
It should still work, i.e. it allows to use it. The aim of this issue is not to disable / remove / block plugins, instead I think it should mainly aim to hide unnecessary "clutter" from the UI.
So, you only want to hide them from the UI, everything remains the same?
I would not like to involve permission management here, as this may be a different issue and lead to other problems, such as not being able to use a plugin for an exceptional case. It would also raise the complexity of the execution engine. I think it's sufficient to look at this issue just from a UI perspective.
Well, we can see the requirement for fine-grained control on plugins already. In fact we had that in other issues before already. So, rather than building multiple controls on which plugins are visible/usable/etc, I'd prefer a generic approach which solves all of those requirements with one tool that already exists.
There could be permissions like "visible" and "executable" for each plugin, which would then allow to grant them to users based on their roles. That then also addresses your exceptional case scenario where e.g. the admin role will have permission to use all plugins, even if invisible to other roles.
- 🇩🇪Germany mxh Offenburg
That's what I understand to be a stable list. When new plugins are being added, the admin can then enable them. I don't see a problem with that approach.
So, you only want to hide them from the UI, everything remains the same?
Yes.
Well, we can see the requirement for fine-grained control on plugins already. In fact we had that in other issues before already.
Related issues should be linked then, so we can collect all of already existing knowledge. Otherwise I cannot see that.
There could be permissions like "visible" and "executable" for each plugin, which would then allow to grant them to users based on their roles.
Role-based control may not be sufficient either. But as mentioned above, it's easier to specify requirements when having all related issues linked at one place (here).
- 🇩🇪Germany jurgenhaas Gottmadingen
That's what I understand to be a stable list. When new plugins are being added, the admin can then enable them. I don't see a problem with that approach.
I don't like this approach TBH. The simplicity to have access to all features by default is what makes ECA so attractive. If new users had to go through a list of 400 actions first, deciding which ones they want to use and explicitly enabling them, is not what we should introduce.
By default, all plugins should be available and if somebody wants to disable some, they should be able to do that. But the vast majority of users will not change anything and go with everything that's available.
Related issues should be linked then, so we can collect all of already existing knowledge. Otherwise I cannot see that.
You brought that up last time, when we implemented the actions to grant access to all cache and key value stores, not only those for ECA. And there we discussed that a permission system would serve the need for those requirements where a site owner doesn't want to allow that.
Role-based control may not be sufficient either.
I'd say that all permission logic in Drupal is built around roles. It's a known concept and building on top of existing stuff has been pretty successful so far.
- 🇩🇪Germany mxh Offenburg
You brought that up last time, when we implemented the actions to grant access to all cache and key value stores, not only those for ECA.
You mean this issue ✨ Extend cache actions beyond ECA Fixed ? I can't find a comment there where I brought up something about permissions.
- 🇩🇪Germany jurgenhaas Gottmadingen
You brought up the concern and I had responded with the permission approach in comment 10: https://www.drupal.org/project/eca/issues/3333644#comment-14893300 ✨ Extend cache actions beyond ECA Fixed
- 🇩🇪Germany mxh Offenburg
I'm feeling misunderstood, and I feel forced to clarify this: In that issue, I raised up a general concern about what ECA should provide, where I recommended to not support "just everything": https://www.drupal.org/project/eca/issues/3333644#comment-14892676 ✨ Extend cache actions beyond ECA Fixed
So what I "brought up" there is something different of what this issue here is about. What you "brought up" there https://www.drupal.org/project/eca/issues/3333644#comment-14893300 ✨ Extend cache actions beyond ECA Fixed does have something to do with this issue, but it's not me who "brought this up".
- 🇩🇪Germany jurgenhaas Gottmadingen
What's relevant for the topic of this issue is that we referred to permissions not only here but also in ✨ Extend cache actions beyond ECA Fixed and the discussion here should keep focused on that. If anyone wants to complain about the way I'm answering questions here, I'm open for private discussions, but I feel offended by the way this discussion goes.
As for hiding events, conditions and actions the permission system seems the right tool to be used. Happy to learn if and why that shouldn't be true.
- 🇩🇪Germany jurgenhaas Gottmadingen
A new aspect to this might be a "Feature Flag API" which is being discussed for Drupal core at ✨ Add an API for feature flags Active , which was triggered by some BC discussion in 🐛 [Views] Table sort class & indicator for active field is wrong. Needs work but could also be used for what we've discussed in here.