Notice: Undefined index: value in Drupal\views\Plugin\views\filter\NumericFilter->acceptExposedInput()

Thank you, the patch in #4 worked for me.

Don't forget to mark the issue as «needs review» to process tests.

Add an exposed grouped filter on Content Type A's ER field to Content Type B

Are you using the patch in #2369119: Fatal error when trying to save a View with grouped filters using other than string values to do this?

It feels like this is fixing a symptom, not the actual bug. Why is 'value' not set, that is the part I don't get.

We would also need a test for this.

@lendude, Yes I also applied the patch from #2369119.

I'm getting this bug. I have an exposed filter for a taxonomy which can accept multiple values and is optional. The $input array has this:

'my_field' => array(0)

As such, the following code sees that $value is already an array, and $value ends up as array(0):

if (!empty($this->options['expose']['identifier'])) {
  $value = &$input[$this->options['expose']['identifier']];
  if (!is_array($value)) {
    $value = [
      'value' => $value,
    ];
  }
}

which means testing $value['value'] gives a notice:

if ($value['value'] === '') {
  return FALSE;
}

I've uploaded my suggested fix, which I think makes more sense than the previous patch, because it ensures the value is in the format expected.

I found my patch above breaks for filters using "between", because instead of 'value' they use 'min' and 'max'.

Best to use #4.

I agree with Lendude though, it seems like something is not right here. Perhaps someone with more knowledge of Views in Drupal 8 could investigate.

Adding a check for min and max values

Hi,
I have also faced this issue. Thanks for the patch and some coding standard has been applied.

The last patch work in my case.

I'm getting this when using a grouped SearchApiString field (which extends SearchApiNumeric which extends NumericFilter).

NumericFilter::acceptExposedInput is called twice per page load, once on form submission and once when rerendering the view. During the form submission call, I can see that $value is an array that is the same length as the number of grouped filters (in my case 2). The keys of the array are 0 and 1 and the values for those keys are either the integer 0 (representing the filter not being applied), or if the filter has been submitted a string value for the key. E.g [0 => "0", 1 => 0] for the first filter being applied.

This seems to get translated at render time as then $value will be an array with the key "value" that equals the actual value that the grouped filter has applied. In my case "Full time" or "Part time".

I've attached the sample of my views field config.

Patches attached didn't work for me on a grouped numeric field using the search API.

Attaching a modified patch which did work although not sure it covers all implications.

Thanks
Tom

How is patch #22 better than #14? Afaict, they have the same feature, but #14 has more.

Re-roll of #14 that applies to 8.7.x

I was still receiving the Notice: Undefined index: value in Drupal\views\Plugin\views\filter\NumericFilter->acceptExposedInput() (line 399 of core/modules/views/src/Plugin/views/filter/NumericFilter.php).
Just required one more !isset check on case 1 of NumericFilter

Still needs tests, still needs proof that this isn't just masking the underlying problem.

Patch in #26 is working for us.

Another +1 for Patch #26.

8.8.1 patched with #26
Notice Error gone, but the grouped filter doesn't work anymore. All options returns same results.

After applying the #26 patch to D8.8, I too am seeing the same thing that @ktch_my reported - all options, same results - which is to say, the results aren't filtered at all.

I think @Lendude's observation is spot-on. The patch is masking the underlying problem. The changes in patch #26 will make the method(s) return FALSE instead of getting the error - and FALSE indicates no filter was selected.

I'm trying to debug it, but if anyone has time before I find the problem, it might be worthwhile to see what changed in those two classes between 8.6 and 8.8. That might point us in the right direction.

I've identified how the patch "masked" the underlying problem.

This code, just before the patched code in acceptExposedInput() in /core/modules/views/src/Plugin/views/filter/NumericFilter.php, never sets $value['value'] because it is using the wrong "identifier".

    // rewrite the input value so that it's in the correct format so that
    // the parent gets the right data.
    if (!empty($this->options['expose']['identifier'])) {
      $value = &$input[$this->options['expose']['identifier']];
      if (!is_array($value)) {
        $value = [
          'value' => $value,
        ];
      }
    }

So, with no $value['value'] defined, the patched code returns FALSE - i.e. no filter selected.

The problem seems to be upstream from this function (method). The identifier it is getting is the original default identifier - which is the full field name. We have overridden all identifiers in all our views. In this case, we used "whse_in" - but the above code is using "field_warehouse_in_date_value". I verified that by changing the identifier in the view back to the default value and the grouped filter worked (in D8.8).

I will keep looking to see if I can find where the actual issue is, but if anyone else finds it before I do, please post back here. At least now we have a work-around.

