Problem/Motivation

The condition returns no cache contexts and no cache tags, so the block access result it takes part in carries nothing saying that the decision depends on which page is being served or on the entity that was inspected.

Drupal's block access handler merges the cacheability of every visibility condition into the access result, in BlockAccessControlHandler::mergeCacheabilityFromConditions(). The default implementation the plugin inherits derives that cacheability from the plugin's context objects, and this plugin declares no contexts: it reads the entity from the route directly. The result is that both methods return an empty array and the access decision looks context free to the render cache.

That is a correctness problem, not a tuning one. A decision computed on one page can be reused on another, so a block can appear on a page that does not hold the selected paragraph type, or stay hidden on one that does. It also means the decision is not invalidated when the entity is edited, so adding or removing the paragraph does not change what visitors see until the caches are cleared by hand.

Steps to reproduce

1. Configure a block with this condition and select a paragraph type.
2. Create the situation where two pages give different answers, one entity holding the paragraph type and one not.
3. Note that the block access result carries neither the route cache context nor the entity cache tags, so nothing tells the render cache that the two pages must be cached separately, or that editing the entity must invalidate the decision.

Proposed resolution

Implement getCacheContexts() to add the route cache context, since the answer depends on which entity the route resolves to.

Implement getCacheTags() to add the cache tags of the entity that was inspected, so editing that entity, which is what adds or removes paragraphs, invalidates the decision. When the route carries no entity there is nothing to depend on and the parent value is returned unchanged.

Both merge with the parent value rather than replacing it, so any cacheability the base class contributes is preserved.

Comments

trebormc created an issue. See original summary.

  • trebormc committed 68a86ddf on 1.0.x
    Issue #3618998 by trebormc: Fix the condition reporting no cacheability.
    
trebormc’s picture

Status: Active » Fixed

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.

Status: Fixed » Closed (fixed)

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