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

Command icon 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:

Comments

godotislate created an issue. See original summary.

mxr576’s picture

Issue summary: View changes
mxr576’s picture

Title: Capture access cacheability in getDefaultOption() methods for all relevant entity list builders » Capture access cacheability in getDefaultOperations() methods for all relevant entity list builders
godotislate’s picture

Version: 11.x-dev » main
godotislate’s picture

Issue summary: View changes

godotislate’s picture

Issue summary: View changes
godotislate’s picture

Status: Active » Needs review

MR 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, except ConfigTranslationEntityListBuilderand FieldConfigListBuilder, 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.

mxr576’s picture

Status: Needs review » Reviewed & tested by the community

Changes looks good for me. Triggered the test only build and it failed as expected

https://git.drupalcode.org/issue/drupal-3540105/-/jobs/8237692

  • catch committed 878ae794 on 11.x
    fix: #3540105 Capture access cacheability in getDefaultOperations()...

  • catch committed 502a59b2 on main
    fix: #3540105 Capture access cacheability in getDefaultOperations()...
catch’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

godotislate’s picture

It's still worth investigating in #3543509: Remove ConfigEntityListBuilder::getDefaultOperations() override whether all these subclasses should be implementations getDefaultOperations() instead of adding to operations in getOperations() instead.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.