Views handler loading should respect configuration

Okay a bit of progress, but that's all from me for today. I think there are some tests where its hard to tell if the test is a fail because we are truly breaking something, or because the test is trying to avoid having to fully reproduce a real-world scenario in order to keep the test small.

An example is core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php. It appears to attempt to override the views data (rather than using the $override or changing the config) and calls some sort of advancedRender() method. I think what we should do for these type of failures is override the config rather than views data within the test method. There are probably quite a few tests that would need to be fixed in order to do that.

So it seems the fix is probably the tip of the iceberg and the tests relying on this are going to be where the real work is here. The question is, is there a real risk we break things in contrib here that do something similar to what the tests are doing? Ie, enough to e.g. only be able to push this in D12 for example.

The question is, is there a real risk we break things in contrib here that do something similar to what the tests are doing

I think that's definitely a concern, as well as custom sites overriding plugins in data_alter hooks without updating their config... How would we deal with that?

Okay a more realistic example is BlockExposedFilterAJAXTest::testExposedFilteringAndReset as that's a functional JS test. In that case the config used by that test is at views.view.test_block_exposed_ajax.yml which has the plugin_id for content type set to 'in_operator' and that causes the select to have Yes No instead of the list of content types. Again it could be that this is because we have created the config manually, and a real-world scenario editing the View would have saved the correct plugin ID.

I did run our Views Reference Field test suite against this without issues and we do make a lot of modifications there.

@catch I think we might need to defer to you whether to pursue this route further vs go back to have the two options in ✨ Continuation Add Views EntityReference filter to be available for all entity reference fields Needs work rather than trying to replace in place.

In that case the config used by that test is at views.view.test_block_exposed_ajax.yml which has the plugin_id for content type set to 'in_operator' and that causes the select to have Yes No instead of the list of content types. Again it could be that this is because we have created the config manually, and a real-world scenario editing the View would have saved the correct plugin ID.

That seems very likely to me, seems like a result of hand-editing.

It is possible that a contrib or custom module has updated filter plugin IDs without a corresponding update to update config, but it would have to do that without the views ever having been resaved. A bit hard to tell without some real-world testing on long-established sites probably.

Okay I created 📌 Make the comments in Views handler Manager around overrides clearer Needs review for the '@ this is cra...' comment (which probably we anyways should not use this specific wording). Hopefully that's a simple/quick fix.

For the automated tests, its going to be a big job as there are tons of cases. I will try to slowly plow through them but please do expect a quite large change to views tests directory. So please do flag if you feel they need to be broken into separate issues. I think it probably makes sense to keep them here as they are best revealed by this code change in ViewsHandlerManager.

Also opened 📌 Try to remove the override functionality from views handlers Active to try to find out how bad it is.

Thanks for the feedback on the progress so far! Replied to comments, will continue to work on this

I also added the related issues that will block this issue.

See MR discussion. Uploading a modified views.view.test_serializer_display_field.yml from rest_test module and also the diff vs. 11.x

I had to call a couple of methods from ViewTestData, install views_test_data module, edit the view, save it, export it, run the test, get config validation errors, hack out the extra keys, import it again, and that fixes the initial test failure for StyleSerializerEntityTest.php (while introducing another one which I haven't looked into yet).

This confirms that these views exports are incredibly old and have not been maintained over time, so we are testing invalid views exports everywhere. Not really surprising but a shame this issue that's already been split from a 500+ comment two part issue is the one dealing with it.

I think we should open another spin-off issue to discuss how we could maintain these properly over time. Config validation might help, but it doesn't solve the problem of having to manually import, save, export the views.

Spun off issue here 🌱 [meta] Decide how to manage test view configuration so it remains up to date Active . Will try to get my head back into this for sorting out this particular issue.

I guess continuing is probably better than waiting to figure out a way to automatically solve it as it is probably going to be complicated as the Views config will change depending on which other modules are enabled at the time. We could look at module/view dependencies, but as your diff shows, we cannot trust that the view config dependencies were correctly set up in the first place either.

Yeah I have no idea how to solve automating this, especially with views_test_data because you can't even just install the module to get it usable, have to create state entries first, but good to have an issue open in case someone else has ideas.

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Started work on this again. As an FYI this will get blocked by 🐛 Views content language field default configuration should use field_language plugin Needs review as we will need that to get the tests here to pass I think.

Okay tests all passing now. Finally got around to fixing the views config for those two failing functional JS tests, both outdated views config.

I wonder whether we can add a deprecation that detects when a views data/views data alter is attempting to change the plugin ID used.

If there is a contrib module relying on views data/views data alter to provide an alternative filter plugin ID already, anyone who has exported their config should be fine as the plugin_id saved should be the altered one. They would however need to change their approach for future installs which could be for example:

• Install hook that changes plugin_id
• Wait for ✨ Configurable views filters to allow for different widgets Active and let users select their plugin_id

So this puts pressure on getting the latter in. If anyone has an examples of contrib modules that rely on views data/views data alter to provide a custom handler plugin would be helpful to be able to test.

Note that we still have 🐛 Views content language field default configuration should use field_language plugin Needs review to get in which was flagged to do as a separate issue in the MR comments which could use a nudge back to RTBC (if agreed its ready for that)

Should this be postponed on 🐛 Views content language field default configuration should use field_language plugin Needs review ?

Hmmm I can imagine there is going to be a lot more back and forth in review here. Technically yes it needs to wait but I expect this particular issue to need a fair bit longer so ideally it gets reviewed first to get the back and forth ball rolling.

Just committed 🐛 Views content language field default configuration should use field_language plugin Needs review