Steps to reproduce:
- Install Forum module
- Delete the "General discussion" forum taxonomy term
- Uninstall Forum module
- Reinstall Forum module again
A PreExistingConfigException is thrown with message 'Configuration objects (comment.type.comment_forum, core.entity_form_display.comment.comment_forum.default, core.entity_view_display.comment.comment_forum.default, field.field.comment.comment_forum.comment_body) provided by forum already exist in active configuration'
Should Forum module clean this up on uninstall? If so, should this be explicit in hook_uninstall, or automatically handled? This scenario could also happen in other cases, especially in contrib, where modules may provide config on behalf of other modules they depend on.
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff-2534532-4-7.txt | 359 bytes | michaellander |
#7 | cannot_reinstall_forum-2534532-7.patch | 1.24 KB | michaellander |
#6 | cannot_reinstall_forum-2534532-6.patch | 507 bytes | michaellander |
#4 | 2534532-forum-reinstall.patch | 762 bytes | longwave |
Comments
Comment #1
longwaveI think this is related to #2461513: The ConfigInstaller should add a dependency on the extension used to install the configuration
Comment #2
TR CreditAttribution: TR commentedBump. This is a problem for many modules which provide configs for other modules.
Comment #3
swentel CreditAttribution: swentel as a volunteer commentedHmm, there's ForumUninstallTest which tests this scenario as far as I can see, so maybe this isn't a problem anymore ?
Comment #4
longwaveForumUninstallTest does not try to reinstall after uninstalling. This patch should demonstrate the failure.
Comment #6
michaellander CreditAttribution: michaellander at Elevated Third commentedI think this is a dependency thing.
Try this:
Comment #7
michaellander CreditAttribution: michaellander at Elevated Third commentedHere is both of them combined.
Comment #8
longwaveAha, that is more straightforward than I realised.
Comment #9
longwaveThough, rather than explicitly adding the module itself to each of its config YAML files, maybe #2461513: The ConfigInstaller should add a dependency on the extension used to install the configuration is the proper way to solve this after all?
Comment #10
michaellander CreditAttribution: michaellander at Elevated Third commentedlongwave, I think that other issue would be a great addition, but the way I handled it is based on node.type.forum.yml and node.type.book.yml(book module). So I think this would be the current proper way, and the other would be a nice to have?
Comment #11
michaellander CreditAttribution: michaellander at Elevated Third commentedThinking on this a bit more, I wonder if we should write a separate test for this? Doesn't seem like reinstall belongs in the uninstall test? I'm also curious if this occurs in other core modules.
Comment #14
catchCommitted/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!
We could possibly expand the install/uninstall test to also include re-install, but that test is already a monster so I think it deserves its own issue.