Problem:
Cache setting “Block caching type” is not used in block instances. Instead DRUPAL_NO_CACHE is used.

How to reproduce:

  • Create Views block display.
  • Set cache to any value (except “Do not cache”).
  • Save View.
  • Place View block with any machine name.
  • You can see placed block settings in sites/default/files/config_[hash]/active/block.block.bartik.[machine_name].yml
    Cache setting with be always -1 (DRUPAL_NO_CACHE).

Now 'cache' and 'admin_label' is added via Drupal\views\Plugin\Derivative\ViewsBlock, but only admin label is added to settings by default: http://drupalcode.org/project/drupal.git/blob/50ae3296382669baebee8b9afa...

Solution:
Add cache setting in settings() method.

CommentFileSizeAuthor
#11 interdiff.txt2.54 KBdawehner
#11 vdc-1987236.patch2.54 KBdawehner
#10 vdc-1987236.patch2.54 KBdawehner
#7 vdc-1987236-7.patch2.65 KBdawehner
#7 interdiff.txt614 bytesdawehner
#5 block_cache-1987236-5.patch2.59 KBdawehner
#5 interdiff.txt1.45 KBdawehner
#1 1987236-drupal_8_views_block_cache.patch765 byteskalabro

Comments

kalabro’s picture

kalabro
she/her
commented
Status: Active » Needs review
StatusFileSize
new 1987236-drupal_8_views_block_cache.patch765 bytes

Status: Needs review » Needs work

The last submitted patch, 1987236-drupal_8_views_block_cache.patch, failed testing.

kalabro’s picture

kalabro
she/her
commented
Status: Needs work » Needs review

Failed NodeAccessLanguageTest is not related to this patch.

msonnabaum’s picture

msonnabaum commented
Priority: Normal » Major

Just ran into this as well. This is clearly missing test coverage considering it's been broken this long.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +VDC
StatusFileSize
new interdiff.txt1.45 KB
new block_cache-1987236-5.patch2.59 KB

Note: never forget the VDC tag.

Status: Needs review » Needs work

The last submitted patch, block_cache-1987236-5.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt614 bytes
new vdc-1987236-7.patch2.65 KB

If ?: would just handle the isset case.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

7: vdc-1987236-7.patch queued for re-testing.

damiankloip’s picture

damiankloip commented 
+++ b/core/modules/views/lib/Drupal/views/Plugin/Block/ViewsBlock.php
@@ -49,9 +49,12 @@ public function settings() {
+    $settings['cache'] = isset($this->pluginDefinition['cache']) ? $this->pluginDefinition['cache'] : DRUPAL_NO_CACHE;

That value will be defaulted in BlockBase anyway. I think this would be better as:

if (isset($this->pluginDefinition['cache'])) {
  $settings['cache'] = $this->pluginDefinition['cache'];
}

Otherwise, looks great.

dawehner’s picture

dawehner
Primary language German
commented
Title: Views block cache setting doesn't work » [regression] Views block cache setting doesn't work
Issue summary: View changes
Issue tags: +front-end performance
StatusFileSize
new vdc-1987236.patch2.54 KB

PERFORMANCE/REGRESSION HAMMER TIME!

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new vdc-1987236.patch2.54 KB
new interdiff.txt2.54 KB

That was the wrong patch.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Patch is fine and tests show it is working. Great work @dawehner.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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

yesct’s picture

yesct commented
Issue tags: -front-end performance +frontend performance

changing to use the more common tag, so the less common one can be deleted, so it does not show up in the auto complete and confuse people.