Steps to reproduce:

  1. Install Forum module
  2. Delete the "General discussion" forum taxonomy term
  3. Uninstall Forum module
  4. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

Bump. This is a problem for many modules which provide configs for other modules.

swentel’s picture

Hmm, there's ForumUninstallTest which tests this scenario as far as I can see, so maybe this isn't a problem anymore ?

longwave’s picture

Status: Active » Needs review
FileSize
762 bytes

ForumUninstallTest does not try to reinstall after uninstalling. This patch should demonstrate the failure.

Status: Needs review » Needs work

The last submitted patch, 4: 2534532-forum-reinstall.patch, failed testing.

michaellander’s picture

Status: Needs work » Needs review
FileSize
507 bytes

I think this is a dependency thing.

Try this:

michaellander’s picture

Here is both of them combined.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Aha, that is more straightforward than I realised.

longwave’s picture

Though, 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?

michaellander’s picture

longwave, 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?

michaellander’s picture

Thinking 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.

  • catch committed 2bfd810 on 8.1.x
    Issue #2534532 by michaellander, longwave: Cannot reinstall Forum after...

  • catch committed b6a1aad on 8.0.x
    Issue #2534532 by michaellander, longwave: Cannot reinstall Forum after...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/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.

Status: Fixed » Closed (fixed)

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