Problem/Motivation

Example use case:

Override node/{node} with a page and then display the node through the entity view block. If you enable caching, then every node page will render the same node, because the block has the same cache key and contexts.

Proposed resolution

#2375695: Condition plugins should provide cache contexts AND cacheability metadata needs to be exposed almost solves this for us. We just need to make sure that the custom context providers provide the cacheability metadata for this, specifically the current user (user) and route-based contexts (route).

Extend the existing tests for overriding the node page for this scenario by enabling caching for the placed blocks.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Issue summary: View changes
Wim Leers’s picture

I just opened #2541344: BlockBase subclasses should merge their cache tags/contexts with the parent's (BlockBase's), which is a related issue.

It's still not 100% clear to me what the problem here is, other than the problem that #2541344 already points out. Is it context providers that aren't returning the proper cacheability metadata?

Wim Leers’s picture

Title: Add cacheability metadata to context providers so that blocks and access can be cached accordingly. » Add cacheability metadata to Page Manager's context providers so that blocks and access can be cached accordingly.
giancarlosotelo’s picture

Fixed and a test only patch provided.

The last submitted patch, 4: add_cacheability_metadata-2540772-TEST_ONLY.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: add_cacheability_metadata-2540772-3.patch, failed testing.

giancarlosotelo’s picture

Status: Needs work » Needs review
FileSize
3.1 KB
722 bytes

Fixed that test.

dsnopek’s picture

I just wanted to add a note that there's plans in the works to switch from Page Manager's current context providers to a "context stack" from CTools:

#2511568: Create "context stack" service where available contexts can be registered

However, if this issue gets done first, we'll just port the approach here into CTools. :-)

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, test makes the problem obvious enough :)

Comment could be a bit clearer, but not exactly sure what.

@dsnopet: Sure, but that issue hasn't moved since it was created, this is a serious bug.

And core now actually already provides the a global context provider system, but that doesn't completely fulfill the needs of page manager (context specific contexts, aka based on the page you're looking at.. you can't just globally register that).

dsnopek’s picture

@dsnopet: Sure, but that issue hasn't moved since it was created, this is a serious bug.

Yes, I agree! I wasn't trying to delay work on this, just post the note so that all the things are linked. I also didn't know how close this was to being finished, and based on your RTBC, it sounds like it's very close!

I'm hoping to at least have a proof-of-concept started on the CTools "context stack" next week at MWDS. :-)

tim.plunkett’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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