Problem/Motivation
Configuration synchronization page is broken due to error.
Steps to reproduce
Remove the file core.extension.yml in config/sync directory.
Visit syncronise tab at: /admin/config/development/configuration
Get the below error:
TypeError: Argument 1 passed to Drupal\field\ConfigImporterFieldPurger::getFieldStoragesToPurge() must be of the type array, bool given, called in /modules/field/field.module on line 323 in Drupal\field\ConfigImporterFieldPurger::getFieldStoragesToPurge() (line 111 of core/modules/field/src/ConfigImporterFieldPurger.php).
Proposed resolution
Improve handling of empty config/sync in the field module in hook field_form_config_admin_import_form_alter.
| Comment | File | Size | Author |
|---|---|---|---|
| #29 | Screenshot at Apr 14 23-56-49.png | 82.32 KB | phthlaap |
| #13 | 3385934-nr-bot.txt | 1.37 KB | needs-review-queue-bot |
Issue fork drupal-3385934
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3385934-11.x
changes, plain diff MR !7103
- 3385934-configuration-synchronization-page
changes, plain diff MR !4720
Comments
Comment #2
useernamee commentedComment #4
useernamee commentedComment #5
cilefen commentedComment #9
joegraduateLooks like the target branch for the MR needs to be updated to to 11.x
Comment #11
joegraduateComment #12
joegraduateI cherry-picked @useernamee's commit into a new branch based on 11.x and created a new MR targeting 11.x
Comment #13
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
joegraduateComment #15
smustgrave commentedThanks for working on this issue.
Will need a failing test case to show the issue.
Comment #18
phthlaap commentedComment #19
joegraduateThis GitLab "Test-only changes" pipeline in MR !7103 fails as expected as of @phthlaap's latest changes:
Comment #20
joegraduateComment #21
smustgrave commentedSo I tried to replicate locally by emptying my config/sync folder and going to /admin/config/development/configuration but I don't get any error or see anything in the logs.
Could additional steps be added to the issue summary.
Comment #22
phthlaap commented@smustgrave just remove the file core.extension.yml instead of all .yml files
Comment #23
smustgrave commentedTitle and issue summary should be updated to reflect the issue.
Comment #24
phthlaap commentedComment #25
phthlaap commentedComment #26
smustgrave commentedTitle and issue summary have both been updated.
Seems like an edge case but definitely do see the issue, tests also show it
Tried to see if there is a better spot to put this test but anywhere else felt like a stretch for any other test function.
Think this should be good.
Comment #27
alexpottI feel this fix is in the wrong place. If core.extension is missing we should have bailed way before hitting hook_config_import_steps_alter.
Given the importance of core.extension I think we can add something to \Drupal\system\SystemConfigSubscriber::onConfigImporterValidateNotEmpty like
Eventually schema validation is going to do this for us.
Comment #29
phthlaap commented#27 It will impact the import configuration form when there are no changes in the core extension.
Comment #30
alexpottAhh interesting... must have forgotten something along the way... posted a formulation on the MR that will work...
Comment #32
phthlaap commented@alexpott the core.extension configuration is validated in core/lib/Drupal/Core/EventSubscriber/ConfigImportSubscriber.php:93. I think we no need to validate again.
It also have tests to cover: core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php::testMissingCoreExtension
But even when it is validated I think we also need fix the core/modules/field/field.module:324 to prevent the TypeError message when access the page /admin/config/development/configuration
Comment #33
phthlaap commentedComment #34
smustgrave commentedRe-ran test-only feature https://git.drupalcode.org/issue/drupal-3385934/-/jobs/1326634 which shows the test is still covering the change.
Believe the feedback from @alexpott has been addressed
I did save credit for the users who have worked on the issue, did not include just applying a suggestion will let committers decide that.
Comment #39
larowlanCommitted to 11.x and backported to 11.0.x, 10.4.x and 10.3.x
Comment #41
alexpott