NULL contextual argument should be skipped during views URL generation

Created on 15 June 2024, 7 months ago

Problem/Motivation

I used the NULL contextual argument to skip the default URL argument value for rewriting it later with the custom argument_default plugin but I faced an issue with URL generation. My view except contextual filters also have exposed filters, and the problem is that exposed form action was wrong.

I investigated this issue deeper and found that this problem comes from \Drupal\views\Plugin\views\display\PathPluginBase::getRoute. When we generate routes for views we include all contextual arguments into the view display URL path.

Steps to reproduce

I have created dummy module that provides node type and view with exposed and contextual filters for reproducing this issue. So, steps to reproduce:

  1. Install the module (this module will generate on install 20 nodes needed for reproducing the problem)
  2. Go to the /views-null-argument-issue/{NodeID} (use any Node ID of views_null_argument_issue node type for NodeID) and check that
    • the view displays 10 items (result summary above of the form)
    • all items have the same type in the table
  3. Submit the exposed form with any filter value
  4. Check the following
    • the URL was changed to like this /views-null-argument-issue/{NodeID}/exception-value?...
    • the view displays 20 items (result summary above of the form)
    • there are type 1 and type 2 items

Proposed resolution

I think the NULL argument should be excluded from route params in the \Drupal\views\Plugin\views\display\PathPluginBase::getRoute to fix this issue.

We can exclude it by creating a new SkipFromRouteParamsInterface interface, and use this interface for filtering all arguments in the \Drupal\views\Plugin\views\display\PathPluginBase::getRoute to skip arguments that implement it.

Another way of fixing this is to exclude the NULL argument from the route params by configurations. We can add a new checkbox to the NULL argument configuration form that will be responsible for skipping it from route parameters.

In the end, both of these ideas should filter $argument_ids to exclude the NULL argument (and other arguments that implement either interface or configuration) in the \Drupal\views\Plugin\views\display\PathPluginBase::getRoute

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

10.3 ✨

Component
ViewsΒ  β†’

Last updated 29 minutes ago

Created by

πŸ‡ΊπŸ‡¦Ukraine chizh273

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @chizh273
  • Issue was unassigned.
  • Status changed to Needs review 7 months ago
  • πŸ‡ΊπŸ‡¦Ukraine chizh273

    I have created MR with SkipFromRouteParamsInterface interface implementation. Could someone review it, pls?

  • Pipeline finished with Failed
    7 months ago
    Total: 634s
    #199821
  • Pipeline finished with Canceled
    7 months ago
    Total: 556s
    #200232
  • Pipeline finished with Canceled
    7 months ago
    Total: 300s
    #200240
  • Pipeline finished with Failed
    7 months ago
    Total: 584s
    #200243
  • Status changed to Needs work 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Thank you for reporting

    MR should point to 11.x as the current development branch

    Also tagged for tests to show the issue.

  • Pipeline finished with Failed
    7 months ago
    Total: 556s
    #201676
  • Pipeline finished with Failed
    7 months ago
    Total: 660s
    #201677
  • πŸ‡ΊπŸ‡¦Ukraine chizh273

    I have updated MR to point to 11.x

    Also, MR works fine for Drupal 10.3.x. and so in addition, I have added a patch version of this MR for Drupal 10.2.x.

  • Pipeline finished with Failed
    7 months ago
    Total: 866s
    #201796
  • Pipeline finished with Failed
    7 months ago
    Total: 924s
    #201795
  • Assigned to chizh273
  • Pipeline finished with Failed
    3 months ago
    Total: 248s
    #296078
  • Pipeline finished with Failed
    3 months ago
    Total: 136s
    #296122
  • Pipeline finished with Failed
    3 months ago
    Total: 2017s
    #296126
  • Pipeline finished with Success
    3 months ago
    Total: 591s
    #296604
  • πŸ‡ΊπŸ‡¦Ukraine chizh273

    I have updated the logic of getting arguments for checking and, also, updated the PathPluginBaseTest test to cover this change.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Removing test case because test-only feature is showing coverage https://git.drupalcode.org/issue/drupal-3454813/-/jobs/2901632

    Will probably need a CR for how to use new interface.

    Also proposed solution mentions 2 approaches but not which was taken.

Production build 0.71.5 2024