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
All views created in Drupal 8 have a root config key of core
with the value set to \Drupal::CORE_COMPATIBILITY
ie. 8.x
Proposed resolution
Discover why this information is needed in View (my suspicion is that this is a leftover from when Views was not in core and this information was move useful) - DONE: Not needed by core.
Remove this key using a update hook. - Done: @see views_post_update_remove_core_key()
Remaining tasks
Write change record- Final reviews/RTBC
- Commit
User interface changes
None
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#11 | 3087692-11.patch | 200.21 KB | Wim Leers |
#11 | interdiff.txt | 6.81 KB | Wim Leers |
#5 | 3087692-5.patch | 194.22 KB | Wim Leers |
#5 | interdiff.txt | 82.17 KB | Wim Leers |
#4 | 3087692-4.patch | 112.05 KB | alexpott |
Comments
Comment #2
MixologicWhen I was auditing CORE_COMPATIBILITY I found this, but could not, anywhere, see that it was necessary or being checked.
Comment #3
Berdir> Discover why this information is needed in View (my suspicion is that this is a leftover from when Views was not in core and this information was move useful)
Yes, IIRC this was carried over from views 7.x where it was used to detect version 2 or 3.
Comment #4
alexpottComment #5
Wim Leerscore: 8.x
butcore: '8'
. Another reason for #2869792: [meta] Add constraints to all config entity types…ViewStorageTest
's expectations.Comment #6
Wim LeersThis will also need an update path to convert all contrib/custom exported views.
Comment #7
alexpott@Wim Leers it doesn't
Because we're removing the core key from the exported property list in the annotation during module install it'll remove the key :)
Yes people need to re-export their views (or remove the key from exported config) but there's nothing more we can do in core.
Comment #8
Wim LeersRight, they don't need to change code. But they need to re-export.
We had to do something similar shortly before the Drupal 8 release. I recall that being considered too disruptive. We had to take extra measures to make that painless.
I should've phrased #6 more clearly: will my existing site continue to work, because the
core
key will just be ignored?AHHHH I had missed
views_post_update_remove_core_key()
🤦♂️🤦♂️🤦♂️🤦♂️ Sorry for the distraction 😅Comment #9
dww+1 to this. The
core:
key in the views config is cruft at this point. We don't have a system any more for version-dependent views config schemas or anything like that. If/when we decide we need it, we'll need to add real support for it. At this point, nothing cares.However, this seems worthy of a CR, especially if contrib maintainers are expected to remove this key from default views they're shipping, etc.
Thanks,
-Derek
Comment #11
Wim Leerscore: 8.0-dev
🤦♂️🤦♂️🤦♂️ — see #5.2 again.Comment #12
rensingh99 CreditAttribution: rensingh99 at Red Crackle commentedComment #13
Wim LeersWhat's left here is only review + RTBC. @alexpott already wrote a change record (https://www.drupal.org/node/3087832) minutes after #9 was posted :)
Comment #14
Krzysztof DomańskiI found additional views configuration files with containing core key.
find core -name '*view*.yml' -exec grep -r 'core:.*8' {} \; -print
Comment #15
dww@Krzysztof Domański - those are fixtures for automated tests to validate DB schema updates. The entire point of those files is to have "stale" data. We could never test this change if we "fix" the fixtures. Those files need to represent the "old" data in Drupal, so tests can start from known old states, run their updates, and assert that the old stuff got correctly converted to the new stuff.
Back to NR.
Comment #16
Wim LeersStill applies cleanly. Retesting.
Comment #17
rensingh99 CreditAttribution: rensingh99 at Red Crackle commentedHi,
I have reviewed the patch # 11 and it worked as designed. It’s removed the core key from all views which is included in core.
Now core key is also removed if we export the configuration for any view.
Export screenshot before applying patch.
Export screenshot after applying patch.
Thanks,
Ren
Comment #21
catchCommitted/pushed to 9.0.x, cherry-picked to 8.9.x and 8.8.x, thanks!
Comment #22
Wim LeersPublished change record: https://www.drupal.org/node/3087832.
Comment #23
rensingh99 CreditAttribution: rensingh99 at Red Crackle commentedHi,
I have reviewed this patch but I did not get any credit.
Please advise me on how I can improve for the next issues.
Thanks
Ren
Comment #24
catchI've retrospectively added commit credit for this. Was a bit borderline for me because we have explicit test coverage in the patch, so the issue did not really need manual testing, but the manual testing that you did was fine.