Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
shortcut_install()
doesn't check config installer syncing flag while editing the seven theme settings.
Proposed resolution
Update the shortcut_install()
so that it only edits config when config installer syncing flag is FALSE
.
Remaining tasks
Review.
Commit.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#26 | 2550357-nr-bot.txt | 85 bytes | needs-review-queue-bot |
#24 | 2550357-24.patch | 2.1 KB | alexpott |
#22 | 2550357-nr-bot.txt | 153 bytes | needs-review-queue-bot |
#13 | fix_shortcut_install-2550357-13.patch | 2.72 KB | jibran |
#6 | fix_shortcut_install-2550357-6-test-only.patch | 1.17 KB | jibran |
Comments
Comment #2
jibranCreated #2550385: Update hook_install documentation for editing configuration as doc follow up.
Comment #3
larowlanconsistent with forum_install()
Comment #4
jibranThank @larowlan.
Comment #5
alexpottIt strikes me that we are missing test coverage. ConfigImportAllTest is the place to do this. We should change the value after this...
And the test the value at the end of the test. Should just be a couple a lines and it'd be nice to have a failing patch to show people what we're on about.
Comment #6
jibranI created this patch but it doesn't fail. Uploading it without the fix. What am I doing wrong here?
Comment #7
jibranNeeds more investigation.
Comment #10
jibran@alexpott any idea how to proceed with this?
Comment #13
jibranWhy do we need to check this?
Why?
system_install
is setting the site uuid do we need to check config installer syncing flag there as well?Meanwhile, I have updated the patch. If we should be checking the flag then we should do it for both
shortcut_install
andshortcut_themes_installed
.Comment #22
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #23
alexpottComment #24
alexpottComment #25
alexpott@jibran - system is a special case. It's always installed and the site UUID is even more special with respect to configuration imports.
Comment #26
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.