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.
While working on #1831774: Config import assumes that 'config_prefix' contains one dot only, I realized that breakpoint.module is the only place in core other than storage controllers and the storage controller test that directly translates config object names into config entity IDs.
Also in there are calls like
config_get_storage_names_with_prefix()
drupal_container()->get('config.storage')->listAll()
entity_get_info() x 2
entity_load_multiple() x 2
Those all scream to be contained in a storage controller.
Comment | File | Size | Author |
---|---|---|---|
#13 | interdiff.txt | 7.06 KB | tim.plunkett |
#13 | breakpoint-1915272-13.patch | 11.59 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettThis is still crazypants.
Comment #2
RainbowArrayMy understanding is that the main change needed here is to create a storage controller. Would that also resolve the issue of translation of config object names into config entity IDs?
If there's an example of a similar conversion to a storage controller, I'd be glad to give this a try.
Comment #3
RainbowArrayI worked with timplunkett on IRC to set up a patch to take care of this move. There's a decent chance this breaks and will need work. Let's test and see!
Comment #4
RainbowArrayUploading the patch: usually pretty useful.
Comment #6
tim.plunkettI'm guessing some of these use statements can be removed now.
This should be renamed to something like deleteBreakpointsBySource or something?
We should implement EntityControllerInterface and use that for injecting these services.
"Removes"
There is a possibility we can rewrite this to use loadByProperties, but that should wait for the patch to pass first.
You're missing a comma after the } the line before entity_keys, which is why this failed to install.
Comment #7
RainbowArrayI must have messed up the patch last night, as it didn't cleanly apply to 8.x. I did a reroll of my own work (shouldn't make patches at 2 a.m. I guess), and then made the changes above except for the third suggestion, as I wasn't sure how to do that.
Hopefully this moves things in the right direction.
We made this patch in a demo on contributing to d8 at the Twin Cities Drupal Open House.
Comment #9
RainbowArrayI uploaded the wrong patch.
Comment #11
RainbowArrayComma error caused the failure, I think.
Comment #13
tim.plunkettAhh, this will always cause problems:
You had Drupal\breakpoint\BreakpointGroupStorageController but core/modules/breakpoint/lib/Drupal/breakpoint/Controller/BreakpointGroupStorageController.php
Extra "Controller" in there.
I also fixed up the DI, we need the breakpoint entity type and storage controller.
Comment #15
tim.plunkett13: breakpoint-1915272-13.patch queued for re-testing.
Comment #16
tim.plunkettComment #17
RainbowArraySince this is passing tests, what's next? Do we need some manual testing that deleting breakpoints still works?
Comment #18
tim.plunkettHmmm, #2080823: Create API to discover config entities' soft dependencies and use this to present a confirm form on module uninstall completely removes every line from this patch. Maybe this is just a dupe of that one?
Comment #19
alexpottI agree with #18 - this will become unnecessary once config entity dependencies are managed properly.