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.
currently, we use the methods on the storage controller, not the object, for non-config-entity config objects.
this stops us from reacting to changes to config objects from this code path, which is blocking other issues.
Comment | File | Size | Author |
---|---|---|---|
#4 | 1887244-4-use-config-methods.patch | 922 bytes | Anonymous (not verified) |
#1 | 1887244-use-config-methods.patch | 807 bytes | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedhere's a patch, let's see what breaks.
Comment #2
chx CreditAttribution: chx commentedThis would be fantastic and is a duplicate of at least two issues , one is the where we wanted to reintroduce hooks on import -- no need, just use the events the other is my meta whining. And, it would unblock ConfigEntities querying because we could use some wrapping of storage engines similar to the cached storage to write a flattened version (key1.key2.key3 - value) to the database allowing for quick querying with EntityQuery. Superb exciting.
Comment #3
sun1) With this we can skip the manual validateName(), since that's baked into Config.
2) Why don't we use the service instead of manually futzing with Config?
Not really happy about another/new usage of setData() here... setData() is going to be incompatible with any kind of context/typed-property system.
But OK, it's not the only setData() in core and this one doesn't make the situation worse.
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedah, good point, fixed.
because that's what we do elsewhere in this code, see config_import_invoke_owner(). please open another issue if you want to change that, i'm not introducing anything new with this.
Comment #5
chx CreditAttribution: chx commentedI would RTBC this but the decision is heyrocker's.
Comment #6
gddI feel like there is a reason we weren't doing this in the first place but I can't remember what it was and I'm not finding any references in past issues and tests pass so lets go with it. I will close #1886478: Bring back hook_config_import_CRUD() hooks as a duplicate, since non-config-entities can now act on the events instead.
Comment #7
webchick"this stops us from reacting to changes to config objects from this code path..."
Shouldn't we have a test for that then?
Comment #8
gddThe unit tests for import are complete and working, whether or not the event is properly being fired should be covered by unit tests around the event system. Unfortunately we don't have those yet, so I filed #1889474: Create tests for config event handling as a major followup. Going back to RTBC here if webchick is OK with that solution.
Comment #9
webchickOk cool, I think that works.
Committed and pushed to 8.x. Thanks!