Adding a patch that contains everything from #26 and a fix for grouped filters. I just updated acceptExposedInput in NumericFilter to use the same group logic as the FilterPluginBase.

Patch #33 work for me on 8.8.1.

Patch #33 on 8.8.1 works for me.

Patch #33 on 8.8.2 works for me.

Adding the related issue that was recently backported to 8.8.x.

Reroll after #2824935: Fix Squiz.ControlStructures.SwitchDeclaration coding standard

This patch is compatible with 9.0.0

Back to NW for the test

Thank you @CodigoVision, the patch #33 works nicely with 8.8.8.

Thanks all. #38 seems to be working Drupal 9.0.2 & PHP 7.3

#38 works for me with D8.9. Thanks everyone!

Confirm that the patch is working on 8.9.5
Great job!

Fails to apply to 8.8.4

For me, #38 mostly worked for me on 8.9.7. I also needed to apply the same logic on lines 251 and 257 of NumericFilter.php.

before: $this->value['value']
after: isset($this->value['value']) ? $this->value['value'] : ""

Re-roll of #38 that applies to 9.2.x

Any chance this could be merged into the latest D8 version? I just updated to D8.9.9 and I still have to re-apply the patch.

Seems like it passed RTBC a while back.

Still needs a test unfortunately

I can't reproduce the bug on 9.2.x. These are steps I made:

1. Install a fresh Drupal site
2. Set $config['system.logging']['error_level'] = 'verbose'; in a settings.php file
3. Go to 'Manage fields' for a 'Basic page' content type at admin/structure/types/manage/page/fields
4. Press 'Add a new field ' button
5. Choose 'Reference >Content' in a 'Add a new field' select box
6. Set 'article' in a 'Label' text field
7. Press 'Save and continue' button
8. Set 'Limited: 1' in a 'Allowed number of values'
9. Press 'Save field settings' button
10. Check 'Article' in a 'REFERENCE TYPE' section
11. Press 'Save settings' button
12. Add the first 'Article' - set its title to 'Article 1'
13. Add the second 'Article' - set its title to 'Article 2'
14. Add the first 'Basic page' - set its title to 'Page 1'
15. Set 'Article 1' in a field 'article' of 'Page 1'
16. Add the second 'Basic page' - set its title to 'Page 2'
17. Set 'Article 2' in a field 'article' of 'Page 2'
18. Add a new view at admin/structure/views/add
19. Set 'Pages' in a 'View name'
20. Set 'Content' + 'Basic page' in a 'View settings' section
21. Check 'Create a page' in a 'Page settings' section
22. Press 'Save and edit' button
23. Press 'Add' button at 'Filter criteria' section
24. Search for 'field_article'
25. Check 'field_article' and press 'Add and configure filter criteria' button
26. Check 'Expose this filter to visitors, to allow them to change it'
27. Select 'Grouped filters' radio button
28. Set 'field_article_target_id_XXX' in a 'Filter identifier' text field
29. Set 'Article 1' in a 'Label column' and set 1 in a 'Value' column of the first row
30. Set 'Article 2' in a 'Label column' and set 2 in a 'Value' column of the second row
31. Press 'Apply' button
32. The messages 'The view Pages has been saved' and 'You have unsaved changes' are displayed.
33. Press 'Save' button
34. The message 'The view Pages has been saved.' is displayed
35. Go to a view page at the /pages address
36. There are a list with four elements: "Page 2", "Page 1", "Article 2", "Article 1"
37. Choose 'Article 1' option in a 'article (field_article)' select box
38. Press 'Apply'
39. 'Page 1' is displayed as expected
40. There is neither error nor notice

Did I miss something?

@kndr, perhaps check the mods in the patch for this issue to see if they have been done in 9.2.x-dev, but not yet merged into 8.9.x-dev.

@ExTexan I can't reproduce the error on a fresh installation 8.9.12-dev too. I followed the steps I mentioned at #51 with no effect. I still miss something in my steps or the bug doesn't exist any longer.

Status report:

View settings:

Filter settings:

Page view at /pages:

Page view at /pages?field_article_target_id_XXX=1:

I'm on 8.9.13 and confirm this problem still happens. In this case it is happening with the report view provided by the Linkchecker module (1.0.0-alpha1)

I've re-rolled my patch to include the changes in #38 and my comments in #47. Applies to 8.9.15.

I am not able to reproduce the warning described in the issue,
followed same steps as @kndr did, in the #15 comment

Hi,
I just got this issue with Drupal 9.2, Grouped filters for a date field with BEF. The thing which breaks it for me is when I change the default `group-info-identifier` (Filter Identifier) value which is something like `field_date` to something else.
The patch in #48 fixed it for me.

