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

added custom module dependencies enforced section under install/config for #1707 #812

Merged
merged 2 commits into from
Mar 31, 2021

Conversation

wgilling
Copy link

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:

$ drush pm-enable islandora_breadcrumbs

In PreExistingConfigException.php line 65:
                                                                                                                         
  Configuration objects (islandora.breadcrumbs) provided by islandora_breadcrumbs already exist in active configuration  

What's new?

A in-depth description of the changes made by this PR. Technical details and possible side effects.

  • the config/install yml file was updated to enforce the module has install "islandora.breadcrumbs" settings to uninstall when removed.

How should this be tested?

  • check out this code branch
  • from the command prompt
    • run cd /var/www/html/drupal/web
    • run drush pm-uninstall islandora_breadcrumbs
    • run 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.

@seth-shaw-unlv
Copy link

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$ 

@seth-shaw-unlv
Copy link

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();
}

@elizoller
Copy link
Member

I can confirm same behavior as Seth. Just for fun I did a rename of islandora.breadcrumbs to islandora_breadcrumbs.settings and enabled, uninstalled, and enabled again and that fixed the issue as someone suggested in the tech call today. it was three changes. rename of the config/install yml file and change name in the schema.yml and change name in the IslandoraBreadcrumbBuilder

@elizoller
Copy link
Member

Although changing the name of the schema and the config might be worth an update hook for existing installs

@dannylamb
Copy link

Let's do that then. @adam-vessey was right.

@adam-vessey
Copy link

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

@elizoller
Copy link
Member

i didn't test that - i left the dependency declarations as is from the PR

@wgilling
Copy link
Author

Adjusted the code to rename the config values islandora_breadcrumbs.breadcrumbs. Ready for re-review.

@seth-shaw-unlv seth-shaw-unlv self-requested a review March 31, 2021 17:31
Copy link

@seth-shaw-unlv seth-shaw-unlv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised. 👍

@seth-shaw-unlv seth-shaw-unlv merged commit d76a664 into Islandora:8.x-1.x Mar 31, 2021
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

Successfully merging this pull request may close these issues.

5 participants