Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The View entity calculates all the dependencies each display introduces on config save. The current code results in dependencies being calculated many times for the same handler or plugin. Also the View config entity is exposed to internals of the display plugin it should not be.
There are multiple examples of test coverage - perhaps the most relevant is Drupal\views\Tests\Entity\ViewEntityDependenciesTest
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 1.22 KB | olli |
#11 | 2413753-11.patch | 11.05 KB | olli |
#9 | interdiff.txt | 2.46 KB | olli |
#9 | 2413753-9.patch | 11.02 KB | olli |
#6 | 2413753.6.patch | 11.4 KB | alexpott |
Comments
Comment #1
alexpottThe patch attached moves the responsibility for calculating the dependencies each display adds to the display plugin. This moves views another step towards using EntityWithPluginCollectionInterface and having even less custom code in View::calculateDependencies().
The comment to add the dependency is removed because we have #2368767: Implement calculateDependencies() in ArgumentPluginBase to get dependencies from validator and default plugins which is taking care of that.
Comment #2
dawehnerI really like that.
Comment #3
alexpottRe-rolled now that #2368767: Implement calculateDependencies() in ArgumentPluginBase to get dependencies from validator and default plugins has landed.
Comment #4
dawehner... it would be nice, if we would have tests for that, I'd guess.
Comment #5
alexpottAdded a display level test to
ViewEntityDependenciesTest
Comment #6
alexpottOn the advice of @dawehner I added a code comment to explain that each display only calculates dependencies for plugins and handlers it configures.
Comment #7
dawehnerNice usage of array_walk!
Comment #9
olli CreditAttribution: olli commentedreroll, removed one use statement and changed @return of getAllPlugins().
Comment #11
olli CreditAttribution: olli commentedComment #12
dawehnerThis is still RTBC IMHO.
Comment #13
Wim LeersThis patch looks awesome!
Comment #14
catchCommitted/pushed to 8.0.x, thanks!