Problem/Motivation

BlockViewBuilder currently only sets a block plugin's cache contexts if the block is cacheable.

This is wrong: cache contexts must always be set.

Proposed resolution

Fix it.

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#5 interdiff.txt804 bytesWim Leers
#5 block_view_builder_cache_contexts-2458413-5.patch4.26 KBWim Leers
#3 interdiff.txt1.07 KBWim Leers
#3 block_view_builder_cache_contexts-2458413-3.patch3.81 KBWim Leers
#1 block_view_builder_cache_contexts-2458413-1.patch2.79 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
block_view_builder_cache_contexts-2458413-1.patch2.79 KB

Status: Needs review » Needs work

The last submitted patch, 1: block_view_builder_cache_contexts-2458413-1.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
block_view_builder_cache_contexts-2458413-3.patch3.81 KB
interdiff.txt1.07 KB

And voila, the expected test failure, proving that we indeed have test coverage for this.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Needs work 
+++ b/core/modules/block/src/BlockViewBuilder.php
@@ -63,21 +69,20 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
+          'contexts' => Cache::mergeContexts($default_cache_contexts, $plugin->getCacheContexts()),

Sigh, I forgot to delete the original line below.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
block_view_builder_cache_contexts-2458413-5.patch4.26 KB
interdiff.txt804 bytes

Such a silly mistake.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed fd3a981 and pushed to 8.0.x. Thanks!

  • alexpott committed fd3a981 on 8.0.x 
    Issue #2458413 by Wim Leers: BlockViewBuilder should specify cache...

Status: Fixed » Closed (fixed)

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