Problem/Motivation

This is a follow-up for #2404105: When a profile installs a block for a theme, it is created for all enabled themes.

There we found out when you install a site with some themes enabled. And then add for example claro to the core.extensions file and execute a drush cim. Then block_theme_initialize creates some blocks, but they get immediately deleted.

$ drush cim                                                                                                                                (9.2.x|✚6…)
+------------+----------------+-----------+
| Collection | Config         | Operation |
+------------+----------------+-----------+
|            | core.extension | Update    |
+------------+----------------+-----------+

 Import the listed configuration changes? (yes/no) [yes]:
 > y

 [notice] Synchronized extensions: install claro.
 [notice] Synchronized configuration: delete block.block.claro_tools.
 [notice] Synchronized configuration: delete block.block.claro_main_menu.
 [notice] Synchronized configuration: delete block.block.claro_footer.
 [notice] Synchronized configuration: delete block.block.claro_account_menu.
 [notice] Synchronized configuration: delete block.block.claro_branding.
 [notice] Synchronized configuration: delete block.block.claro_breadcrumbs.
 [notice] Synchronized configuration: delete block.block.claro_content.
 [notice] Synchronized configuration: delete block.block.claro_messages.
 [notice] Synchronized configuration: delete block.block.claro_powered.
 [notice] Synchronized configuration: delete block.block.claro_search.
 [notice] Synchronized configuration: delete block.block.claro_help.
 [notice] Synchronized configuration: delete claro.settings.
 [notice] Synchronized configuration: delete block.block.claro_local_actions.
 [notice] Synchronized configuration: delete block.block.claro_local_tasks.
 [notice] Synchronized configuration: delete block.block.claro_page_title.
 [notice] Finalizing configuration synchronization.
 [success] The configuration was imported successfully.

Proposed resolution

block_theme_initialize should not run, so that there is no need to delete these created blocks.

CommentFileSizeAuthor
#6 block_theme_initialize_no_create-3182716-6.patch658 bytesPrimsi
#5 block_theme_initialize_no_create-3182716-5.patch657 bytesPrimsi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chr.fritsch created an issue. See original summary.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Active » Needs review
FileSize
block_theme_initialize_no_create-3182716-5.patch657 bytes

Discussed this with Berdir. Here is an initial patch.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
FileSize
block_theme_initialize_no_create-3182716-6.patch658 bytes
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative, +Needs tests

Seems like a perfect test case scenario.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Actually not sure if this is the best approach? With or without the patch the theme being installed will not get the default blocks you would get when enabling a theme the normal way.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I don't know what you mean. That's the whole point of this issue? Config import must not never have any side effects, it should only import exactly the configuration that you prepared and nothing. You have to install as usual the theme at some point, then it initializes the blocks, then you customize them, and then you export and import the final state.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Ah just doesn’t seem like a valid workflow. But if that’s the case I don’t think there’s any test case to add. Unless there is way to see output from drush.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs work » Needs review
Issue tags: -Needs tests

Based on #9 the patch in #6 does what is expected.

I manually edited my core.extension.yml file by adding bartik to it
Did a drush cim
Verified I did not see any blocks created.

Since with or without the patch the blocks aren't there after configuration is imported I'm not sure there is a test for this. Unless we can see the output of drush cim.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

One way to test for it would be to have a hook_block_insert() implementation in a test module and registering what kind of blocks get deleted during config import, but yes, I'm not sure it's worth that.

That, or we make a kernel test where we directly call that function and simulate that config sync is on, that might be easier?

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

I vote for the 2nd option.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Actually not 100% how to simulate the act of importing partially. Any good examples?

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

rcodina’s picture

rcodina CreditAttribution: rcodina at Seidor commented

I ended up here because I'm doing a migration from core 9.5.11 to 10.1.5. On 9.5.11 we had Adminimal theme as admin theme and now we are enabling and configuring the Claro theme. The enabled Claro blocks are all set in yml config files (default blocks, no custom modifications). The problem is that when I import the configuration using customer's environment database the following error shows up:

In EntityStorageBase.php line 519:                                                                                              
                                                                                                                                  
    'block' entity with ID 'claro_footer' already exists.

Using patch on #6 it avoids the import error. However, the blocks we have set in yml files doesn't get imported. Moreover, If do a "drush cex", it deletes the claro blocks I had defined on those files:

 [notice] Differences of the active config to the export directory:

+------------+-----------------------------------------+-----------+
| Collection | Config                                  | Operation |
+------------+-----------------------------------------+-----------+
|            | block.block.claro_breadcrumbs           | Delete    |
|            | block.block.claro_content               | Delete    |
|            | block.block.claro_messages              | Delete    |
|            | block.block.claro_local_actions         | Delete    |
|            | block.block.claro_page_title            | Delete    |
|            | block.block.claro_primary_local_tasks   | Delete    |
|            | block.block.claro_secondary_local_tasks | Delete    |
+------------+-----------------------------------------+-----------+

Why they get deleted? Why they don't get imported?

Once the configuration import using customer's environment database is done, If I uninstall claro and I reenable it using the UI, all the default blocks are recreated. Then, If I export the configuration and I compare it with previous claro blocks yml file, the only change is in the uuid property. So it seems that for deploying on customer environment, we would need to uninstall and install again the Claro theme.