Problem/Motivation
Example scenario:
A block plugin that allows to select a config entity as part of the block instance settings.
That config entity is then a dependency of that block because the block does not function without it. But there is no way to expose that.
This is different from block derivatives as there is no exposed plugin definition for each config entity.
Core has such a use case too, but in that case, it's a content entity, which makes it a different (but also unsolved problem): aggregator feed blocks.
Proposed resolution
Add a method to ConfigurablePluginInterface to calculate dependencies.
Remaining tasks
Determine what other configurable plugins need to add code (could be a follow-up)
User interface changes
API changes
ConfigurablePluginInterface now has a new method, calculateDependencies().
Comment | File | Size | Author |
---|---|---|---|
#18 | interdiff-14-15.txt | 699 bytes | xjm |
#18 | config-2248151-15.patch | 15.71 KB | xjm |
#16 | 12-14-interdiff.txt | 1.23 KB | xjm |
#16 | config-02248151-14.patch | 15.68 KB | xjm |
#12 | 2248151.12.patch | 14.93 KB | alexpott |
Comments
Comment #1
xjmSo this is at least a beta target, maybe (probably) a beta blocker.
I had trouble understanding how this was different from e.g. menu blocks but this is the usecase:
I have a block plugin that doesn't use derivatives; instead, it adds to the block instance configuration. When I place an instance of this block, there's a form element where I can select the vocabularies that this block will display something-or-other for; could be one, or none, or several. So each block instance may have a dependency on one or more vocabularies. Since the
config_dependencies
key is part of the plugin definition as provided by derivatives -- not its configuration as stored in the block instance entity -- it doesn't help us.Comment #2
xjmComment #3
tim.plunkettWorking on this.
Comment #4
tim.plunkettThis is bigger than blocks. I can reproduce in HEAD with actions and user roles:
For each user role, two actions are configured for adding and removing that role from a user.
The action export has no reference to these.
Comment #5
tim.plunkettHere is an alternate solution, this moves all of the plugin related dependencies into one place. However, this needs an interface of some sort, and only works if you extend PluginBase.
Comment #6
tim.plunkettI think #4 is much more clear in terms of who is responsible for what. Regular plugins do not care about dependencies, just configurable ones.
But #5 is cleaner code IMO, and makes the entity less aware of what plugins need.
I'm happy with either, and I defer to @Berdir/@alexpott.
Comment #7
xjmYeah this is beta-blocking then.
Comment #11
tim.plunkettThis is 4 rerolled. Ditching 5, it's too much for all plugins, and overkill.
Comment #12
alexpottLooks great. Minor clean up to use
addDependencies()
instead ofaddDependency()
to avoid code repetition.Comment #13
tim.plunkettHah, didn't know that was a method! That alleviates some of my worries about the ugliness of that, thanks.
Comment #14
xjmWhoa this is RTBC already. Awesome!
That said this needs MUCH better documentation...
Comment #15
alexpottGood point @xjm should have noticed that.
Comment #16
xjm@alexpott and I came up with this expansion of the docs. We can add this to start, and then expand the scope of #2235363: Document config dependencies to configuration dependencies in general, because there's a lot more than that one key that needs documenting (as @tim.plunkett pointed out).
Comment #18
xjmSorry, one more fix.
Comment #19
alexpottrtbc if green
Comment #20
catchLooks great. Committed/pushed to 8.x, thanks!
Comment #22
tstoecklerThis commit added an empty db.sqlite file to the repo. I suppose that should be deleted.
Comment #24
catchMissed that reviewing. Fixed locally and pushed.
Comment #25
XanoThis patch alters \Drupal\Component\Plugin\ConfigurablePluginInterface with references to \Drupal\Core, and effectively this is a core and not a component feature. Should we create \Drupal\Core\Plugin\ConfigurablePluginInterface and move this code there?
Comment #26
alexpott#25 I guess so yep
Comment #27
catchThe references are to \Drupal\Core but the code doesn't introduce any dependency and could be used without core concepts.
I don't think we should move it, but not sure how to clean that up either.
Comment #28
tim.plunkettThanks!