Installing blocks

[rupertj]

The way we currently install blocks doesn't seem to be totally reliable. I'm proposing we add a mechanism to localgov_core to add them reliably.

The problem:
LGD modules that install blocks currently use config to do it - typically with two files per block: one for localgov_base and one for localgov_scarfolk. This works fine for those themes in our tests, and for new installs of a site that are yet to switch to their own theme. Themes that inherit from localgov_base also inherit its block placements when they don't have their own blocks, but themes that inherit from localgov_base and have their own blocks placed don't inherit blocks. (See https://www.drupal.org/docs/develop/theming-drupal/creating-sub-themes#s-inheriting-block-placement).

Hence if you add a module to an existing site that has a custom theme that inherits from localgov_base, the newly installed module's blocks will not be placed.

An initial solution:
As we've run into this issue a few times on localgov_publications, I've fixed it there by placing the blocks in hook_install(), into the site's active theme and the two LGD themes. This works, but if we want to fix this same issue in other modules, we'll end up duplicating that code.

What I'm proposing here:
We put a modified version of the code that's currently placing blocks in publications in a new sub module of localgov_core, which we'll call localgov_default_blocks.

If a module wishes to place default blocks, it declares a dependency on localgov_default_blocks and implements a hook to define the blocks. The contents of this hook will look like what's currently in the function localgov_publications_install_block_definitions() here: https://github.com/localgovdrupal/localgov_publications/blob/157554c463c8d70dbb190340cf4b79a6352c0180/localgov_publications.install#L99 . localgov_default_blocks will work much like the existing localgov_roles module does. It'll implement hook_modules_installed, call the hook to get the default block definitions and then set those blocks up. The code for this'll look a lot like the code in localgov_publications_install_blocks(): https://github.com/localgovdrupal/localgov_publications/blob/157554c463c8d70dbb190340cf4b79a6352c0180/localgov_publications.install#L72.

Questions / comments / feedback please :)

[rupertj]

Andy B commented in the slack thread:

I'd say its fine to try this for blocks in a hook install, just be careful about placing them in the correct region, maybe sitck to onces that follow localgov_base conventions (sidebar_first, sidebar_second) etc, though that still means those on alternative themes have to place them themselves it's no different to current situation.

Which is an excellent point. The code in publications doesn't currently check that the region we're putting blocks into exists in themes, but a central solution should.

[rupertj]

This would also be useful for work on the subsites_extras module that we've got planned.

[ekes]

Ignoring recipes for now, just designing within our ecosystem, I think the reference to assigning permissions to our roles is very good.

What both are providing is helpful default initial configuration for LocalGov Drupal sites installing modules.

I'm wondering why do we put the roles one into a separate sub-module? Rather than directly in localgov_core. Do we want to make it so people can switch it off if they want? Maybe roles and blocks initial configuration functionality could go into localgov_core directly?

I also don't think it necessarily needs to be an enforced dependency. Most LGD modules do depend on Core. But the ones that don't shouldn't require it for this. If you are installing those modules, which don't depend on core, into another site you just have to do the configuration yourself. The vast majority of users will have Core and will get the initial default configuration.

Last thought. Looking at what the hooks are returning. Would it be neater that this is yml? It's very much a pattern in Drupal for defining configuration, even in contrib (like Group has it's own module.group.permissions.yml). Could we write a hook_modules_installed that scans for installed_module.localgov_drupal.blocks.yml and installed_module.localgov_drupal.permissions.yml (or config/localgovdrupal/blocks.yml and config/localgovdrupal/permisions.yml). I could see this then being a useful pattern for other things down the line.

[andybroomfield]

I think the simpler method could be to keep the current format of providing a default block in config/optional for localgov_base and then using a hook simmilar to hook_localgov_roles_default to then place the block in active theme.

Or this might not need to be a hook, just a feature that scans for localgov_base blocks and if the active theme is derived from that then place the block.

[andybroomfield]

@ekes I did wonder why localgov_roles is a seperate module, thought that was simmilar to localgov_media in that it provides the default roles, but not every site will want those.

[rupertj]

Stephen commented in the tech Meetup that we should check if the active theme is a sub theme of localgov_base.

[rupertj]

Add a settings switch to enable/disable this for blocks.

[rupertj]

@willguv This is the issue about blocks that I mentioned in the meeting yesterday. There's a PR linked in my first comment, and me and @ekes have linked in issues around block placement from our other modules that this PR could fix.

[willguv]

@finnlewis could the PR that @rupertj mentioned be reviewed soon please? It's blocking the wider adoption of pubs and subsite extras by councils.

While the Netcall issue is urgent, this one is more urgent. Thanks for looking!