Problem/Motivation

#3427374: taxonomy_tid ViewsArgumentDefault plugin doesn't add cache tags was committed in attempt to fix an issue where when "Load default filter from node page, that's good for related taxonomy blocks" is used the view does not include any cache metadata from the node being viewed, resulting in stale data displayed after updates.

As mentioned in https://www.drupal.org/project/drupal/issues/3427374#comment-15645693 this fix does nothing and the issue persists. Opening this as a new issue given the time that has passed since that was closed.

Cache metadata is collected from the handlers when the view is saved (https://git.drupalcode.org/project/drupal/-/blob/11.2.8/core/modules/vie...) - cache metadata is not collected at runtime from the argument / handler.

#3091671: Validate user has access option in contextual filter does not bubble access cacheability for views page is a similar issue - where cache metadata from the argument validator doesn't bubble up.

Together - both issues suggest we need a better way to apply cache metadata at run time from various views plugins. Or a clarified process to do this properly.

From that related issue @danflanagan8 identified an existing issue #2853592: Cacheability metadata can't be set from within argument default handlers. and test / test plugin in core that implies a supported way to do this is to apply cache data to the view element itself.

Steps to reproduce

Using the standard profile:

  • Create a new view block display of taxonomy terms, add a contextual filter for "Taxonomy term: Term ID"
  • Select "Provide default value" for "When the filter value is NOT available"
  • Select "Taxonomy term ID from URL"
  • Select "Load default filter from node page, that's good for related taxonomy blocks"
  • Place the block for article content type
  • Add an article with tags, observe the view is present
  • Edit the article, observe the view is present with the old data

Proposed resolution

Head in same direction as #3091671: Validate user has access option in contextual filter does not bubble access cacheability for views page - apply the cache metadata to the view element itself when the default value is loaded - unless there is a better way to do this.

Remaining tasks

  • Revert 3427374
  • Add a test using an actual view, test was flawed in 3427374 so we potentially need to verify via a functional test here?
  • Fix cache metadata
  • Review

User interface changes

n/a

Introduced terminology

n/a

API changes

n/a

Data model changes

n/a

Release notes snippet

Issue fork drupal-3560357

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ericgsmith created an issue. See original summary.

ericgsmith’s picture

Status: Active » Needs review

https://git.drupalcode.org/issue/drupal-3560357/-/jobs/7465803#L57 test only change shows the issue.

After adding the test I see there is already an existing TaxonomyDefaultArgumentTest just testing a view title - I guess we should move this view and test into that existing test rather than a new one. Happy to do that, will wait for a feedback on the change and test itself first.

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

This one has been around for a while

I did a rebase and everything is still all green.
Test-only run is still failing as expected https://git.drupalcode.org/issue/drupal-3560357/-/jobs/8138898

Not 100% sure if a CR is needed to remove getCacheTags() from the plugin but change of adding CacheableMetadata makes sense to me.

No additional feedback.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new1.31 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Bot rebellion

alexpott’s picture

Version: main » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 8ebac7b2e26 to main and 6c383e892bb to 11.x and 8df2a7d84e6 to 11.3.x. Thanks!

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.

  • alexpott committed 8df2a7d8 on 11.3.x
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

  • alexpott committed 6c383e89 on 11.x
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

  • alexpott committed 8ebac7b2 on main
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

godotislate made their first commit to this issue’s fork.

godotislate’s picture

Status: Fixed » Needs review

This broke HEAD for SQLite and Postgres tests.

Issue is that there are two taxonomy terms referenced by the node, so the argument default plugin returns 1+2, but the test view was not configured to handle multiple results. Updated the view config in MR 15038 and ran the SQLite and Postrgres tests which
passed:
https://git.drupalcode.org/project/drupal/-/pipelines/764387
https://git.drupalcode.org/project/drupal/-/pipelines/764388

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Let's just commit @godotislate's new MR - nice fix. I'm not a huge fan of the break phrase stuff and we have to wonder how this view works on MySQL when it is doing a where tid = "1+2"

  • catch committed 711dca06 on 11.3.x
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

  • catch committed ce4861f6 on 11.x
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

  • catch committed 73637c4e on main
    fix: #3560357 Taxonomy term ID from URL ViewsArgumentDefault plugin does...

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed the follow-up to main, 11.x and 11.3.x, thanks!

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.