I reproduced this bug by using #59 steps.

The reason for this bug is that we have two different identifiers for exposed and grouped filters. (If a filter is exposed and grouped, handler uses `$this->options['group_info']['identifier']` as identifier, if the filter is just exposed but isn't grouped `$this->options['exposed']['identifier']`);
The `NumericFilter` handler ignores this case, so I just fixed it.
Another question is why we need two identifiers for grouped and exposed handlers.

I can confirm #61 works for me. Thanks for the patch.

Getting similar error in Views: Drupal 8.9.14

Notice: Undefined index: empty in Drupal\views\Plugin\views\filter\NumericFilter->adminSummary() (line 390 of core/modules/views/src/Plugin/views/filter/NumericFilter.php).
Drupal\views\Plugin\views\filter\NumericFilter->adminSummary() (Line: 1100)

Solution is working for me: (line 390 of core/modules/views/src/Plugin/views/filter/NumericFilter.php).
Remove or comment below line and add new line
---//$output = $options[$this->operator];

Add this line
+++$output = empty($options[$this->operator])?'':$options[$this->operator];

On 9.2.8, I was getting the following error:

`Notice: Undefined index: min in Drupal\views\Plugin\views\filter\Date->acceptExposedInput() (line 160 of /var/www/web/core/modules/views/src/Plugin/views/filter/Date.php)`

Patch at #60 did not work for me.
Patch at #48 did work.

I have a grouped filter (uses "in between") that gave me this error, on Drupal 9.2.8 - patch #48 worked for me.

I have the same behaviour as #66. The input array only holds an empty array for the key in question. Not sure why the info is missing though...
#48 works for me too.

Was getting `Notice: Undefined index: value in Drupal\views\Plugin\views\filter\NumericFilter->acceptExposedInput()` on line 428 of NumericFilter.php. #48 fixed it for me as well.

I am applying patch #48 for 9.2.9 but still it is not working for me. The reason I think is that I set my grouped (and exposed) filter as multiple. When selected as single select, it works fine for me.

PR created from #60

I wanted to take care of tests, but I'm not able to reproduce this issue following the steps in the issue summary.
Can anyone update the steps, please? I believe I'm missing something minor, so please share as many details as you can.

Please also include a full notice report, so we can see which line is affected.
Thanks!

Same issue here, patch #56 (https://www.drupal.org/project/drupal/issues/2825860#comment-14096179) fixes the issue, the patches after don't.

Was experiencing a similar issue to https://www.drupal.org/project/drupal/issues/3105759 .
Patch #48 worked for me on Drupal 9.3.3

I was experiencing a similar issue on Drupal 9.3.5, Patch #48, and (#53)
These two patches worked for me.

Since PHP 8 this now results in a fatal TypeError. I've managed to replicate this in a vanilla installation and have updated the issue summary with steps to reproduce.

I've also attached a config export of the view that results in the fatal error.

Here's a failing test. It could use some work and could probably be a Kernel test, but it should at least demonstrate the issue.

@mstrelan thanks, this is great! It helped me to discover that the catch is in using another identifier for the grouped filter.
I agree with the proposed fix in the merge request, so I'm attaching a patch with the MR contents, so we can run tests for multiple branches.

Speaking of tests, it might be confusing but we have multiple test classes for Date filter, that's why I opened: #3252479: Combine views date filter test classes.
We also already have a test for exposed grouped date filters, so I just needed to edit a couple of lines to get the same failure. You can check a test-only patch attached here.

I'm also changing the target version to 10.0.x, but we'll try running tests on all active versions.

The patch was failing because of some phpstan issues in the main branches.

The fix looks great and good to see test coverage. But #82 is modifying existing test coverage from a pass to a fail, so are we losing test coverage there or was that testing something in the wrong way to give a false positive?

Before that, we were testing where the "exposed" identifier matches the "group" identifier. Those identifiers are stored as different keys and might be different, which is also often confusing.

Anyway, I think it won't hurt to test both scenarios (default identifier + modified), so I went ahead and expanded the test a bit.

@Matroskeen nice, thanks for the explanation and the extended test coverage, looks great now.

Not a fan of hacking that functional test since it can't really test all the options without being super expensive. Here's an option with unit tests.

Writing the test exposed that the current patch can trigger an "PHP Warning: Undefined variable $value" with some (invalid) configurations.

Updated the merge request with the changes because that's where I was working.

@neclimdul, if you would look at Drupal\views\ViewExecutable::_build($key), you'll see that FilterPluginBase::acceptExposedInput() is called after FilterPluginBase::convertExposedInput(), FilterPluginBase::storeGroupInput() and even FilterPluginBase::groupMultipleExposedInput() if multiple selection is enabled.

I think this order is important. At least, FilterPluginBase::convertExposedInput() modifies group_info plugin property, which is later used in FilterPluginBase::acceptExposedInput().

Furthermore, for some plugins like \Drupal\taxonomy\Plugin\views\filter\TaxonomyIndexTid, you'll also need to call ::validateExposed() because it sets $this->validated_exposed_input property, which is expected by \Drupal\taxonomy\Plugin\views\filter\TaxonomyIndexTid::acceptExposedInput().

I love the idea to have unit tests for things like that, but those Views filters are complicated...

So, first, sorry I didn't get this posted before you started work on your tests and this got RTBC'd. I had it mostly written Wednesday last week but I got sick and only just got back to it and was like "Woah! There's a bug, this isn't RTBC!" I'd actually planned on filling out the unit test a bit more before posting it if possible but wanted to get it into the discussion quickly since y'all where moving fast :)

