-
Notifications
You must be signed in to change notification settings - Fork 118
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
added custom module dependencies enforced section under install/config for #1707 #812
Conversation
This doesn't appear to be working for me. I built a new dev box and pulled in the PR: vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ git checkout -b asulibraries-issue-1707 8.x-1.x
Switched to a new branch 'asulibraries-issue-1707'
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ git pull https://github.com/asulibraries/islandora.git issue-1707
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 7 (delta 3), reused 7 (delta 3), pack-reused 0
Unpacking objects: 100% (7/7), done.
From https://github.com/asulibraries/islandora
* branch issue-1707 -> FETCH_HEAD
Updating 836d521..3cc6ec8
Fast-forward
modules/islandora_breadcrumbs/config/install/islandora.breadcrumbs.yml | 6 ++++++
1 file changed, 6 insertions(+) I then uninstalled the module and attempted to re-enable it; which failed as we would expect (since the old config was still in place): vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush pm-uninstall islandora_breadcrumbs
[success] Successfully uninstalled: islandora_breadcrumbs
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush pm-enable islandora_breadcrumbs
In PreExistingConfigException.php line 65:
Configuration objects (islandora.breadcrumbs) provided by islandora_breadcrumbs already exist in active configuration
pm:enable [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--xh-link XH-LINK] [--druplicon] [--notify [NOTIFY]] [--] <command> [<modules>]... So I then purged the old config. I then attempted to re-enable the module (which should now include the fixed config), uninstall it again (which should cleanly remove the config), and then attempted to re-enabled the module, but the same error persists: vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush config-delete islandora.breadcrumbs
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush pm-enable islandora_breadcrumbs
[success] Successfully enabled: islandora_breadcrumbs
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush pm-uninstall islandora_breadcrumbs
[success] Successfully uninstalled: islandora_breadcrumbs
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ drush pm-enable islandora_breadcrumbs
In PreExistingConfigException.php line 65:
Configuration objects (islandora.breadcrumbs) provided by islandora_breadcrumbs already exist in active configuration
pm:enable [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--xh-link XH-LINK] [--druplicon] [--notify [NOTIFY]] [--] <command> [<modules>]...
vagrant@islandora8:/var/www/html/drupal/web/modules/contrib/islandora$ |
I've played around with this a bit and no combination of clearing cache or running cron helped. I wonder if this is a bug in core... The only thing that appears to work is moving the config from install to optional. (This doesn't purge it; it simply leaves it behind and doesn't complain when you re-enable it. Yay for no complaints; boo for orphaned configs.) Alternatively, we can try an uninstall hook: /**
* Implements hook_uninstall().
*/
function islandora_breadcrumbs_uninstall() {
\Drupal::configFactory()->getEditable('islandora.breadcrumbs')->delete();
} |
I can confirm same behavior as Seth. Just for fun I did a rename of |
Although changing the name of the schema and the config might be worth an update hook for existing installs |
Let's do that then. @adam-vessey was right. |
If the renaming fixes it, are the explicit dependency declarations presently introduced by this PR still required? Maybe a better place to point, but seems to be suggested that it's not necessary in https://git.drupalcode.org/project/drupal/-/blob/8.9.x/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php#L430-435 |
i didn't test that - i left the dependency declarations as is from the PR |
Adjusted the code to rename the config values islandora_breadcrumbs.breadcrumbs. Ready for re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as advertised. 👍
Related issue: (link)
Islandora/documentation#1707
What does this Pull Request do?
makes it so that the islandora_breadcrumbs module will clean up the configuration related to the module when it is uninstalled.
A brief description of what the intended result of the PR will be and/or what problem it solves.
When the drush command to install / uninstall this module was executed, there would be an error upon installing if the configuration remained behind from a previous install. That error would be as follows:
What's new?
A in-depth description of the changes made by this PR. Technical details and possible side effects.
How should this be tested?
cd /var/www/html/drupal/web
drush pm-uninstall islandora_breadcrumbs
drush pm-enable islandora_breadcrumbs
(this step would have displayed the error before the code change)You should see no errors. If you do no see any errors, the code change worked to remove the configuration.