Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If an empty string ("") is passed in as contextual filter argument, it isn't considered a missing argument but is filtered against.
This was raised before in #2815863: Contextual filter values of "" don't trigger "When the filter value is NOT available" against D7 Views module, and originally came up in #1699378-20: Allow tokens in entity reference views selection arguments and was considered a Views (now: Core) bug in #1699378-94: Allow tokens in entity reference views selection arguments.
Steps to reproduce
Example:
$arguments = [
0 => '', // is processed in query
1 => NULL, // is skipped and default argument or default action is used
2 => 'foo' // is processed in query
];
/** @var \Drupal\views\ViewExecutable $view */
$view = Views::getView($target_id);
$view->setArguments($arguments);
$view->preExecute();
$view->execute();
An empty string as argument could be an result of a token replacement like this:
$value = '[node:field_with_no_user_input]';
$token_data = [$entity->getEntityTypeId() => $entity];
$token_service = \Drupal::token();
$arguments[] = $token_service->replace($value, $token_data);
Proposed resolution
I think sanitizing the empty string to no argument given is the expected behavior. We should however think twice making sure there really isn't a usecase for the current behavior resp. no sitebuilder's assumption might be broken.
Remaining tasks
TBD
User interface changes
TBD
API changes
TBD
Data model changes
TBD
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | interdiff_23FAIL-23PASS.txt | 594 bytes | danflanagan8 |
| #23 | 3027640-23.patch | 1.45 KB | danflanagan8 |
| |||
| #23 | 3027640-23-FAIL.patch | 893 bytes | danflanagan8 |
| #22 | empty_string_contextual_filter-3027640-21.patch | 1.76 KB | marc.groth |
| #20 | empty_string_contextual_filter-3027640-20.patch | 1.78 KB | stefan.butura |
Comments
Comment #2
PanchoHere's the patch against 8.6.x-dev.
Note that this surprisingly doesn't affect the similar case where a contextual filter argument is retrieved from a query parameter (say
id) and a fallback value is given (say,all). Even with this patch, this means/admin/testview?id=continues to evaluate to a given argument = "", resulting in aPage not found, while/admin/testviewresults in the expected view using the fallback parameter. This might be a separate bug, however I didn't find the correct place to fix that one.Comment #3
PanchoComment #5
Pancho!emptybreaks a test as it evaluates both "" and "0" as empty. "0" however is a legitimate value.isset && !== ""does the job just as well, while being less aggressive. The tests should be green now.Comment #6
PanchoComment #7
PanchoCommitted in D7 Views now, see #2815863-15: Contextual filter values of "" don't trigger "When the filter value is NOT available".
Comment #10
Kristen PolPatch applies cleanly to 9.1.x.
Comment #11
Kristen PolThanks for the issue and patch.
1) The changes are similar to the Drupal 7 change: https://git.drupalcode.org/project/views/commit/610f570
2) Although the code below is technically correct, IMO it would be easier to read if the logic was like the Drupal 7 code or, at least, there are parentheses added around
isset($this->args[$position]) && $this->args[$position] !== ''.3) Kicking of tests for 9.1.x.
Comment #12
Kristen PolMarking needs work for tests and consideration of feedback in #11.
Comment #13
Adsyy CreditAttribution: Adsyy as a volunteer commentedHi, I apply changes as #11 suggested.
I don't know what is best between if structure or ternary inline operator, I think ternary can be a bit harder to read but in this case it's not.
Hope I helped !
Comment #14
Kristen PolThanks for the update. The change from #13 looks good and addresses the feedback from #11. Moving back to needs work for tests.
Comment #15
Kristen PolTagging for steps to reproduce and manual testing too.
Comment #17
marc.groth CreditAttribution: marc.groth commentedThe patch works really well for when you preview the view in the admin page... However results are still being displayed on the front end. In my case I have a contextual filter against taxonomy vocabularies and have verified that the node itself doesn't have any of these values set (i.e. no terms are associated with the node).
I have checked the query (using hook_views_query_alter()) and it looks like this field value is not present on the front end. Whereas in the preview it is (like this):
"field" => "taxonomy_index.tid"
"value" => null
"operator" => "IS NULL"
I'm not too sure why this would be present on the preview but not where it is actually output as a block. Is there something I've missed? For now I have added my own check for this value and if it is not in the query then I add it to ensure that there are no results; however it would be good if this could happen automatically somehow.
Comment #18
allaprishchepa CreditAttribution: allaprishchepa commentedFaced the same issue.
Actually, it was related to that one: https://www.drupal.org/project/views_data_export/issues/2950819
But it's because of the empty string in the contextual filter argument.
We have a Contextual filter by the Taxonomy term reference field. And default value is set as "Taxonomy term ID from URL". So if the current node has no corresponding term, we have this error.
Symfony\Component\Routing\Exception\InvalidParameterException: Parameter "arg_0" for route "view.content.export_csv" must match "[^/]++" ("" given) to generate a corresponding URL. in Drupal\Core\Routing\UrlGenerator->doGenerate()I modified the patch from #13 and added the same checking for argument returned with getDefaultArgument().
Comment #20
stefan.butura CreditAttribution: stefan.butura at Eau de Web commentedI've noticed that the query_parameter argument default handler still handles empty strings ("") as valid filters. I've attached a patch that fixes this.
Comment #22
marc.groth CreditAttribution: marc.groth commentedThe patch in #20 no longer applies so I have re-rolled it (attached).
Comment #23
danflanagan8Here's a fail test along with the fix from #13.
I don't I think I understand what's going on in #18, but it looks related to this: #3239685: View parameter doesn't exist when editing block layout: Parameter "arg_0" must match "[^/]++"
And with #20, I'm concerned that it's getting out of scope. The D7 issue that we're kind of modeling after does not touch any default argument plugins. That might be better handled in a separate issue.
Comment #24
danflanagan8I just closed #2937759: If computed view argument is empty the default value or action is not used as a duplicate of this issue. It had some nice stuff in the IS that I have used to update the IS here.
Comment #26
danflanagan8The patches in #23 ran as expected. They are ready for review. I'm removing the Needs steps to reproduce tag since I update the IS in #24.
Comment #28
quietone CreditAttribution: quietone at PreviousNext commentedAdding credit as recommended by @danflanagan8 in #bugsmash
Comment #29
Kristen PolThanks for the updated patch. Adding tag due to TBDs in the issue summary.
1. Patch still applies cleanly to 9.4.
2. Test-only patch fails and tests with patch pass.
3. Patch is simple and only addresses the issue in the issue summary.
4. Test has been added.
Remaining tasks:
1. Update issue summary.
2. Manual testing.
Comment #32
smustgrave CreditAttribution: smustgrave at Mobomo commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
For the issue summary update requested in #29