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
When writing a custom Views Display Extender plugin, you may need to validate it before submission of view edit form. Validation of plugins is performed in the validate() method of the DisplayPluginBase class. However, there is no validation of extender plugins in this method.
Proposed resolution
Add display extenders validation in DisplayPluginBase::validate().
Comment | File | Size | Author |
---|---|---|---|
#10 | views-display-extenders-validation-2967567-3.patch | 2.53 KB | WalkingDexter |
Comments
Comment #2
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedPatch with proposed resolution.
Comment #3
dawehnerNice! Do you mind expanding the test in
core/modules/views/tests/src/Kernel/Plugin/DisplayExtenderTest.php / \Drupal\views_test_data\Plugin\views\display_extender\DisplayExtenderTest
??Comment #4
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedThe changes relate to the validation of the view, and not to the display extenders. Testing the validation of the view is already implemented in \Drupal\Tests\views\Kernel\ViewExecutableTest.
Comment #5
borisson_Either way, we need tests to ensure that this behavior doesn't regress over time.
Comment #6
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedOk, what tests are needed? I mean, if we expand the test
\Drupal\views_test_data\Plugin\views\display_extender\DisplayExtenderTest
, then what functional needs testing? Just checking that the validation of display extender returns correct errors?Comment #7
borisson_#6 yes. Just checking that we return the correct errors is exactly what we need :)
Comment #8
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedAdded testing of validation.
Comment #10
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedAdded testing of validation [2].
Comment #11
borisson_I think the tests here look really solid and it looks like it tests the correct thing.
Comment #12
LendudeDo we need a CR for this? Since this could 'break' existing sites (well they would have been broken, but silently, and after this change, it won't be silent anymore).
Comment #13
catchLet's add the change record, CNW for that and tagging.
Comment #15
WalkingDexter CreditAttribution: WalkingDexter at Initlab commentedCreated a change record => https://www.drupal.org/node/2986918
Comment #16
andypostLooks great to go
Comment #17
alexpottCommitted b659cbd and pushed to 8.7.x. Thanks!