- Issue created by @narendraR
- First commit to issue fork.
- Merge request !7953Draft: fix: validate views.settings.display_extenders โ (Open) created by mikelutz
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
YAY! Welcome, @mikelutz! ๐๐ฅณ Unfortunately GitLab seems to just have gone down!?! ๐ฑ I'm getting 503s!
- Status changed to Needs work
8 months ago 3:34pm 8 May 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Excellent start! Thanks for helping out making all of Drupal core's config validatable! I left some pointers in my review on the merge request ๐
- ๐บ๐ธUnited States mikelutz Michigan, USA
The test failure is on the display_extenders section. It's a sequence of enabled display_extender plugins, but it's inserted directly from the form values of a mulit-checkbox element. So enabled plugins look like "plugin_id: plugin_id" and disabled plugins look like "plugin_id: '0'". As it is now, validation would mean the value is either an existing display_extender plugin OR the string "0". So the question is do we write that, maybe as an extra option to PluginExists, or a one-off for this situation for views that can be used with the Choice constraint? Or do we start array_filtering the values from the form before storing them, and add an update hook to clean up existing configs first?
- ๐ฌ๐งUnited Kingdom scott_euser
scott_euser โ changed the visibility of the branch 3440962-add-validation-constraints to hidden.
- Status changed to Postponed
6 months ago 7:46pm 8 July 2024 - ๐ฌ๐งUnited Kingdom scott_euser
Okay I created ๐ Views display extenders should only save enabled extenders Needs review to prove that display_extenders issue + sort it with the array_filter. We are already filtering them on load in Drupal\views\Views::getEnabledDisplayExtenders() so it seems logical to filter them out on save. To avoid a more complicated update, I think it does not hurt to keep the array filter in the ::getEnabledDisplayExtenders().
So I think this is postponed until ๐ Views display extenders should only save enabled extenders Needs review gets merged right?