Problem/Motivation
As found in #2309247: Views do not depend on modules providing their displays several views have block displays. The block displays were used to be provided by block module so a mechanism was added to bubble up that dependency to the view level. However, since then the block display was moved to views in #2315333: Move block plugin code out of block.module. The views themselves have not been updated.
Proposed resolution
Update the views to not depend on block module anymore. Now the views only depend on the modules they are contained with so no need to move them around or set further dependency on the modules.
Remaining tasks
Review. Commit.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#6 | 2309715-block-displays-are-now-in-views-6.patch | 8.02 KB | Gábor Hojtsy |
#6 | interdiff.txt | 1.71 KB | Gábor Hojtsy |
#4 | 2309715-block-displays-are-now-in-views.patch | 6.31 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor Hojtsy#2309247: Views do not depend on modules providing their displays landed. Moshe posted these notes there:
Looks like we don't have an agreement on this. As the current config stands, it may include fragments that are unknown to the system. One of the 3 remaining beta blockers is exactly about this "random fragment of thing in config" problem at #2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration.
At least for translations, its problematic that random fragments may be included and we have no way to figure out the config schemas for them. So none of the translatable strings in them will ever show up on localize.drupal.org for example.
I think its generally a problem anyway that modules would need to deal with configuration that may contain things that are structured and owned by an "unknown" thing.
Comment #2
tim.plunkett#2315333: Move block plugin code out of block.module should help us out.
Comment #3
Gábor HojtsyComment #4
Gábor Hojtsy#2315333: Move block plugin code out of block.module got committed. This is a much simpler issue now. The block dependency is not there anymore because the block display plugin is provided by views.
Comment #5
Gábor HojtsyComment #6
Gábor HojtsyShould also move the block display config schema. Not sure why that was not done in #2315333: Move block plugin code out of block.module. No changes to the schema, just moving.
Comment #7
tim.plunkettThe schema move was just an oversight, thanks for fixing that here.
Comment #8
webchickNice clean-up.
Committed and pushed to 8.x. Thanks!
Comment #10
jhodgdonOn #2323899-15: Provided default Node views need language filtering, gauravkhambhala reports that when exporting the Archive view (for instance) the block section still has "provider: block" in the config. So is this really fixed?
Comment #11
dawehnerJust to be clear, there is still code in views which relies on the block entity existance. So dropping that was kinda a bad idea.
Comment #12
jhodgdonThe person who reported #10 says it is no longer valid. Not sure about dawehner's point; setting back to "Fixed" for #10 though.
Comment #13
Gábor Hojtsy@jhodgdon: yeah I found it if I exported a new view, the dependencies were correct, but if I exported an existing view that was installed with the incorrect dependencies, those would be kept. I think (not all) parts of the view are recreated through their plugins again.
@dawehner: does that mean views module now requires block module?
Comment #14
effulgentsia CreditAttribution: effulgentsia commentedRe #11: which code? When I search for "Drupal\block" within core/modules/views, nothing turns up. But maybe the dependency is there and just not explicitly included like that?
Comment #15
dawehner@Gábor Hojtsy
Yes it does technically, see
\Drupal\views\Plugin\views\display\Block::remove()
(which will be resolved in https://www.drupal.org/node/2031859) and all the test code inside block module referring to the views block display.Comment #16
Gábor HojtsyWoah, does this blow up when block module is not enabled or just silently gets you an empty array? Answer from the code seems to be an empty array:
So this will return an empty array and therefore not delete blocks when you remove a block display IF block module is not enabled, but is that a dependency? Sounds like the right thing happens for the situation at hand. Not sure what test code in block module are you referring to?
Comment #17
dawehnerAll the integration tests between \Drupal\Core\Block and \Drupal\views still exists inside the block module.
This fatals:
Note, the remove() call could be triggered during a config import, when a view is removed.
I have never said it is an actual dependency, but currently it is a technical one, if we are honest. In other words please help on #2031859: Invoke an event[s] when a plugin ID disappears or we will have to make views module be dependent on block module.