After 280+ comments in a solution-searching thread, we finally found both an approach and sponsor to allow Group to support the grouping of config entities. This thread will serve to group all of the individual tasks so that we can keep a nice overview of progress.
Here is the summary from that other issue:
- We create a content entity type called ConfigWrapper (group_config_wrapper)
- This entity type has almost zero UI, routes, operations, etc.; it only exists in code
- If you want to be able to group certain config, you write a plugin that targets said config entity type
- Then, for each plugin targeting a config entity type, we automatically create a bundle for ConfigWrapper
- Behind the scenes, we secretly wrap the config entity in order to add it to a group, but as far as handlers are concerned, they're dealing with the actual config entity
- We do not write a UI yet that allows you to add config entities to a group, modules for Menu, Webform, etc. will have to write their own UI
- Third-party code should be almost oblivious to the fact that wrappers exist
- Query access checks don't even run because config entity types don't use query access
- Regular access checks work the same as before because we'll adapt the group content storage to deal with wrappers, so loadByGroup() et al will still work as before
This should lead to minimal impact and actually be fully backwards compatible. The only major change will happen in the group content storage and people directly dealing with queries on any GroupContent table will need to make their code aware of wrappers.
Some important things of note:
- The wrapper would literally be that, a reference to a config entity
- It would not have any other fields
- If the target config entity is removed, so is the wrapper and GroupContent entity
- If the GroupContent entity is removed, the wrapper remains
In essence: A wrapper cannot exist without wrapped config, but a wrapper existing does not necessarily mean the config entity is grouped.
All contributors to the other issue will be credited here.
Comments
Comment #14
kristiaanvandeneyndeCrediting contributors to related issue.
Comment #26
kristiaanvandeneyndeMore credits, if you feel like I've missed you, ping me on Slack.
Comment #30
kristiaanvandeneyndeOof, so many people. Think I've got all of them now.
Comment #31
heddnThe approach suggested here sounds like the one we took in group content menu. +1 on a path forward.
Comment #32
tstoecklerNot opposed to the approach here in particular, but wanted to note that having configuration entities be part of groups is only one use-case for #2797793: Entities identified by strings as group content. The other is content entities with string IDs. Those are not used in core, but are and always have been explicitly supported in core and are often used for business applications where entities are imported from external systems and it is useful to have Drupal use the same ID as the other system(s). Not sure if #2797793: Entities identified by strings as group content should be re-opened for that, if we should have a separate issue for that or in general what your thoughts are on that.
Comment #33
kristiaanvandeneyndeAs per #2797793-250: Entities identified by strings as group content, I'm afraid content entities with string IDs are still second rate citizens in core. Until they are fully supported, I don't see how I can support them. See #2496699: Allow comments to be attached to entities using a string primary key
Comment #34
kristiaanvandeneyndeApproach changed a bit during development, plugins now directly can target config entity types.
Comment #35
tstoecklerRe #33: I would definitely argue otherwise, but that's neither here nor there. Certainly helpful to have a clear stance from a maintainer on this, that helps to plan how to move forward with that feature request. So thanks anyway, even if I would have appreciated a different response a tad more ;-)
Comment #36
kristiaanvandeneyndeThanks for understanding, I'm not opposed to supporting it but I follow Comment's lead as to make sure my approach doesn't get broken by core all of a sudden. So as long as Comment doesn't support it, I won't either.
Comment #37
kristiaanvandeneyndeI've managed to significantly simplify how we use wrappers, adjusted the IS to reflect that.
Comment #38
nigelcunningham commented@kristiaanvandeneynde, could I get you to clarify what you mean by "content entities with string IDs are still second rate citizens in core" - that might help in getting some work done on making them be better supported.
Also, I'm missing the context for "I follow Comment's lead" (not seeing someone with the alias Comment - could we get a link to the message for future reference?
Thanks!
Comment #39
kristiaanvandeneyndeRe #38, Group can add many entity types into a group, just like Comment in core can attach comments to many entity types. The safest bet years ago to deliver this similar "any entity type goes" logic was to copy it from Comment to make sure that our use case would not all of a sudden become impossible because of changes in core. After all, if Comment supports it, core supports it.
So far, Comment only supports entities with integer IDs, so that's as far as Group goes. If you want to see this fixed, then try and get the link I posted in #33 fixed.
Comment #40
kristiaanvandeneyndeThe functionality works now, just added a few clean-up tasks.
Comment #41
kristiaanvandeneyndeSee https://www.drupal.org/project/group/releases/2.0.0-beta1
HUGE thanks to ANNAI for the sponsorhip