As for the review, I'll agree these methods are super complicated and a mess. There's no clear interface so the boundaries of what should happen and what sort of per-conditions a method has are not clear. Also, the array of values as an interface makes things complicated. Its a hard thing to write tests for but honestly that just means the CRAP score is probably pretty high and its even _more_ important to test each unit individually.

Looking at the code you mentioned, I don't think it changes the approach to testing this.

• It looks like the functions you mention also fails to handle the tested options and would also emit warnings. Also, if I'm reading them correctly they pass through for that case so if they didn't, they'd still trigger the exact code being tested in the unit test.
• Those methods are inherited so more complexity. We're inheriting unrelated bug fixes to a base class that could break expected pre-conditions. More reason to not trust their behavior and test an isolated unit.
• Similarly, even if those methods fixed it right now, the _point_ of unit testing it is that a bug fix to those methods could have the side effect of passing a different set of values to this method. This method has to be bug free by itself with given inputs. The provided test tries to do that with "reasonable" values still relying on the default processing in init to set a baseline.
• Its a plugin and the point is to be unit testable. And the point of unit testing is to test the smallest testable code. So if possible a single test should be testing the inputs/outputs of a single method on its own.

Additional note, its not one or the other testing. Your functional test might still be useful in a different way since it's troubling that there doesn't seem to be a test that the grouped numeric filter actually interacts with the exposed form correctly. That's a pretty important _function_ to test as a whole.

Thanks for the explanation! I interpreted the suggested unit test as a replacement for the functional testing, so I was skeptical. We can definitely keep both, but the question is: should we add both tests in the same issue? I would be happy to work on a new META issue to add unit tests for other plugins and potentially refactor the plugins (there is a big room for improvement there). Looking forward to your suggestions.

Agree with #93, it would be great to spin the unit tests into a separate issue and churn there.
Back to RTBC for #86

no, there's a bug in that patch. Also, I think the suggestion was to add a meta for _other_ plugins. The suggestion was that maybe we add both and I think that's a good approach.

Ok, it's NW then because the MR doesn't contain any of the tests from #86.

Here is a patch that combines the MR and the patch in #86.

Making some minor changes to the latest patch:

1. Removed helper getView method - we use it in one place and can always add back where there is a need (I think adding new method is easier than moving existing ones);
2. Used getStringTranslationStub() method to inject the translation plugin;
3. Added comments, return type hints, and other small changes;

+++ b/core/modules/views/tests/src/Unit/Plugin/views/filter/NumericFilterTest.php
@@ -0,0 +1,117 @@
+        // Missing value is ok?
+        TRUE,

Really minor knit, but we don't want to leave a comment like this, that is just going to confuse in the future. So we are either ok with this and then the comment should say that, or we are not ok with this and we need to open a follow up to address this and link to that.
I think we are ok with this?

+RTBC
#99 works fine with D9.4.1

#99 works fine with D9.4.2 and PHP8.1.8

