Problem/Motivation
In #2473873: Entity operations lack cacheability support, resulting in incorrect dropbuttons, passing cacheable metadata to the entity lister builder operations was added. There are several list builder subclasses that implement getDefaultOperations() and getOperations, and while the commented out second parameter was added to the method signatures, the cacheability argument still needs to be captured via func_get_args() and also needs to be passed to the parent method call in each of them.
Pratically, what is missing is capturing and using extra method parameters in getDefaultOperations(), like this:
$args = func_get_args();
$cacheability = $args[1] ?? new CacheableMetadata();
$operations = parent::getOperations($entity, $cacheability);
In addition, there are list builders whose getDefaultOperations() methods do access checks that may need the cacheability of those access checks added to the passed in cache metadata, so that they are captured.
core/modules/field_ui/src/FieldConfigListBuilder.php
core/modules/taxonomy/src/VocabularyListBuilder.php
core/modules/workspaces/src/WorkspaceListBuilder.php
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3540105
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3540105-capture-access-cacheability
changes, plain diff MR !14507
Comments
Comment #2
mxr576Comment #3
mxr576Comment #4
godotislateComment #5
godotislateComment #7
godotislateComment #8
godotislateMR is ready: https://git.drupalcode.org/project/drupal/-/merge_requests/14507
I expanded test coverage in
Drupal\Tests\system\Kernel\Entity\EntityOperationsCacheabilityTest::testEntityOperationsCacheability()for most of the outstanding classes, exceptConfigTranslationEntityListBuilderandFieldConfigListBuilder, which were hard to set up for that method even with a data provider.If there needs to be test coverage for those two specifically, can look at adding it somewhere else.
Comment #9
mxr576Changes looks good for me. Triggered the test only build and it failed as expected
https://git.drupalcode.org/issue/drupal-3540105/-/jobs/8237692
Comment #12
catchCommitted/pushed to main and 11.x, thanks!
Comment #15
godotislateIt's still worth investigating in #3543509: Remove ConfigEntityListBuilder::getDefaultOperations() override whether all these subclasses should be implementations
getDefaultOperations()instead of adding to operations ingetOperations()instead.