Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should modules that provide content types be uninstallable / reinstallable? #762

Open
andybroomfield opened this issue Aug 28, 2024 · 7 comments

Comments

@andybroomfield
Copy link
Contributor

This stems from localgov_news, and from localgovdrupal/localgov_news#119

The PR is about making all existing config be enforced so it uninstalls. The node and field types of localgov_news are already enforced. This means that when localgov_news is uninstalled, the content type for the news is also uninstalled. This can be a problem if there are existing content of provided type.

See localgovdrupal/localgov_news#120

Other content types, notablly localgov_services, do not enforce config. Instead they will leave the services content types (landing, sub landing, page) and associated fields intact. This does mean that those modules cannot be reinstalled untill the provided config is deleted.

@andybroomfield
Copy link
Contributor Author

@millnut
Copy link
Member

millnut commented Sep 2, 2024

We could look at creating a module uninstall validator using the Drupal\Core\Extension\ModuleUninstallValidatorInterface and inform the users that the module cannot be uninstalled as content exists for the news content type. This way it's a user decision to remove all the content before uninstalling the module.

@ekes
Copy link
Member

ekes commented Sep 3, 2024

What are our use cases here?

Like with the content and the content types left in place site developers can ...
Forcing site developers to remove all content to be able to disable and re-enable modules because...

@ekes
Copy link
Member

ekes commented Sep 3, 2024

We could look at creating a module uninstall validator using the Drupal\Core\Extension\ModuleUninstallValidatorInterface and inform the users that the module cannot be uninstalled as content exists for the news content type. This way it's a user decision to remove all the content before uninstalling the module.

MT: We should certainly check that we're not leaving orphaned content in the db before removing the config.

@finnlewis finnlewis changed the title Should modules that provide content types be uninstallable / reinstallable Should modules that provide content types be uninstallable / reinstallable? Dec 3, 2024
@rupertj
Copy link
Member

rupertj commented Jan 4, 2025

What are our use cases here?

Like with the content and the content types left in place site developers can ... Forcing site developers to remove all content to be able to disable and re-enable modules because...

I think in general, letting a module be uninstalled completely and then reinstalled cleanly is the right thing to do. It lets people experiment without fear of things breaking.

If you'd like a specific example, some of the early releases of publications didn't uninstall cleanly, and meant that a couple of councils who were really helpful and tried out early builds got bitten by not being able to install the first stable release. Ok, they arguably shouldn't have put an alpha release into production, but I think that we should do the right thing and make sure that people doing that shouldn't have a bad time, even if we wouldn't recommend it.

@markconroy
Copy link
Member

If we are going to do this, let's make sure there's a massive warning on the uninstall page saying "If you uninstall this module, you will lose all your content from "

@andybroomfield
Copy link
Contributor Author

If you try to uninstall this module when there are already news articles, those nodes will still exist and throw as WSOD.
If we are going to have this module being uninstallable and delete content types, we should prevent uninstallation if content exists.

localgovdrupal/localgov_news#120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants