WSE Config fails to install properly on 11.x

This is happening because wse_config and workspaces are being installed at the same time. The wse_config module depends on workspaces (via the wse module), so workspaces is installed first. This means that when we loop round do entity updates we try and create a reference field on the wse_config entity type before the entity type actually exists. Because we process the workspaces in this loop before wse_config.

I think this is a core bug. I think we should process entity type creations before we do any field adding.

Yeah this is a core bug. We should only be processing field storage updates if the provider of the entity type is not in the module list.

This is a critical bug because it prevent correct installation of modules that provide fields for entity types they do not provide themselves.

Another way to recreate this problem with only core modules is to make taxonomy dependent on workspaces... and then install minimal and then taxonomy... kaboom! Works fine on 11.1.x.

#6 seems like a possible test coverage shortcut - could be done with hook_system_info_alter() in a test module.

Added a test to the MR per #6/#7. The tricky part was that the entity storage exception actually is caught by the module installer and logged. Then the workspace field ends up getting created correctly later anyway. Had to create a logger that would re-throw the exception.

Tweaked the service definition YML a bit and rebased. Not sure whether the log recorder should have more of an API with an interface defining methods, instead of expecting to work directly with state, but leaving as is for now.

That's a tricky test.

Coverage looks good and fix looks good.
I'm running test only just to be sure.

Took a couple of reads to see how it was working, but I'll keep it in mind if I ever need this.

Tried to make the issue title a bit more descriptive could probably use an issue summary update in case people end up back here in the future.

I think we can handle the logging without needing a new test module. Happy to defer that to a followup given this is a critical - please push back if so

Made the change so that the test class itself is the logger.

This looks good now. Once this is RTBC I'll file an issue to add a test trait to make KTB log testing very simple.

catch → closed merge request !10573

xpost

Looks like there's an existing issue about KTB log testing: ✨ Allow kernel tests to fail or expect logged errors Needs work .

Sadly the issue with modules providing fields for other entity types is not fully fixed, the Trash module tests also fail with a similar error like the one in the IS: https://git.drupalcode.org/project/trash/-/jobs/3799129

1) Drupal\Tests\trash\Functional\TrashNodeTest::testTrashAndRestoreNode
Drupal\Core\Entity\EntityStorageException: Exception thrown while performing a schema update. Cannot add field 'node_field_data.deleted': table doesn't exist.

FWIW, adding container_rebuild_required: true to trash.info.yml fixes them.

@amateescu did that fail with the same error before this commit or is it a new error now?

If we've not made things any worse here, I think we should continue in a follow-up, but re-opening for now so this doesn't get lost.

@catch / @amateescu I've confirmed that this change did not result in te trash module breaking this way. It was already broken by the module installer changes in 11.x. Going to mark this as fixed and open a new issue.

Created 🐛 Trash and node modules cannot be installed at the same time on 11.x Active

Automatically closed - issue fixed for 2 weeks with no activity.