@lendude, yeah I think we are okay with that (I don't see a need to disagree with FilterPluginBase::acceptExposedInput()).
I just removed that comment, so there is no interdiff attached.

+RTBC #99

Error still thrown with #99 and #103. Problem is $input assumes the value key exists if the array is empty.

-      if (!is_array($value)) {
+      if (!is_array($value) || !$value) {

If the grouped filter has "Allow multiple selections" enabled the $value = &$input[$this->options[$key]['identifier']]; is either an empty array or an array of the grouping IDs. Last patch only resolves the error in this scenario when no filter is selected. Still occurs the moment one or more grouped filters are selected.

Using Drupal 10.0.x & php 8.1 I followed the steps mentioned below which are included in the summary:

1. Install Drupal standard profile
2. Edit the default Content view at /admin/structure/views/view/content
3. Add a "Changed" filter
4. Select "Expose this filter to visitors, to allow them to change it"
5. Select "Grouped filters"
6. Change the "Filter identifier" to "date"
7. Set the Grouping 1 label to "Last week", operator to "Is greater than", value type to "An offset from ..." and value "-7 days"
8. Press Apply and then Save
9. Navigate to /admin/content
10. Set the "Changed" filter to "Last week" and press "Filter".

But i don't see any warning / notice as mentioned.

I'm confused, we had a failing test that reproduces the issue and a fix for it as well. If there is some other issue that needs steps to reproduce then let's log a separate issue for it.

I assumed this issue covered any filter using the numeric handler. #99 and #103 indeed fix the error when grouping dates. Other field types using the numeric handler may or may not be working as expected with those patches.

In my particular use case, the exposed filter is a state machine (https://www.drupal.org/project/state_machine) field type with several grouped filters using the "Is equal to" operator. Optional and allow multiple selections is checked. With the patch in #99 or #103 applied, an identical PHP notice is thrown both without and with exposed input applied.

Anyways, I won't hijack the original bug smash. Moving back to the needs review status.

Apologies @robphillips I missed your comments above. Perhaps this should all be fixed together after all.

Re #107:
I verified the steps again - it's reproducible. I checked your screenshot and it seems that #6 step is missing - "changed" Filter identifier should be changed to something else, for example, "date". I'm attaching yml file with the configured view. You can import it to your website and check /admin/content page.

Re #109-110:
This issue is targeting a case when the numeric (or any child filter) is exposed, grouped, and has a non-default "Filter identifier" value. @robphillips your case also requires "Allow multiple selections" to be selected, which is already a bit different. We're very close to the RTBC/commit here, so expanding the scope will require changes to the automated tests and another round of reviews. I suggest the following plan:
1) Restore the last patch to #103 version (doing it here);
2) Create a follow-up to target an issue reported by @robphillips;
3) If it depends on this one, you can add [PP-1] at the beginning of the title and mark it postponed;

(now really attaching a yml file for the test)

I with #111 agree on keeping the scope as it is. Please open a follow-up issue for the other scenario that might still be broken.

Assuming this comes back green, moving to RTBC.

+++ b/core/modules/views/src/Plugin/views/filter/NumericFilter.php
@@ -404,16 +404,21 @@ public function acceptExposedInput($input) {
-    if (!empty($this->options['expose']['identifier'])) {
-      $value = &$input[$this->options['expose']['identifier']];
+    $key = $this->isAGroup() ? 'group_info' : 'expose';
+    if (!empty($this->options[$key]['identifier'])) {
+      $value = &$input[$this->options[$key]['identifier']];
       if (!is_array($value)) {
         $value = [
           'value' => $value,
         ];
       }
     }
+    else {
+      // Invalid identifier configuration. Value can't be resolved.
+      return FALSE;
+    }

Given we're doing an early return here I think we should change the method to be....

    // Rewrite the input value so that it's in the correct format so that
    // the parent gets the right data.
    $key = $this->isAGroup() ? 'group_info' : 'expose';
    if (empty($this->options[$key]['identifier'])) {
      // Invalid identifier configuration. Value can't be resolved.
      return FALSE;
    }
    $value = &$input[$this->options[$key]['identifier']];
    if (!is_array($value)) {
      $value = [
        'value' => $value,
      ];
    }

Also later on in the code we do

if (empty($this->options['expose']['required'])) {

Should this also be changed to if (empty($this->options[$key]['required'])) {?

@alexpott,
Made changes as per comment #114.
Needs review.

@Ratan Priya, I believe those were not the exact changes requested.

see interdiff and new patch.

Thanks for rolling the new patch.

+++ b/core/modules/views/src/Plugin/views/filter/NumericFilter.php
@@ -407,22 +407,20 @@ public function acceptExposedInput($input) {
-    if (empty($this->options['expose']['required'])) {
+    if (empty($this->options[$key]['required'])) {

I don't know if this change is correct... It'd be great to get input from a view maintainer... pinging @Lendude!

#115 failed on Drupal\Tests\views\Kernel\Handler\FilterNumericTest
#116 might also fail on the same one due to the $key change, hard to say

***EDIT*** #116 passed Drupal\Tests\views\Kernel\Handler\FilterNumericTest 18 passes

+++ b/core/modules/views/src/Plugin/views/filter/NumericFilter.php
@@ -404,20 +404,23 @@ public function acceptExposedInput($input) {
+    if (empty($this->options[$key]['required'])) {

@alexpott well spotted, that change is indeed not correct and needs to be reverted.

'expose' uses the 'required' key while the 'group_info' uses the 'optional' key

So if I'm seeing this correctly, currently this logic always triggers in grouped filters since 'expose''required' will always be empty in that case. I suggest we open a follow up for that? The logic added here doesn't make that any more broken than it already is right?

Also bumping to major since, as @mstrelan pointed out in #78, this leads to a fatal error on PHP8

reroll

here's the interdiff

patch 116 is the good one, ignore 121

Patch 116 is clean on 9.3.x, 9.4.x, 9.5.x, 10.x and 10.1.x

patch number 116 passes on 10.1.x, 10.x, 9.5.x, 9.4.x and 9.3.x on PHP 8.1

Patch 116 didn't fix the issue I have:
TypeError: Cannot access offset of type string on string in Drupal\views\Plugin\views\filter\NumericFilter->valueForm() (line 250 of core/modules/views/src/Plugin/views/filter/NumericFilter.php).
Drupal\views\Plugin\views\filter\NumericFilter->valueForm(Array, Object) (Line: 939)
Drupal\views\Plugin\views\filter\FilterPluginBase->buildExposedForm(Array, Object) (Line: 111)
Drupal\views\Form\ViewsExposedForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 531)
Drupal\Core\Form\FormBuilder->retrieveForm('views_exposed_form', Object) (Line: 278)
Drupal\Core\Form\FormBuilder->buildForm('\Drupal\views\Form\ViewsExposedForm', Object) (Line: 134)
Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase->renderExposedForm() (Line: 1240)
Drupal\views\ViewExecutable->build() (Line: 392)
Drupal\views\Plugin\views\display\PathPluginBase->execute() (Line: 196)
Drupal\views\Plugin\views\display\Page->execute() (Line: 1657)
Drupal\views\ViewExecutable->executeDisplay('page_1', Array) (Line: 81)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func_array(Array, Array) (Line: 101)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 772)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 363)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 241)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 564)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 242)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 132)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 164)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Adding a new patch as #116 does not address the feedback in #120, and the patch #121 reverts the wrong line.

I is not clear if #128 is related to this patch as I was unable to reproduce this error - but given the comment includes with "didn't fix the issue I have" implies this issue existed before the patch and so I think it is unrelated. @DiDebru can you please confirm?

Looks like we can remove the expected errors from the baseline too.

Queued tests for 129 on 9.5 as this patch will not apply to 9.5 - and will check 10.1 / 10.0 on this patch

I've made some investigation and I like to propose other solution.

Why it happens?

The warning is triggered in NumericFilter::acceptExposedInput() and it called twice. With grouped filter first time there is just an integer (e.g. 1) in input that triggers the warning, the second time input gets expected input as array with keys ['value', 'type', 'min', 'max']. It looks strange as it is expected to have the same input on each call.

First call in ViewExposedForm::submitForm() that triggers warning:

    // Form input keys that will not be included in $view->exposed_raw_data.
    $exclude = ['submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', 'reset'];
    $values = $form_state->getValues();
    foreach (['field', 'filter'] as $type) {
      /** @var \Drupal\views\Plugin\views\ViewsHandlerInterface[] $handlers */
      $handlers = &$form_state->get('view')->$type;
      foreach ($handlers as $key => $info) {
        if ($handlers[$key]->acceptExposedInput($values)) {
          $handlers[$key]->submitExposed($form, $form_state);
        }
        else {
          // The input from the form did not validate, exclude it from the
          // stored raw data.
          $exclude[] = $key;
        }
      }
    }

Second time in ViewExecutable:_build() without warning

    $handlers = &$this->$key;
    foreach ($handlers as $id => $data) {

      if (!empty($handlers[$id]) && is_object($handlers[$id])) {
        $multiple_exposed_input = [0 => NULL];
        if ($handlers[$id]->multipleExposedInput()) {
          $multiple_exposed_input = $handlers[$id]->groupMultipleExposedInput($this->exposed_data);
        }
        foreach ($multiple_exposed_input as $group_id) {
          // Give this handler access to the exposed filter input.
          if (!empty($this->exposed_data)) {
            if ($handlers[$id]->isAGroup()) {
              $converted = $handlers[$id]->convertExposedInput($this->exposed_data, $group_id);
              $handlers[$id]->storeGroupInput($this->exposed_data, $converted);
              if (!$converted) {
                continue;
              }
            }
            $rc = $handlers[$id]->acceptExposedInput($this->exposed_data);
            $handlers[$id]->storeExposedInput($this->exposed_data, $rc);
            if (!$rc) {
              continue;
            }
          }
          $handlers[$id]->setRelationship();
          $handlers[$id]->query($this->display_handler->useGroupBy());
        }
      }
    }

So for me there are two questions:

• Why NumericFilter::acceptExposedInput() is called twice?
• Why NumericFilter::convertExposedInput() that converts group input to expected format is missed in first case?(By the way I've tried to add it to first case, but it breaks the view execution)

One more strange thing found

Also I pay attention to broken $exclude variable, it declared twice in ViewExposedForm::submitForm() and isn't useful in first case.

    // Form input keys that will not be included in $view->exposed_raw_data.
    $exclude = ['submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', 'reset'];
    foreach (['field', 'filter'] as $type) {
          // ... Some code miss ...
          $exclude[] = $key;
    }

    // Rewrites upper results here.
    $exclude = ['submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', 'reset'];
    foreach ($values as $key => $value) {
      // Used last assignment only.
      if (!empty($key) && !in_array($key, $exclude)) {
        // ... Some code miss ...
      }
    }

Changes history

I've go to look for this changes in history and found that all this things above were added in this issue #2651102 (patch). The patch adds checkbox processing to form values and inside NumericFilter::acceptExposedInput() and non-working first $exclude assignment. The code before the patch:

    foreach (array('field', 'filter') as $type) {
      /** @var \Drupal\views\Plugin\views\ViewsHandlerInterface[] $handlers */
      $handlers = &$form_state->get('view')->$type;
      foreach ($handlers as $key => $info) {
        $handlers[$key]->submitExposed($form, $form_state);
      }
    }

Looks like NumericFilter::acceptExposedInput() was used to process checkboxes before call NumericFilter::submitExposed(), but:

• NumericFilter doesn't use ::submitExposed() method, it inherits empty functionality
• No core views filter has the method
• No Search API views filter has the method
• If custom module implements ::submitExposed(), it could care about checkbox processing inside
• $exclude improvements aren't working

Proposal

I propose to revert a part of issue #2651102 described above, that causes warning and looks like doesn't have effective changes. @catch, @mikeker, welcome to comment!

This 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.

Can confirm this issue and that the patch addresses it.
It will need test coverage though.

FWIW there are tests in the patches up until #131. I haven't really looked at the new approach but perhaps if we had an updated patch with the test from #130 and the fix from #131 we can confirm if this fixes the same issue.

Existing tests changed to handle the notice. One more notice found when group identifier doesn't match to simple exposed one and fixed.

One more small improvement to prevent possible notices if identifier is empty (looks it could happen on wrong config only), but anyway there is no sense to make additional check if parent::acceptExposedInput() returns FALSE.

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

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Fixed CSpell issues, but phpcs warning isn't happens locally and surprises me at all, the variable isn't unused.

FILE: ...l10/core/modules/views/src/Plugin/views/filter/NumericFilter.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
411 | WARNING | Unused variable $value.
----------------------------------------------------------------------

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

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Added Updated patch and fixed CCF #137

@Akram, as far as I understand renaming "_test*" methods to "test*" means that it runs by phpunit engine itself, I afraid with the changes these methods called twice now, by phpunit engine and from ::testDateFilter() directrly.

NW on #141

Took a quick look at #drupalsouth code sprint. Looks like function testDateFilter() can be dropped then.

Took a brief look and seems there are some out of scope changes that were probably introduced in #139

IMHO, I don't support an idea to beatify all the file (not the changed lines only), I think doc-comments should be added in a scope of other issue, but not in a bug. By the example from previous patches I've just add `$value = NULL;` to pass PHPCS, rebased to 11.x branch and no more changed from #137.

P.S. by the way PHPStorm not highlights the variable as unused(

Confirmed the problem in the issue summary following the steps.
Note doesn't matter the identifier name, tried a few different ones but always got the error.

Applying the patch #146 solved the issue and see it has test coverage as well.

Thanks!

it seems that patch actually passes.

back to RTBC

Sometime between #130 and #131 this patch changed dramatically.

Can we elaborate why?

We had subsystem maintainer signoff on the approach in ##13 (see #113 and #120)

The changes to remove all the special keys like form build ID definitely feel wrong to me.

Can we go back to #130 and can those with the new approach elaborate why it isn't suitable?

Where did the unit test go? It was exposing several ways this could be triggered that had nothing to do with what ever is being changed in the exposed form handler I suspect might not being addressed based on a skim of the current patch.

@larowlan, I've made research of the issue and tried to provide explanation in #131. If anyone have some exact questions - you are welcome.

@neclimdul Do you propose just to add the Unit test from #130?

Think the confusion is that in 129-130 the size was about 8KB

Then your post went to around 2KB with an explanation

But then in 139 it jumped up to 13KB

Was anything lost in that jump back down to 5KB, if tests were lost could we add back?

May help to hide all patches but the right one (making sure it matches the issue summary, didn't check). Also quickest solution is turn to an MR

Taking a look at this - aim to restore the test improvements from #130. Will move this to an MR per smustgrave suggestion.

Some MRs from DrupalSouth 2024:

• !7154 is approach and tests from #130 (which had subsystem maintainer signoff in #113 and #120)
• !7145 was tests from #130 + solution approach from #131, I am closing this as tests didn't pass

Removing old patches from display, MR here is approach + tests from #130 updated.

Interdiff is not cooperating with me trying to get an interdiff from 130 to the MR, so I'll summarise as best I can from visual review:

• Change from 130 to core/phpstan-baseline.neon moves to core/.phpstan-baseline.php
• In FilterDateTest.php we pass form options as strings not integers now ($this->getSession()->getPage()->findField($filter_identifier)->selectOption('1'); not ...->selectOption(1);
• declare(strict_types=1); in NumericFilterTest.php

Missed status update, back to NR

Hiding patches for clarity.

Left a comment on the MR that may need to be clarified. If I'm correct that it's the wrong key then surprised nothing broke.

Workaround added to issue description - if step 6 from the issue description is omitted, the bug does not appear.

Reproducible on 11.x, updated repro details in the issue description to use "Authored on" as that makes it easier to verify results on an empty site (create two test nodes, set "Authored on" date back for one node). View config I tested with here.

Test coverage can be seen https://git.drupalcode.org/issue/drupal-2825860/-/jobs/1173292

Following the issue summary believe I am able to see the issue and the MR does address it.

Also appears all feedback for this bug has been addressed so believe this one is good.

Assigning issue credit. Not crediting screenshot confirming bug, or works for me, or doesn't work for me. Trying to credit people who moved the fix and issue forward. If I make a mistake I'm sorry! Thanks.

Committed and pushed ba08bc0dcb to 11.x and b5f62fa4ae to 10.3.x and f78530b4a7 to 10.2.x. Thanks!

Re-roll of patch #48 for branch 10.3.x.

I had the same issues, applied the patch from #175 and it worked correctly. It seems it is still needed even after the previous merge at #170

Agreed. The work here that was committed in 10.2.x, 10.3.x, 11.x is absolutely needed, but i'm still having issues with the actual value.

Inside here:

if (!empty($info[$this->operator]['values'])) {
  switch ($info[$this->operator]['values']) {
    case 1:
      if ($value['value'] === '') {
        return FALSE;
      }
      break;

    case 2:
      if ($value['min'] === '' && $value['max'] === '') {
        return FALSE;
      }
      break;
  }
}

for grouped (checkbox?) input, $value is this:

array (
  1 => '1',
)

which causes the error still as there is no 'value' key to check against for this grouped exposed input value.

Patch 175 which is a re-roll of 48 actually checks to inspect $value to make sure that the expected keys are there which seems to be what's needed in some circumstances?

There is another issue here in relation to this though... where ViewsExposedForm::submitForm() https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/modules/vie...

seems to declare an $exclude variable at the top of the function, which is added to if acceptExposedInput() returns false but then immediately beneath this outside of the loop, it's declared again (with the same initial keys) which overrides any entries that were added by this loop!

So you can get into a weird situation where the handler for the exposed input returns false, but then beneath it can still be processed as it's no longer inside of $exclude... which in my circumstances is actually useful as I want the exposed filter value to be valid... but technically it shouldn't be doing this!?

Maybe someone with some more knowledge around this could provide some insights about the data structure of grouped $value's and how this is expected to work without the additions in patch 175...

Update: Turns out in this instance the indexed field type of this field in SOLR was text, but in reality even though the field was stored as a yes / no value, the intention for purposes of searching is a boolean.
After modifying the field type to boolean in the search api configuration and modifying the indexed value before being indexed to be a boolean true / false instead of string yes / no, FilterPluginBase is used instead of NumericFilter and doesn't have this problem with the grouped output.

Just writing my ramblings here in case it helps anyone in the future.

Re-roll of patch #175 for branch 11.2.4.

Tried to fix merge conflict in 11.x branch. It looks like the 3 changed files need to be reverted? e.g. The Group attribute needs to be used not the Group annotation. I just realised this issue is closed.

@oily the fix has been committed to 11.x

Re-roll of patch #178 for branch 11.3, and also added a line for exposed operator.

Re-roll for 11.3.9

The fix already exists in 11.3, it was committed to 11.x 2 years ago.