(Found in #2381277: Make Views use render caching and remove Views' own "output caching")

Problem/Motivation

The Views table style plugin does not specify cache contexts for click sorting, which prevents it from working correctly with render caching.

Proposed resolution

Add the related cache context.

Remaining tasks

  • Write a patch
  • Review it

User interface changes

None

API changes

None

CommentFileSizeAuthor
#8 views-cache_click_sorting-2433591-8.patch3.17 KBplach
#8 views-cache_click_sorting-2433591-8.interdiff.txt514 bytesplach
#2 views-cache_click_sorting-2433591-2.patch3.21 KBplach
#2 views-cache_click_sorting-2433591-2-test.patch1.93 KBplach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented
Issue tags: +Needs tests

.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach as a volunteer commented
Status: Active » Needs review
Issue tags: -Needs tests
FileSize
views-cache_click_sorting-2433591-2-test.patch1.93 KB
views-cache_click_sorting-2433591-2.patch3.21 KB

This should fix the issue. Let's see whether there is any test needing adjustments...

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented 
+++ b/core/modules/views/src/Plugin/views/style/Table.php
@@ -428,5 +429,28 @@ public function wizardSubmit(&$form, FormStateInterface $form_state, WizardInter
+    $contexts = [];
+
+    foreach ($this->options['info'] as $field_id => $info) {
+      if (!empty($info['sortable'])) {
+        $contexts[] = 'url.query_args:order';
+        $contexts[] = 'url.query_args:sort';
+        break;
+      }
+    }
+
+    return $contexts;

I think we should array_unique the contexts?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented
Status: Needs review » Needs work

Given that there might be multiple ones.

Maybe it could be also pretty helpful to add a test to admin/content, given that this is also some form of view.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented
Status: Needs work » Reviewed & tested by the community

Nevermind, I haven't seen the break.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: views-cache_click_sorting-2433591-2.patch, failed testing.

The last submitted patch, 2: views-cache_click_sorting-2433591-2-test.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach as a volunteer commented
Status: Needs work » Needs review
FileSize
views-cache_click_sorting-2433591-8.interdiff.txt514 bytes
views-cache_click_sorting-2433591-8.patch3.17 KB

Still too many branches ;)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented
Status: Needs review » Reviewed & tested by the community

Ha, yeah that fix makes totally sense.

catch’s picture

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

Committed/pushed to 8.0.x, thanks!

  • catch committed 99088f6 on 8.0.x 
    Issue #2489966 by plach: The Views table style plugin does not specify...

Status: Fixed » Closed (fixed)

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