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
We have 140+ default/test views in core with configurations created by UI/hand. Over this period, we have made few changes in plugins, configuration keys etc. So not all views configuration in same state.
Here is the list of views in core.
| Location | view config file |
|---|---|
| core/modules/action/tests/action_bulk_test/config/install | views.view.test_bulk_form.yml |
| core/modules/aggregator/config/install | views.view.aggregator_rss_feed.yml |
| core/modules/aggregator/tests/modules/aggregator_test_views/test_views | views.view.test_aggregator_items.yml |
| core/modules/block/tests/modules/block_test_views/test_views | views.view.test_view_block.yml views.view.test_view_block2.yml |
| core/modules/comment/config/install | views.view.comments_recent.yml |
| core/modules/comment/tests/modules/comment_test_views/test_views | views.view.test_comment_row.yml views.view.test_comment_rss.yml views.view.test_comment_user_uid.yml |
| core/modules/contact/tests/modules/contact_test_views/test_views | views.view.test_contact_link.yml |
| core/modules/content_translation/tests/modules/content_translation_test_views/test_views | views.view.test_entity_translations_link.yml |
| core/modules/dblog/tests/modules/dblog_test_views/test_views | views.view.test_dblog.yml |
| core/modules/entity_reference/tests/modules/entity_reference_test/config/install | views.view.test_entity_reference.yml |
| core/modules/entity_reference/tests/modules/entity_reference_test_views/test_views | views.view.test_entity_reference_view.yml |
| core/modules/field/tests/modules/field_test_views/test_views | views.view.test_view_fieldapi.yml |
| core/modules/file/config/install | views.view.files.yml |
| core/modules/file/tests/modules/file_test_views/test_views | views.view.file_extension_view.yml |
| core/modules/forum/tests/modules/forum_test_views/test_views | views.view.test_forum_index.yml |
| core/modules/node/config/install | views.view.archive.yml views.view.content.yml views.view.content_recent.yml views.view.frontpage.yml views.view.glossary.yml |
| core/modules/node/tests/modules/node_test_views/test_views | views.view.test_contextual_links.yml views.view.test_field_type.yml views.view.test_filter_node_uid_revision.yml views.view.test_language.yml views.view.test_node_bulk_form.yml views.view.test_node_revision_nid.yml views.view.test_node_revision_vid.yml views.view.test_node_row_plugin.yml views.view.test_node_view.yml views.view.test_status_extra.yml |
| core/modules/rest/tests/modules/rest_test_views/test_views | views.view.test_serializer_display_entity.yml views.view.test_serializer_display_field.yml views.view.test_serializer_node_display_field.yml |
| core/modules/statistics/tests/modules/statistics_test_views/test_views | views.view.test_statistics_integration.yml |
| core/modules/taxonomy/config/install | views.view.taxonomy_term.yml |
| core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views | views.view.test_filter_taxonomy_index_tid.yml views.view.test_groupwise_term.yml views.view.test_taxonomy_node_term_data.yml views.view.test_taxonomy_parent.yml |
| core/modules/tracker/tests/modules/tracker_test_views/test_views | views.view.test_tracker_user_uid.yml |
| core/modules/user/config/install | views.view.user_admin_people.yml views.view.who_s_new.yml views.view.who_s_online.yml |
| core/modules/user/tests/modules/user_test_views/test_views | views.view.test_access_perm.yml views.view.test_access_role.yml views.view.test_field_permission.yml views.view.test_filter_permission.yml views.view.test_groupwise_user.yml views.view.test_plugin_argument_default_current_user.yml views.view.test_user_bulk_form.yml views.view.test_user_data.yml views.view.test_user_name.yml views.view.test_user_relationship.yml views.view.test_user_uid_argument.yml views.view.test_view_argument_validate_user.yml views.view.test_view_argument_validate_username.yml views.view.test_views_handler_field_role.yml views.view.test_views_handler_field_user_name.yml |
| core/modules/views/tests/modules/views_test_config/test_views | views.view.test_access_none.yml views.view.test_aggregate_count.yml views.view.test_ajax_view.yml views.view.test_alias.yml views.view.test_area_title.yml views.view.test_area_view.yml views.view.test_argument_date.yml views.view.test_argument_default_current_user.yml views.view.test_argument_default_fixed.yml views.view.test_attachment_ui.yml views.view.test_cache.yml views.view.test_click_sort.yml views.view.test_destroy.yml views.view.test_display.yml views.view.test_display_attachment.yml views.view.test_display_defaults.yml views.view.test_display_empty.yml views.view.test_display_feed.yml views.view.test_display_invalid.yml views.view.test_display_more.yml views.view.test_dropbutton.yml views.view.test_entity_area.yml views.view.test_entity_row.yml views.view.test_entity_row_renderers.yml views.view.test_entity_type_filter.yml views.view.test_example_area.yml views.view.test_executable_displays.yml views.view.test_exposed_admin_ui.yml views.view.test_exposed_block.yml views.view.test_exposed_form.yml views.view.test_exposed_form_buttons.yml views.view.test_field_classes.yml views.view.test_field_get_entity.yml views.view.test_field_output.yml views.view.test_field_tokens.yml views.view.test_filter.yml views.view.test_filter_date_between.yml views.view.test_filter_group_override.yml views.view.test_filter_groups.yml views.view.test_filter_in_operator_ui.yml views.view.test_get_attach_displays.yml views.view.test_glossary.yml views.view.test_grid.yml views.view.test_group_by_count.yml views.view.test_group_by_in_filters.yml views.view.test_handler_relationships.yml views.view.test_handler_test_access.yml views.view.test_history.yml views.view.test_http_status_code.yml views.view.test_mini_pager.yml views.view.test_page_display.yml views.view.test_page_display_arguments.yml views.view.test_page_display_menu.yml views.view.test_page_display_route.yml views.view.test_page_view.yml views.view.test_pager_full.yml views.view.test_pager_none.yml views.view.test_pager_some.yml views.view.test_preview.yml views.view.test_redirect_view.yml views.view.test_search.yml views.view.test_simple_argument.yml views.view.test_store_pager_settings.yml views.view.test_style_mapping.yml views.view.test_style_opml.yml views.view.test_table.yml views.view.test_tag_cache.yml views.view.test_tokens.yml views.view.test_view.yml views.view.test_view_argument_validate_numeric.yml views.view.test_view_argument_validate_php.yml views.view.test_view_broken.yml views.view.test_view_delete.yml views.view.test_view_display_template.yml views.view.test_view_empty.yml views.view.test_view_handler_weight.yml views.view.test_view_pager_full_zero_items_per_page.yml views.view.test_view_render.yml views.view.test_view_status.yml views.view.test_view_storage.yml views.view.test_views_groupby_save.yml |
| core/modules/views/tests/modules/views_test_data/test_views | views.view.test_access_static.yml |
Proposed resolution
Revisit all views and make sure auto generated with latest config/plugin changes. It might not be applicable for test views in views module.
Remaining tasks
Discuss and make sure it is necessary.
Issue patch
Test.
User interface changes
N/A
API changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #29 | 2316909-2.29.patch | 303.03 KB | alexpott |
| #23 | 2316909.23-interdiff.txt | 1.34 KB | Berdir |
| #23 | 2316909.23.patch | 317.48 KB | Berdir |
| #17 | 2316909.17.patch | 308.62 KB | alexpott |
| #14 | resaved_views-2345725-14.patch | 99.58 KB | Berdir |
Comments
Comment #1
vijaycs85Comment #2
dawehnerWell, just like all config shipped with core modules, views also has to regularly be resaved based upon core/its modules changes.
It almost feels like we would need a recurring issue for that :)
In general for tests I would like to stay away from requiring a full blown export as otherwise writing tests will be impossible,
given that you'd need to get it working in the actual UI.
Comment #3
dawehnerSo +1 for suggesting to rexport the default ones again.
Comment #4
Gábor HojtsySounds like somewhat overlapping with #2325269: Test and fix views in test_views directories against their configuration schema, right?
Comment #5
vijaycs85Agreed. but we need to make sure the test configs are excluded for config schema or properly hand written, so won't throw any error on config schema.
Comment #6
dawehnerOn top of that I also hat that all codes implicit assumes that all config is set always. This feels more like a implementation detail of the config system in an abstract way.
Comment #7
Gábor HojtsyWell, in fact config schemas don't assume completeness, the validation just validates the keys that are there are according to their type definition. That is why regenerating test/default config based on the schema and diffing that with their current core state would help spot both outdated config and missing schemas. (ie. to remove items that are not in schema and compare with the starting point).
Comment #8
vijaycs85as we are in beta stage, I guess we can fix this one as it can solve lots of other config schema issues like #2325269: Test and fix views in test_views directories against their configuration schema
Comment #9
alexpottPromoting this to critical since it blocks #2325269: Test and fix views in test_views directories against their configuration schema and ensuring views comply with schema allows them to be translated. Postponing on #2267453: Views plugins do not store additional dependencies since that issue will change all views to expose their dependencies.
Comment #10
alexpottComment #11
tim.plunkettPer #9
Comment #12
catchComment #13
Wim LeersComment #14
BerdirDid not see a quicker way to do this than manually import all views if necessary (test_views directory) and then update using full or single export.
All modules up until statistics, including test views updated. Will continue another time.
Comment #15
dawehnerThank you, quite some work!
There are confusing things in there:
Why should we have to add a new field?
Comment #17
alexpottI've got a script to do this - https://gist.github.com/alexpott/f0650fbb7954df8497e4 (this script is built to do this for any config entity type - its to be run using drush scr)
The script fixes all the files and outputs:
Neither of these appear to be used to removing.
Comment #19
BerdirNice, that is a lot easier than what I did ;)
I checked the difference of our two patches and they are quite different. Take the mentioned content_recent above, your patch only removes a single dependency there and does not add the provider lines, for example? That said, I have no idea why mine would add that field, I don't think I made any manual changes...
Wondering if that is because you have different modules installed, so the schema is not present?
Comment #20
alexpottThe provider key should have been removed by #2267453: Views plugins do not store additional dependencies it looks like you weren't running with that patch - since that would also account for the dependency removals in your patch.
Comment #23
BerdirYeah, I messed up.
The test fails are because the display order changed, @alexpott mentioned that there is an issue to make that more reliable, for now, changed the tests to sort explicitly.
Comment #24
vijaycs85Thanks @alexpott for the nice script. One thing I noticed in the config is every time we generate config, the order of elements get shuffled. Though it doesn't matter as it is yaml file, it can make the config reviews so hard.
Is there any issue to fix the order? otherwise, we should raise one.Update: raised #2378503: Save configuration in a consistent order
Comment #25
vijaycs85Comment #26
dawehner@vijaycs85
That sorting everytime you safe is fixed in #2350821: Sort views displays by display name ... its freaking annoying if you actually build a site.
Comment #27
vijaycs85@dawehner, good point. Though I agree that the special treatment for displays is necessary(default, block, page, others), Do you think it can/should be done by a separate element (say weight field), so that the overall config can still stay in a generic order?
Comment #28
alexpottWe should postpone this on #2350821: Sort views displays by display name
Comment #29
alexpottNew patch now that #2350821: Sort views displays by display name has landed. The tests that failed #17 pass without the fixes from #23.
Comment #30
vijaycs85I guess, reviewing a configuration update patch generating by a script == reviewing a feature updated patch in drupal 7. i.e. there *can not* be anything wrong in the generated code, until there is nothing wrong in the generated script. So definitely this is RTBC.
However it is also important to get the script reviewed and *may be* make it as part of core before we get this patch in.
Comment #31
dawehnerCame up with a bunch of things I saw in the rexport, read through the full patch, sorry alex ...
Issues which cleary have been a schema bug, got reported on https://www.drupal.org/node/2380457
Other questions are attached onto this issue:
Mh, this is an integer, are we sure NULL is a proper value? I would argue that the proper value should rather be 0?
Mh, null instead of an empty string? This is odd, given that the schema looks correct.
Can we do something about those things?
This is defined as integer, mhh
OH
DONE, this is not the first time I will dream views exports :p
Comment #32
Gábor HojtsySome test views are missing plugin_id elements and views schemas are very much centered around wiring the definitions together with the plugin_id. #2379683: Fix configuration schema issues in contact (indirectly user and system) modules adds plugin_ids for a test view which fails in those tests. As we add more tests, we'll find more of these but looks like this script does not identify / rectify those. Not sure if that is realistic to expect from this script though. For illustration of how much views relies on plugin_id:
Comment #33
Berdir@dawehner: null is afaik always a valid value.
Should we postpone this again on the schema fixes in #2380457: Some fixes of the views config schema ? Otherwise we have to do it again after we changed it.
Comment #34
BerdirAnother angle at "reviewing" this, I combined this patch with #2325269: Test and fix views in test_views directories against their configuration schema, and the amount of errors when from 1214 to 191, so this certainly helps, but still quite a few errors. Attaching the patch, so we can see the remaining errors, maybe we can also fix them in #2380457: Some fixes of the views config schema or yet another issue, but again, the re-save only really makes sense once the schema is (mostly) correct...
On the other side, I suspect that quite a few of the remaining problems is left-over stuff, that should be removed, which in turn is only useful *after* this :)
Comment #36
alexpottI think there is value in committing #29 and then working on the other issues and then doing it again - now we have https://gist.github.com/alexpott/f0650fbb7954df8497e4 this does not take long. It'll massively reduce the noise in other views schema issues.
Comment #37
Gábor HojtsyI agree this would massively reduce the noise as demonstrated by Berdir. Let's commit #29.
Comment #38
catchYes removing noise is good. Let's do this now, then again when it's useful.
Comment #40
vijaycs85yay! thanks all. We might need to run this again once after fixed all open config schema issues (like #2380457: Some fixes of the views config schema).
Comment #41
xjmComment #43
alexpottCreated #2567183: Re-export all built-in configuration in core