Problem/Motivation

When using a grouped filter in a View that is not 'string' based, saving the view gives an "Error" page with message "InvalidArgumentException: The configuration property display.default.display_options.filters.created.group_info.group_items.1.value.type doesn't exist. in Drupal\Core\Config\Schema\Mapping->get() (line 66 of core/lib/Drupal/Core/Config/Schema/Mapping.php)".

This is all due to the fact that the grouped filter value schema is always a string.

This applies for example to Date, Datetime, numeric and Boolean grouped fields.

Major bug according to https://www.drupal.org/core/issue-priority#major-bug

Trigger a PHP error through the user interface, but only under rare circumstances or affecting only a small percentage of all users, even if there is a workaround.

Other Issues

#2576927: grouped exposed taxonomy filters return invalid when a valid entity is present (not strictly postponed) has a test of the preview, but not a test of save cause of this issue.

#2648950: Use form element of type date instead textfield when selecting a date in an exposed filter is postponed on this issue.
#2049603: Exposed Group Filter assigns incorrect tid to SQL Query is postponed on this issue.

Proposed resolution

Update the handling of the grouped filter type in the code to allow for configuration to set other value types than strings. Then update the views filter Schema to set non-string grouped filter values where needed.

Note: We hope we fix all schema errors in this issue, but there might be still some obscure ones left. We can always add new issues and fix the other ones, but in the meantime solve the problem for people out there.

Steps to reproduce

Create a content view with a filter "Content: Authored On". Expose the filter and select "Grouped filters". Create a group with value type "A date..." and another group with "An offset..." selected, click "Apply".

Go back to edit the filter. Both groups now have value type "A date..." selected.

Saving the view gives an "Error" page with message "InvalidArgumentException: The configuration property display.default.display_options.filters.created.group_info.group_items.1.value.type doesn't exist. in Drupal\Core\Config\Schema\Mapping->get() (line 66 of core/lib/Drupal/Core/Config/Schema/Mapping.php)".

Files: 

Comments

Status: Needs review » Needs work

The last submitted patch, views-date-filter-test.patch, failed testing.

olli’s picture

Status: Needs work » Needs review
FileSize
9.99 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,099 pass(es). View
3.13 KB

This contains schema changes from #2357145: Views can not be saved with a numeric (grouped) filter (not in interdiff).

olli’s picture

FileSize
6.87 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,885 pass(es). View
olli’s picture

Issue tags: +Configuration schema
olli’s picture

FileSize
6.78 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: failed during invocation of run-tests.sh. View
olli’s picture

FileSize
8.45 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,153 pass(es). View
2.76 KB

Extended tests to cover single filter.

The last submitted patch, 5: 2369119-5.patch, failed testing.

olli’s picture

+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -129,6 +129,13 @@ views.filter.group_item.numeric:
+views.filter.group_item.date:
+  type: views_filter_group_item
+  label: 'Group items'
+  mapping:
+    value:
+      type: views.filter_value.date
+

Is it possible to make this dynamic? In #2357145: Views can not be saved with a numeric (grouped) filter we added this:

views.filter.group_item.*:
  type: views_filter_group_item
  label: 'Default'

views.filter.group_item.numeric:
  type: views_filter_group_item
  label: 'Group items'
  mapping:
    value:
      type: views.filter_value.numeric

If not, I think we could alternatively remove views_filter_group_item and move it into views_filter. Maybe it was added due to the confusion early in #2357145: Views can not be saved with a numeric (grouped) filter?

olli’s picture

FileSize
9.77 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,450 pass(es), 10 fail(s), and 1 exception(s). View
2.07 KB

Here's the alternative from #8 that removes views_filter_group_item.

Status: Needs review » Needs work

The last submitted patch, 9: 2369119-9.patch, failed testing.

olli’s picture

Status: Needs work » Needs review
FileSize
11.56 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2369119-11.patch. Unable to apply patch. See the log in the details link for more information. View
2.07 KB

Status: Needs review » Needs work

The last submitted patch, 11: 2369119-11.patch, failed testing.

pjonckiere queued 11: 2369119-11.patch for re-testing.

The last submitted patch, 11: 2369119-11.patch, failed testing.

olli’s picture

Status: Needs work » Needs review
FileSize
12.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,148 pass(es), 0 fail(s), and 2 exception(s). View
1.46 KB

Status: Needs review » Needs work

The last submitted patch, 15: 2369119-15.patch, failed testing.

olli’s picture

Status: Needs work » Needs review
FileSize
12.9 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,157 pass(es). View
506 bytes
olli’s picture

FileSize
12.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,435 pass(es). View

reroll

Patch #6 (also fixes date filter) still applies.

Lendude’s picture

This seems like a duplicate of #2595027: View crash on saving having exposed filter with groupings, different filter, same error. Curious if this fixes that issue too.

Patch didn't apply anymore. Minor reroll.

Status: Needs review » Needs work

The last submitted patch, 19: 2369119-19.patch, failed testing.

Lendude’s picture

Title: Problems with views grouped date filter » Problems with a number of views grouped filters
Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.71 KB
14.62 KB

Small update of the issue summary to make this a bit more general.

Manually tested this for both date and permission filter and manually works for both. The test from #2595027: View crash on saving having exposed filter with groupings still fails though. No idea why. Added the test from that issue to this patch and closed that as a duplicate.

Putting on needs review to fire up the testbot, but this will fail.

claudiu.cristea’s picture

Status: Needs review » Closed (duplicate)

It's the same issue as #2595027: View crash on saving having exposed filter with groupings? Closing this as duplicate.

claudiu.cristea’s picture

Status: Closed (duplicate) » Needs review

I was wrong.

Status: Needs review » Needs work

The last submitted patch, 21: 2369119-21.patch, failed testing.

rakesh.gectcr’s picture

The above patch is working fine! +1 to RTBC, Not understanding why the bot getting failed.

I rolled the same patch with latest pull, Let see is that making any difference.

rakesh.gectcr’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 25: 2369119-22.patch, failed testing.

halefx’s picture

Patch in #25 worked for me as well on 8.0.5

dagmar’s picture

I don't have time to help with this issue, but I did some research and it seems the plugin_id of the testing views loaded on the tests that are failing have some differences when created by the UI.

See:

http://cgit.drupalcode.org/drupal/tree/core/modules/views/tests/modules/...

It says: 'node_type'

But when the filter is created using the UI and saved, the plugin_id for type now says: bundle

I think you should review that part of the code, maybe the views.view.test_exposed_admin_ui.yml needs to be updated. But I'm not sure if this will make the tests pass.

My two cents.

geekygnr’s picture

#25 fixed the issue for me as well in 8.0.5

yongt9412’s picture

We should not have failing tests to allow this to be set as RTBC.

asrob’s picture

Version: 8.0.x-dev » 8.2.x-dev

It worked well with drupal core 8.0.5 but it doesn't work with 8.1.0-rc1. That's why I bumped version to 8.2.x-dev.

claudiu.cristea’s picture

Version: 8.2.x-dev » 8.1.x-dev

I disagree. This is a very ugly bug that should be fixed in a patch release of the current minor. So, 8.1.x.

dagmar’s picture

andypost’s picture

+1 to fix in 8.1, very annoying bug

andypost’s picture

Issue tags: +Needs reroll
checking file core/modules/node/config/optional/views.view.content.yml
checking file core/modules/user/config/optional/views.view.user_admin_people.yml
checking file core/modules/views/config/schema/views.data_types.schema.yml
checking file core/modules/views/config/schema/views.filter.schema.yml
checking file core/modules/views/src/Plugin/views/filter/Date.php
Hunk #1 succeeded at 131 (offset -5 lines).
checking file core/modules/views/src/Plugin/views/filter/FilterPluginBase.php
Hunk #1 succeeded at 1000 (offset -9 lines).
Hunk #2 succeeded at 1261 (offset -10 lines).
Hunk #3 succeeded at 1358 (offset -10 lines).
checking file core/modules/views/src/Tests/Handler/FilterDateTest.php
Hunk #1 succeeded at 2 (offset -5 lines).
Hunk #2 succeeded at 11 (offset -5 lines).
Hunk #3 succeeded at 48 (offset -5 lines).
Hunk #4 succeeded at 156 (offset -5 lines).
checking file core/modules/views_ui/src/Tests/ExposedFormUITest.php
Hunk #1 FAILED at 174.
1 out of 1 hunk FAILED
vprocessor’s picture

Assigned: Unassigned » vprocessor
vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
14.5 KB

rerolled

Status: Needs review » Needs work

The last submitted patch, 38: 2369119-38.patch, failed testing.

vprocessor’s picture

Assigned: Unassigned » vprocessor

The last submitted patch, 38: 2369119-38.patch, failed testing.

vprocessor’s picture

Assigned: vprocessor » Unassigned
Lendude’s picture

Status: Needs work » Needs review
FileSize
1.25 KB
15.29 KB

It was missing a schema for the bundle filter. Added that. This is now passing locally for me.

Status: Needs review » Needs work

The last submitted patch, 43: 2369119-43.patch, failed testing.

dagmar’s picture

Issue summary: View changes
FileSize
53.43 KB

Take a look to the status filter for admin/content, this is the default if you install drupal with this patch and click on the exposed status filter.

Same situation applies fro admin/people/list

dagmar’s picture

Status: Needs work » Needs review
FileSize
16.92 KB
2.1 KB

I cannot replicate locally the failure reported by automated tests.

I'm adding a new test to check the scenario I posted in the previous screenshot that fails locally.

Lendude’s picture

I cannot replicate locally the failure reported by automated tests.

Weird, Drupal\system\Tests\Update\UpdatePostUpdateFailingTest fails for me locally too.

+++ b/core/modules/node/config/optional/views.view.content.yml
@@ -400,11 +400,11 @@ display:
           plugin_id: boolean

Can't really rhyme the plugin_id with the set values. That's where this seems to be going wrong now. But haven't found a fix yet either.

dagmar’s picture

Sorry, I was testing the wrong Testcase. Anyway now we have more test coverage.

Status: Needs review » Needs work

The last submitted patch, 46: 2369119-46.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
4.4 KB
14.32 KB

@dagmar the extra test is exactly what was needed because it tests the boolean exposed filter. Undid a bit of work that olli did in #8 because the fix there was never going to work like that. The problem with the boolean filter was that 'boolean' is fine for the non-exposed filter, you only need to have 2 settings there, but it wasn't going to work for the exposed filter (it needs yes-no-all). You just can't fit three options into a boolean.
So we need a way to make the exposed filter use a different config then the non-exposed filter. So views.filter.group_item.* is back, with some overrides for filters where the 'value' is a different format for exposed and non-exposed filters.

All previous failing test now pass locally for me, so lets see what happens here.

Lendude’s picture

And for more fun with boolean filters, check out #2459289: Boolean default values are not saved

Status: Needs review » Needs work

The last submitted patch, 50: 2369119-50.patch, failed testing.

Lendude’s picture

olli’s picture

Nice work!

  1. #50:
    So views.filter.group_item.* is back, with some overrides for filters where the 'value' is a different format for exposed and non-exposed filters.

    views.filter.group_item.* allows to override *grouped* exposed filter schema if you need it to be different from single exposed filter. Single exposed filter uses same 'value' as non-exposed filter and that is broken with yes-no-all, right?

    After fixing the other issue from #51 this views.filter.group_item.boolean might not be needed anymore.

  2. +++ b/core/modules/views/config/schema/views.data_types.schema.yml
    @@ -777,7 +777,7 @@ views_filter_group_item:
         value:
    -      type: label
    +      type: views.filter_value.[%parent.%parent.%parent.%parent.plugin_id]
           label: 'Value'
    

    In numeric filter the group item would use views.filter_value.numeric so we can still remove views.filter.group_item.numeric with this?

  3. +++ b/core/modules/views/config/schema/views.filter.schema.yml
    @@ -120,7 +120,16 @@ views.filter.group_item.numeric:
    +views.filter.group_item.boolean:
    +  type: views_filter_group_item
    +  label: 'Group items'
    +  mapping:
    +    value:
    +      type: views.filter_value.string
    

    Do we need that "label: Group items", or should that be the default in "views.filter.group_item.*"?

Lendude’s picture

Thanks for the feedback olli!

54.1 I agree, the override should be on the exposed filter, but as far as I know there is no way to currently override the format for the exposed filter. So adding that here feels like a bit of a scope creep. So I'd say just focus here on getting the grouped filters working with the current fix, then in #2459289: Boolean default values are not saved see what we can do about overriding the format for the exposed filter (or just convert all the boolean filters to strings since that is the only filter that I see that needs a different exposed filter).
I'll post some of our findings there too.
Unless I'm wrong and there is a way to override the exposed filter format, then yeah we should totally use that!

54.2 Absolutely right

54.3 Yeah makes more sense

dawehner’s picture

  1. +++ b/core/modules/views/config/schema/views.filter.schema.yml
    @@ -94,7 +94,7 @@ views.filter_value.numeric:
     views.filter_value.equality:
    -  type: views.filter_value.numeric
    +  type: views.filter_value.string
       label: 'Equality'
    

    IMHO config schema should simply mirror the class hierarchy. For that, it would be great if this type would be actually its own thing.

  2. +++ b/core/modules/views/src/Tests/Handler/FilterDateTest.php
    @@ -153,4 +156,83 @@ protected function _testUiValidation() {
    +    $edit['options[group_info][group_items][1][title]'] = 'simple-offset';
    +    $edit['options[group_info][group_items][1][operator]'] = '>';
    +    $edit['options[group_info][group_items][1][value][type]'] = 'offset';
    +    $edit['options[group_info][group_items][1][value][value]'] = '+1 hour';
    +    $edit['options[group_info][group_items][2][title]'] = 'between-offset';
    +    $edit['options[group_info][group_items][2][operator]'] = 'between';
    +    $edit['options[group_info][group_items][2][value][type]'] = 'offset';
    +    $edit['options[group_info][group_items][2][value][min]'] = '+1 hour';
    +    $edit['options[group_info][group_items][2][value][max]'] = '+2 days';
    +    $edit['options[group_info][group_items][3][title]'] = 'between-date';
    +    $edit['options[group_info][group_items][3][operator]'] = 'between';
    +    $edit['options[group_info][group_items][3][value][min]'] = format_date(150000, 'custom', 'Y-m-d H:s');
    +    $edit['options[group_info][group_items][3][value][max]'] = format_date(250000, 'custom', 'Y-m-d H:s');
    

    Nice test coverage!

Unless I'm wrong and there is a way to override the exposed filter format, then yeah we should totally use that!

Yeah I'm not sure either!

In general for me its not that much of scope creep to be honest, its still kinda related to the issue, but sure, please go with what you prefer.

olli’s picture

#55.1 I agree, the interdiff looks good!

#56.1 I agree. Do you mean change views.filter_value.equality from type: views.filter_value.string to type: string and this

+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -131,3 +132,10 @@ views.filter_value.combine:
+views.filter_value.bundle:
+  type: sequence

to type: views.filter_value.in_operator?

Lendude’s picture

@olli oh yeah that looks like a nice addition.

I've rearranged the schema in views.filter.schema.yml a bit because there was a header for '# Schema for the views filter value' but they were actually scattered throughout the entire file which was driving me nuts. So all filter values now fall under that heading. Makes the patch and interdiff a little harder to read, but makes the actual file a lot easier to read, so thought it was worth it.

So the only odd duck in the file is now

views_filter_boolean_string:
  type: views_filter
  label: 'Boolean string'

That sounds like what the exposed boolean filter should be using, but then it really should be views.filter.boolean_string and it should actually do something other then just inheriting views_filter I think. Maybe something to look at in #2459289: Boolean default values are not saved

But, let's see what this does.

sukanya.ramakrishnan’s picture

The latest patch in 58 failed for me when i chose a between operator for a grouped filter on a date field.

InvalidArgumentException: The configuration property display.default.display_options.filters.field_from_date_value_1.value.min doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 74 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.field_from_date_value_1.value.min', '') (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.field_from_date_value_1.value', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.field_from_date_value_1', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display', Array) (Line: 212)
Drupal\Core\Config\Config->save() (Line: 280)
Lendude’s picture

Thanks for the feedback @sukanya.ramakrishnan. Not sure why this only revealed itself after adding the catch-all, but datetime needs its own thing.

This fixes the issue, but we need a test where we add a datetime field.

The last submitted patch, 60: interdiff-2369119-58-60.patch, failed testing.

mikeker’s picture

Version: 8.1.x-dev » 8.2.x-dev
Status: Needs review » Needs work

Setting to NW for the tests. Also bumping to 8.2.x as this patch adds strings.

The last submitted patch, 60: 2369119-60.patch, failed testing.

mikeker’s picture

Status: Needs work » Needs review
FileSize
1.14 KB
16.54 KB

Fixes the test that was broken in 8.2.x. Doesn't address the need for tests mentioned in #60 so this should go back to NW after the testbot runs.

dagmar’s picture

Status: Needs review » Needs work
Lendude’s picture

Status: Needs work » Needs review
FileSize
3.95 KB
20.14 KB

Added test for datetime field, it fails without the change in #60.

jhedstrom’s picture

Issue tags: -Needs tests

Removing the 'needs tests' tag.

Can the test-only patch be uploaded to demonstrate the issue/fix?

+++ b/core/modules/views/src/Plugin/views/filter/Date.php
@@ -131,7 +132,15 @@ public function acceptExposedInput($input) {
-    $type = $this->value['type'];
+    $type = NULL;
+    if ($this->isAGroup()) {
+      if (is_array($this->group_info)) {
+        $type = $this->group_info['type'];
+      }
+    }
+    else {
+      $type = $this->value['type'];
+    }

@@ -154,8 +163,11 @@ public function acceptExposedInput($input) {
-    // restore what got overwritten by the parent.
-    $this->value['type'] = $type;
+    // Restore what got overwritten by the parent.
+    if (!is_null($type)) {
+      $this->value['type'] = $type;
+    }
+

Interesting, this same bit of code is being changed over in #2648950: Use form element of type date instead textfield when selecting a date in an exposed filter, although in a slightly different way. I'm going to move that issue back to NR to take a look at this approach.

mpdonadio’s picture

With the YAML changes to the schemas, does this require an update path?

Lendude’s picture

@mpdonadio, I don't think so. This patch should make existing Views config not be broken anymore without changes to the current config.

@jhedstrom absolutely, here it goes!

Status: Needs review » Needs work

The last submitted patch, 69: 2369119-69-TEST_ONLY.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review

Back to Needs Review per the results of #66.

mpdonadio’s picture

This needs an IS update to describe what is really going on and what this patch is fixing. What is a little odd about this situation, is the bug is most evident in the Views UI if you already have content created and then make the view. If you do it the other way around, then you get the error when the view is executed. I don't totally grok this patch to update the IS myself, though (I'm following this because I am working on the patch mentioned in #67).

Few nits with the patch.

  1. +++ b/core/modules/views/src/Tests/Handler/FilterDateTest.php
    @@ -153,4 +182,115 @@ protected function _testUiValidation() {
    +    $edit['options[group_info][group_items][3][value][min]'] = format_date(150000, 'custom', 'Y-m-d H:s');
    +    $edit['options[group_info][group_items][3][value][max]'] = format_date(250000, 'custom', 'Y-m-d H:s');
    +
    

    Weird format, 'Y-m-d H:i:s' would be more normal.

  2. +++ b/core/modules/views/src/Tests/Handler/FilterDateTest.php
    @@ -153,4 +182,115 @@ protected function _testUiValidation() {
    +  /**
    +   * Test datetime grouped filter.
    +   */
    +  protected function testFilterDatetimeUI() {
    +    $this->drupalLogin($this->drupalCreateUser(array('administer views')));
    

    Two things here. The docblock doesn't describe this test accurately. I think "Test datetime filter UI." would be more accurate. Also, the other tests use the weird underscore convention and get called from testDateFilter(). For consistency sake, this should probably do the same thing.

Lendude’s picture

Thanks for the feedback @mpdonadio. Updated the IS to hopefully beter describe what is going on here.

72.1 yeah that format is strange, changed it.
72.2 updated to follow the _underscore format. You are right, lets stick to that (and since we don't need a fresh drupal install for this test anyway, this way saves on overhead)

Lendude’s picture

Issue tags: +DevDaysMilan
FileSize
5.61 KB

Sorry, forgot the interdiff...too much fun at DevDays to keep track of things!

mpdonadio’s picture

Took one more look at this. I think the IS is mostly OK, but could explain why there are also code changes in addition to the schema changes. Right now, it somewhat appears to have out-of-scope changes if you aren't familiar with the patch. Either way, I don't feel comfortable setting RTBC on this, as I am still scratching my head a bit on a few parts of the schema changes.

Lendude’s picture

Issue summary: View changes

@mpdonadio I added little addition to the IS to account for the non-schema changes in the patch, do you think this covers it now?

ronaldtebrake’s picture

Unfortunately got an error similar to #59 at first:

exception 'InvalidArgumentException' with message 'The configuration property display.default.display_options.filters.field_event_date_value.value.min doesn't exist.' in                                 [error]
/var/www/html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php:74
Stack trace:
#0 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(179): Drupal\Core\Config\Schema\ArrayElement->get('display.default...')
#1 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default...', '')
#2 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default...', Array)
#3 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default...', Array)
#4 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default...', Array)
#5 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default...', Array)
#6 /var/www/html/core/lib/Drupal/Core/Config/StorableConfigBase.php(211): Drupal\Core\Config\StorableConfigBase->castValue('display.default', Array)
#7 /var/www/html/core/lib/Drupal/Core/Config/Config.php(212): Drupal\Core\Config\StorableConfigBase->castValue('display', Array)
#8 /var/www/html/core/modules/locale/locale.module(384): Drupal\Core\Config\Config->save()
#9 /var/www/html/core/modules/locale/locale.module(316): locale_system_set_config_langcodes()

Steps taken:
- Applied the latest patch on 8.1.3 (worked nicely)
- Ran all the unit tests, no errors there
- Tried to do a new site install with importing the existing views through config yml files.
Exception only occurred when I did a drush si with --locale=XX.
A site install in English gave no issues with the installation, but saving the view through UI after the install gave the Exception you see above as well.
- Tried to save the same existing view with the following settings: Screenshot settings

After that I decided to remove the filter, add it again and export it through the UI to see any difference in the YML files.
Noticed that there was a diff on plugin_id: datetime added to the exported yml file. Which resulted in having no errors anymore :).
Not sure if it is because of outdated yml files or anything that the plugin_id wasn't there before.

pminf’s picture

After upgrading to Drupal 8.1.7 and applying the latest patch of this issue, I still get an error when saving one of my views, which includes a geolocation field filter:

The configuration property display.default.display_options.filters.field_geo_location_proximity.value.min doesn't exist.

I don't know if this a Drupal core issue or an issue of the contrib module geolocation.

This error also occurs on configuration import of the view. This is the filter part:

      field_geo_location_proximity:
          id: field_geo_location_proximity
          table: node__field_geo_location_proximity
          field: field_geo_location_proximity
          relationship: none
          group_type: group
          admin_label: ''
          operator: '<='
          value:
            min: ''
            max: ''
            value: '35'
            lat: '52.520015'
            lng: '13.404954'
            units: km
          group: 1
          exposed: true
          expose:
            operator_id: ''
            label: 'Distance (in kilometers)'
            description: 'Lat:52.5234146 Long:13.3702304 '
            use_operator: false
            operator: ''
            identifier: POI
            required: false
            remember: false
            multiple: false
            remember_roles:
              anonymous: anonymous
              authenticated: authenticated
              admin: '0'
          is_grouped: false
          group_info:
            label: ''
            description: ''
            identifier: ''
            optional: true
            widget: select
            multiple: false
            remember: false
            default_group: All
            default_group_multiple: {  }
            group_items: {  }
          expose_units: 0
          plugin_id: geolocation_filter_proximity

I'm only able to import the view configuration successfully, if I remove display.default.display_options.filters.field_geo_location_proximity.value from the configuration file.

If I only set display.default.display_options.filters.field_geo_location_proximity.value.max or something different I also get the appropriate error:

The configuration property display.default.display_options.filters.field_geo_location_proximity.value.max doesn't exist.

Unfortunately I cannot reproduce this error on a fresh installation. Can somebody please give me a clue, where to look for the error cause? I guess it's an configuration schema issue but I'm not familiar with this topic.

Lendude’s picture

@pminf yes this looks like a configuration issue in geolocation. The config that defines the views filter should also define the views filter value.

+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -128,6 +111,43 @@ views.filter_value.boolean:
+views.filter_value.numeric:
+  type: mapping
+  label: 'Numeric'
+  mapping:
+    min:
+      type: string
+      label: 'Min'
+    max:
+      type: string
+      label: 'And max'
+    value:
+      type: string
+      label: 'Value'

something along these lines but then geolocation specific.... But that will only start working with this patch applied. So I'd advice to open an issue for geolocation with a link to this issue.

Grayside’s picture

Works for me on a Content Type filter.

pminf’s picture

@Lendude: Thanks for the info. I opened a ticket: Core patch for views grouped filters results geolocation config error

Grayside’s picture

The generated query for grouped, exposed content type filters looks like this:

SELECT node_field_data.nid AS nid
FROM 
{node_field_data} node_field_data
WHERE (( (node_field_data.type IN  ('typea', 'typeb', 'typec', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0')) ))
ORDER BY node_field_data.changed DESC
LIMIT 51 OFFSET 0

Looks like a zero entry for each type excluded from the group.

Lendude’s picture

@Grayside, you think this is because of the patch or because of an unrelated grouping issue (since this patch is mostly config changes I wouldn't expect this, but who knows)? It looks like the query should work (it doesn't look great, but if it gets the right result set I can live with a follow up issue).

aleevas’s picture

Version: 8.2.x-dev » 8.1.x-dev
FileSize
154.99 KB

Works good for me
test

andypost’s picture

Version: 8.1.x-dev » 8.2.x-dev
Status: Needs review » Reviewed & tested by the community

should be fixed in upstream first
we using this patch for a time

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 73: grouped_filters-2369119-73.patch, failed testing.

mpdonadio’s picture

Issue tags: +Needs reroll

Looks like this needs to be re-rolled against 8.2.x

herved’s picture

It seems the patch needs reroll because #2686483: Add support for database condition operator "NOT BETWEEN" was committed and changed core/modules/views/src/Tests/Handler/FilterDateTest.php

herved’s picture

Status: Needs work » Needs review
andypost’s picture

Status: Needs review » Reviewed & tested by the community

back to rtbc

andypost’s picture

mpdonadio’s picture

Issue tags: -Needs reroll

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

seanr’s picture

This bug exists in 8.1.x as well. Any plans for a backport? This is blocking functionality I need for a client. :-/

mikeker’s picture

@seanr, Once this gets committed to 8.3.x it can be backported assuming there are no string changes that would prevent that. If not, you could still apply the patch manually.

FWIW, the patch applies cleanly to the latest head of 8.3.x; rerunning the tests against 8.3.x.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 88: grouped_filters-2369119-88.patch, failed testing.

The last submitted patch, 88: grouped_filters-2369119-88.patch, failed testing.

Lendude’s picture

Status: Needs work » Reviewed & tested by the community

Unrelated fails, back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 88: grouped_filters-2369119-88.patch, failed testing.

daffie’s picture

Status: Needs work » Reviewed & tested by the community

The testbot for 8.3 passes.

Jeroen’s picture

Has anyone tried this with a list of integers or a list of texts?

I am trying to create an exposed filter as a checkbox. So I'm using the trick with grouped filters, allowing multiple selection (to show checkboxes) but only creating 1 group.

When I create a field "field_shower" which is either

a list of text with the values:

shared|Shared
private|Private

or a list of integers with the values:

0|Shared
1|Private

the grouping doesn't work and fails with:

InvalidArgumentException: The configuration property display.default.display_options.filters.field_shower_value.group_info.group_items.1.value.1 doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 74 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

and

InvalidArgumentException: The configuration property display.default.display_options.filters.field_shower_value.group_info.group_items.1.value.shared doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 74 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

respectively.

Lendude’s picture

@Jeroen is this after applying the patch? Because for any filter that isn't a straight up string grouping will fail without this patch.

tlyngej’s picture

Looks like this patch needs a re-roll after https://www.drupal.org/node/2459289 has been commited

tlyngej’s picture

I believe that this would cut it for a re-roll

tlyngej’s picture

Status: Reviewed & tested by the community » Needs review

Gah, forgot to change back to NR.

mpdonadio’s picture

Status: Needs review » Reviewed & tested by the community

Manually compared the patches, and looks like a good reroll.

dagmar’s picture

Regarding #101. I tried that on simplytest.me and I couldn't replicate the error.

mpdonadio’s picture

I asked something similar back in #68, but since there are schema changes, does this need an empty hook_update_N() to force a rebuild so the new schema gets picked up even though it just makes the existing config work (and not require an update path)? TBH, I don't recall these discussions from last year (see #2507899: [policy, no patch] Require hook_update_N() for Drupal 8 core patches beginning June 29 for discussion and jumping off points).

zuernBernhard’s picture

So if it works- why do we have to wait until 8.3 ? Would love to see it in the next core release ...

mpdonadio’s picture

Version: 8.3.x-dev » 8.2.x-dev

A committer is going to have to make the decision on whether this can go into 8.2.x (I am tentatively setting it back). The policy is at https://www.drupal.org/core/d8-allowed-changes

We are current in RC, so it is not currently eligible w/o a framework manager marking it eligible. After 8.2 is release, it should be able to go in. Bugfixes are allowed, and there isn't anything disruptive in this.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 104: grouped_filters-2369119-104.patch, failed testing.

dagmar’s picture

Status: Needs work » Reviewed & tested by the community

Back to RTBC.

dawehner’s picture

Priority: Normal » Major

Given that produces an exception, I think this is actually a major issue.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php
@@ -1028,17 +1028,20 @@ protected function buildExposedFiltersGroupForm(&$form, FormStateInterface $form
       $children = Element::children($row['value']);
       if (!empty($children)) {
         foreach ($children as $child) {
-          foreach ($row['value'][$child]['#states']['visible'] as $state) {
-            if (isset($state[':input[name="options[group_info][group_items][' . $item_id . '][operator]"]'])) {
-              $row['value'][$child]['#title'] = '';
+          if (!empty($row['value'][$child]['#states']['visible'])) {
+            foreach ($row['value'][$child]['#states']['visible'] as $state) {
+              if (isset($state[':input[name="options[group_info][group_items][' . $item_id . '][operator]"]'])) {
+                $row['value'][$child]['#title'] = '';
 
-              if (!empty($this->options['group_info']['group_items'][$item_id]['value'][$child])) {
-                $row['value'][$child]['#default_value'] = $this->options['group_info']['group_items'][$item_id]['value'][$child];
+                // Exit this loop and process the next child element.
+                break;
               }
-              // Exit this loop and process the next child element.
-              break;
             }
           }
+
+          if (!empty($this->options['group_info']['group_items'][$item_id]['value'][$child])) {
+            $row['value'][$child]['#default_value'] = $this->options['group_info']['group_items'][$item_id]['value'][$child];
+          }
         }

This code is so unmaintainable :(

+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -79,24 +67,6 @@ views.filter.string:
-views.filter_value.numeric:
-  type: mapping
-  label: 'Numeric'
-  mapping:
-    min:
-      type: string
-      label: 'Min'
-    max:
-      type: string
-      label: 'And max'
-    value:
-      type: string
-      label: 'Value'

@@ -128,6 +111,43 @@ views.filter_value.boolean:
+views.filter_value.numeric:
+  type: mapping
+  label: 'Numeric'
+  mapping:
+    min:
+      type: string
+      label: 'Min'
+    max:
+      type: string
+      label: 'And max'
+    value:
+      type: string
+      label: 'Value'

This change is mixing bug fixes with moving stuff around. Making it super hard to review and work out what is a fix and what is just a move. Makes it super hard to review. Is there anyway to split the fixes and moves up into different patches? Afaik the order in the YAML file should not be significant unless there are duplicates.

alexpott’s picture

Status: Needs work » Needs review
FileSize
18.65 KB
3.12 KB

So the patch attached here makes it much easier to work out what is actually changing in the .schema.yml and we can file a followup to make the ordering more sensible. Fixed a couple of nits too.

alexpott’s picture

+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -109,18 +112,20 @@ views.filter.standard:
-views.filter.group_item.numeric:

So this is the only schema removal. This has been debated above (good to see). It'd be nice to confirm this has test coverage.

Status: Needs review » Needs work

The last submitted patch, 115: 2369119-115.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
591 bytes
18.77 KB

We have some grouped numeric filter tests:

    // Ensure the string "0" can be used as a value for numeric filters.
    $this->drupalPostForm('admin/structure/views/nojs/add-handler/test_exposed_admin_ui/default/filter', array('name[node_field_data.nid]' => TRUE), t('Add and configure @handler', array('@handler' => t('filter criteria'))));
    $this->drupalPostForm(NULL, array(), t('Expose filter'));
    $this->drupalPostForm(NULL, array(), t('Grouped filters'));
    $edit = array();
    $edit['options[group_info][group_items][1][title]'] = 'Testing zero';
    $edit['options[group_info][group_items][1][operator]'] = '>';
    $edit['options[group_info][group_items][1][value][value]'] = '0';
    $this->drupalPostForm(NULL, $edit, t('Apply'));
    $this->assertUrl('admin/structure/views/view/test_exposed_admin_ui/edit/default', array(), 'A string "0" is a valid value.');
    $this->assertNoGroupedFilterErrors();

    // Ensure "between" filters validate correctly.
    $this->drupalGet('admin/structure/views/nojs/handler/test_exposed_admin_ui/default/filter/nid');
    $edit['options[group_info][group_items][1][title]'] = 'ID between test';
    $edit['options[group_info][group_items][1][operator]'] = 'between';
    $edit['options[group_info][group_items][1][value][min]'] = '0';
    $edit['options[group_info][group_items][1][value][max]'] = '10';
    $this->drupalPostForm(NULL, $edit, t('Apply'));
    $this->assertUrl('admin/structure/views/view/test_exposed_admin_ui/edit/default', array(), 'The "between" filter validates correctly.');
    $this->assertNoGroupedFilterErrors();

So I think that should do for the bit that got removed?

Rerolled @alexpotts patch so that it's passing again locally. A little too much got removed (which is why I reordered it all in the first place, because it was really hard to keep track of what needed to go where).

Status: Needs review » Needs work

The last submitted patch, 118: 2369119-118.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
393 bytes
18.78 KB
+++ b/core/modules/views/config/schema/views.filter.schema.yml
@@ -97,7 +97,7 @@
-views.fiter_value.equality:
+views.filter_value.equality:

This override didn't do anything because of the typo in the name and it was just falling back on defaults which were good, without the typo it needs to have a type.

alexpott’s picture

Well the failing tests of #115 and #118 at least prove we have some test coverage!

Lendude’s picture

Status: Needs review » Needs work

Discussed with @alexpott on IRC, we could use some more explicit test coverage of buildExposedFiltersGroupForm() changes.

Also we should really have a javascript test for the #states stuff but that could be moved to a follow-up since that would add some major complexity to fixing this.

matthew.h’s picture

While we're waiting on getting this fixed is there at least a workaround for this?

Lendude’s picture

@matthew.h applying the patch, thats about all you can do. It should work fine, the only thing missing is additional test coverage.

Lendude’s picture

Status: Needs work » Needs review
FileSize
5.07 KB
23.68 KB

Since the changes to buildExposedFiltersGroupForm are all about showing the right fields with the right values, the easiest way to check this is with a javascript test after all. So a fairly basic test to check if the right fields are shown with the right values.

It would be better to also set these values in the test, but phantomJS dies when trying to 'Apply' the exposed filter values (see also #2754985: Add javascript test coverage for adding an exposed filter in Views UI):
Zumba\GastonJS\Exception\BrowserError: There was an error inside the PhantomJS portion of GastonJS.
This is probably a bug, so please report it:
click
html.touchevents.details.js body.user-logged-in.path-admin div.ui-dialog.ui-widget.ui-widget-content.ui-corner-all.ui-front.ui-dialog-buttons.views-ui-dialog-scroll.views-ui-dialog.js-views-ui-dialog div#drupal-modal.ui-front.ui-dialog-content.ui-widget-content div form#views-ui-config-item-form--a8Az0vK1h_o div#edit-options--TXBWT3fLHBE.scroll.js-form-wrapper.form-wrapper

so we have to make due with pre-set values.

lomasr’s picture

FileSize
44.11 KB
44.28 KB

1. Reproduced the error : Please see the screen.
2. Applied the patch in #125 , it worked cleanly . Now one group have value type "A set..."
3. When I save the view it still gives me the same error.

Lendude’s picture

You need to clear cache after applying the patch or the new config schema won't be used.

mpdonadio’s picture

If we need to clear caches after the patch is applied, then we need an empty update hook to do the same thing to ensure it happens when users update core.

Also a minor tweak to avoid a legacy assertion in a new test class.

A quick look at the new test looks good to me.

Lendude’s picture

If we need to clear caches after the patch is applied, then we need an empty update hook to do the same thing to ensure it happens when users update core.

Yeah I realised the same thing 5 seconds after posting that comment.

Also a minor tweak to avoid a legacy assertion in a new test class

Heh, didn't know that was on AssertLegacyTrait, not all the individual methods are marked as deprecated, so didn't show up in (my version of) PHPStorm.

@mpdonadio++

lomasr’s picture

After clear cache it worked . Thanks

attheshow’s picture

I'm getting the same issue mentioned by @Jeroen (comment #101) after applying the patch and clearing cache. My issue is when using a grouped filter on a List (text) type of field.

Lendude’s picture

*sigh* yup views.filter.list_field is broken too.

Attempt at a fix for that + test. The test fails but the fix works when manually testing this, so probably still something wrong with the test, but not clear headed enough to get it working right now, so here is the progress so far.

Status: Needs review » Needs work

The last submitted patch, 132: 2369119-132.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
1.47 KB
27.44 KB

Test and hopefully fix.

danielnolde’s picture

Patch in #134 fixed the Exception for me (trying to save a view with grouped exposed filter on a timestamp field).
This seems to be a valid solution, hope to see it get into core soon.

Thanks, Lendud!

attheshow’s picture

Patch #134 is working for me with a grouped filter on a List (text) type of field. Thanks!

Andrej Galuf’s picture

Unfortunately, #134 did all sorts of nasties for me, including breaking a custom numeric field extension without min and max values, while still not solving the grouped exposed filter problem (grouped roles filter).

Lendude’s picture

breaking a custom numeric field extension without min and max values

@Andrej Galuf did you have a config schema for that custom field extension? If yes, what did you need to change to get it to work? If no, did adding a config schema resolve your issue?

And I'm sure there are more grouped filters that are not working yet, but we need to get this in to at least have a basis on which to solve those issues.

did all sorts of nasties for me

Anything besides these things? Being specific helps find the issues and solving them...

boinkster’s picture

Patch #134 doesn't work for me.
I've got an exposed grouped filter on an Entity Ref field. I manually built a select list from Label|nid. It works fine until the remember option is set, then it throws a "An illegal choice has been detected. Please contact the site administrator." when the page is refreshed.

Lendude’s picture

@boinkster that is unrelated to what we are trying to fix here, this is about saving the grouped filter and getting a WSOD. Please search the issue queue for your problem and open a new issue for that if you can't find anything related. Did a really quick search but could only find some D7 issues that appear to be the same problem.

boinkster’s picture

Thanks, I'll open another issue (I found the same D7 post). For my use case, I wasn't getting a WSOD even with unpatched core.

youfei.sun’s picture

Same issue when using a taxonomy field as grouped exposed filter, #134 won't fix that. Guess all numeric field types has to be fixed or we can have some solution can handle all types of content.

mpdonadio’s picture

Status: Needs review » Needs work

Don't have time for this right now, but per @catch:

We shouldn't use hook_update_N() for empty updates to trigger a cache flush - it adds all the potential complexity of schema versions between minor branches, conflicts with other patches, inconsistencies if there's a revert etc. hook_post_update_NAME() achieves the same thing without most of that trouble.

And per @catch the post-update hook can be empty in this case to achieve the same thing.

Lendude’s picture

Thanks for the update @mpdonadio, nice to know what to do in such cases.

This should fix any filters inheriting views.views_filter.in_operator and views.views_filter.many_to_one as the removal of the fix but passing test from #134 should indicate.

This should fix the grouped filter issues indicated in #137 and #142.

youfei.sun’s picture

I've tested latest patch, it seemed to fix the roles issue in #134, but for taxonomy, the view could save correctly, however the sql query generated is mistaken. Instead of a where clause with corresponding taxonomy term ids, the sql generated became the group ids.

Lendude’s picture

@youfei.sun the generated sql issue sounds unrelated to what we are trying to fix here, so please look around and see if you can find an issue for that and open one if you don't.

youfei.sun’s picture

@Lendude you are right, that belongs to here https://www.drupal.org/node/2049603.

I've got a patch uploaded for d8 to this issue.

chris.smith’s picture

I've applied the patch in #145 and was able to resolve the issue. I have not completed a thorough review, but from the surface, seems to be OK.

xSDx’s picture

Tested Patch #145 and was able to solve the issue.

sagesolutions’s picture

Applied patch #145 and was able to fix the related issue Creating a view with exposed filters fails on save

imrancluster’s picture

I had a custom query's problem. I found the following file which have force fully added "ONLY_FULL_GROUP_BY"

core/lib/Drupal/Core/Database/Driver/mysql/Connection.php

'sql_mode' => "SET sql_mode = 'ANSI,STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,ONLY_FULL_GROUP_BY'"

I removed the "ONLY_FULL_GROUP_BY" from the file then my module run as expected.

Thanks

remram’s picture

I'm facing the same problem!!! I want to add a group filter to my view. Users should be able to filter by content type.
The preview is working fine but when I try to save my view, it runs to an exception error. I have no idea where the problem is. I'm using Drupal 8.2.2

The website encountered an unexpected error. Please try again later.

InvalidArgumentException: The configuration property display.default.display_options.filters.type.group_info.group_items.1.value.acc_grp doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 74 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type.group_info.group_items.1.value.acc_grp', 'acc_grp') (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type.group_info.group_items.1.value', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type.group_info.group_items.1', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type.group_info.group_items', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type.group_info', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters.type', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options.filters', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default.display_options', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display.default', Array) (Line: 211)
Drupal\Core\Config\StorableConfigBase->castValue('display', Array) (Line: 212)
Drupal\Core\Config\Config->save() (Line: 280)
Drupal\Core\Config\Entity\ConfigEntityStorage->doSave('group_naviagation_view', Object) (Line: 392)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 259)
Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object) (Line: 364)
Drupal\Core\Entity\Entity->save() (Line: 637)
Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 986)
Drupal\views_ui\ViewUI->save() (Line: 312)
Drupal\views_ui\ViewEditForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 585)
Drupal\Core\Form\FormBuilder->processForm('view_edit_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 48)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'edit', Array) (Line: 226)
Drupal\views_ui\Controller\ViewsUIController->edit(Object, NULL)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 144)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 64)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 652)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Lendude’s picture

@remram did you apply the patch in #145 (see Applying patches)? That should resolve your issues.

remram’s picture

@Lendude: I'm not the only one, who is working on this project. I will try it in the near future. Thank you so far :)

smaz’s picture

Another +1 for patch in #145 fixing my issue: I was unable to save a view that had grouped filters on a taxonomy term field.

Cheers!

tlyngej’s picture

I don't feel that I'll have the skills to review this, even though I've made a few re-rolls, so can't put it to RTBC.

Any chance that anyone else could, so that this would make it to 8.3?

dawehner’s picture

Issue summary: View changes

As talked with @lendude this issue fixes a lot of problems already. There might be more scheme issues in other filters, but creating new issues is easy :)
I documented this in the issue summary as well.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This solves a lot of problems for a lot of people. The SQL errors are unrelated to this issue. Please open up new issues so we can fix them as wel..

oriol_e9g’s picture

Yes, RTBC++

gbyte.co’s picture

Successfully using the last patch as well.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 145: 2369119-145.patch, failed testing.

Lendude’s picture

Status: Needs work » Reviewed & tested by the community

Unrelated fails

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 145: 2369119-145.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Reviewed & tested by the community

Not sure who triggered a retest, but the latest fail was b/c #2828559: UpdatePathTestBase tests randomly failing, which is unrelated. In this case, we don't need to re-test. We should let a committer decided what to do.

Lendude’s picture

@mpdonadio I triggered the retest, it failed on the same test twice, just wanted to be absolutely sure it was still passing before starting to spam everybody in the issue with more RTBC->'needs work' flip-flop messages.

But it's green now, so I'm happy!

anavarre’s picture

FileSize
139.22 KB
159.88 KB

deleted - please ignore

anavarre’s picture

Blanca.Esqueda’s picture

Status: Reviewed & tested by the community » Needs work

I was having the same issue ( D8 ), unable to save the view having a exposed grouped filter.
Applied the patch and I was able to save the view.

Unfortunately it didn't work as expected.
I'm using a taxonomy in my grouped filter - to limit the terms to be displayed on the dropdown -.
The dropdown displays the titles correctly but the values selected for the grouped option are not going to the query correctly.

As sample on my grouped filter interface added two options:
Option1 equal to 54 (tid of the term selected)
Option2 equal to 69 (tid of the term selected)

But when I try to filter the view using those options, the results are not the correct ones.
Checked the query itself and the condition for that filter uses the values of the dropdown (incremental pointer) instead of the real values:
Option1 --- taxonomy_target_id = 1 (instead of 54)
Option2 --- taxonomy_target_id = 2 (instead of 69)

Blanca.Esqueda’s picture

Also, something else that I noticed is that it is not passing more than one value.

Added a grouped option selecting two values, and when I checked the query still only passing one value and the wrong one.
Attached three images for clarity:
1.- grouping filter interface
2.- kint displaying the filter elements.
3.- View Query with an option selected on the filter. (from my previous comment).

Lendude’s picture

Title: Problems with a number of views grouped filters » Fatal error when trying to save a View with grouped filters using other then string values
Status: Needs work » Reviewed & tested by the community

Applied the patch and I was able to save the view.

This patch is about that. This is not about solving all problems with all grouped filters in one patch. Updated the title to clarify what we are trying to fix here.

@Blanca.Esqueda please open follow ups for the issues you describe and postpone them on this issue. We need to get this in first before we can start fixing any underlying problems.

Back to RTBC for the fix for the fatal.

claudiu.cristea’s picture

@Lendude, the test ExposedFormUITest::testExposedGroupedFilter() has been created as part of duplicate issue #2595027: View crash on saving having exposed filter with groupings. It would be nice if that will be credited too in this ticket.

Lendude’s picture

@claudiu.cristea++

Yes @maintainers please also take #2595027: View crash on saving having exposed filter with groupings into consideration when assigning credit.

wturrell’s picture

@Lendude - what's the status with filters grouped by Content Type?

Patch 145 - NB: on a "used", rather than "clean", 8.2.5 install, with a rebuilt cache – doesn't seem to fix it for me; I see #80 saying it's fixed and #153 saying it isn't.

Like #153, works fine in the preview, fails to save changes.

I've a simple grouping on Content: Content Type using the 'is one of' operator for both groups.

The missing config property obviously matches the first checked content type in the first group of the filter.

Anything else I can try to help you?

Lendude’s picture

FileSize
110.85 KB

@wturrell patch works for me.

With a filter set up like this for the Content View:

Without the patch I get the fatal, applied the patch, drush cr, reload page, save View, save successful.

wturrell’s picture

Sorry this was my fault - the patch wasn't being applied, now I've done so, 2369119-145.patch works on the old site and also on a clean 8.2.x install (grouping Content Type by articles and basic pages).

[Rest of this is newbie explanation and can be skipped by more clever people..]

On the failing site I was using 'git apply' on a drupalvm install (with drupal-composer/drupal-project 8.x branch) and I was getting no output other than [ok], and then I realised no files were changing and I wasn't getting any output for git apply --verbose, git apply --summary or git apply --check either..

I switched from 'git apply' to patch -p1 after reading the only explanation I found, here.

git apply doesn't even try if it's already in a repository (other than the one for the project the patch is for)

Which I take to mean you can only apply patches to https://git.drupal.org/project/drupal.git, otherwise it will silently skip all the commit ID hashes that don't match? (That said, I'd hoped git apply --check might give me some output, seeing as the help text says it's to check if it's applicable...)

Also, on your screenshot, what is it you're filtering on to get 'node' and 'other' as the only two options?

(Thanks)

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

YesCT’s picture

Issue tags: +DrupalCampNJ2017

I'm triaging this at the NJ Camp. #2474049: [meta] Major issue triage

YesCT’s picture

Title: Fatal error when trying to save a View with grouped filters using other then string values » Fatal error when trying to save a View with grouped filters using other than string values
Issue summary: View changes
Issue tags: +Triaged for D8 major current state

I went through the instructions in #2474049: [meta] Major issue triage and it gets all the way to adding the triaged for d8 major current state. (has duplicates marked, summary is up to date, patch applies .. oh, 8.4.x vs 8.3.x, it does apply to 8.4.x... flow chart says to retest, since this is rtbc, it has been being retested on .. hm 8.2.x so I added custom test runs for retesting on 8.3.x and 8.4.x. leaving the version in the meta data for this issue on 8.3.x, partly cause the major issue triage doesn't say to evaluate that field.)

Not needed as part of the major triage meta, but I also checked into if this was major.
And according to https://www.drupal.org/core/issue-priority#major-bug

Trigger a PHP error through the user interface, but only under rare circumstances or affecting only a small percentage of all users, even if there is a workaround.

Cause this is a views issue that deals with database things (and cause someone already decided that the issue should be tested (when it was 8.2.x) on other databases, also adding custom test runs for those on 8.3.x and 8.4.x.

----

Taking a break for now. Next, looks like this could use a code review (especially to make sure that the feedback from alex was addressed).

alexpott’s picture

I can confirm that my feedback has been addressed. The additional test coverage has been added and changes have been made to keep the schema in line with class inheritance.

And having a javascript test of this form in core gives us more than we've ever had before.

alexpott’s picture

It's important we get an answer for @Grayside's question in #82

Lendude’s picture

@alexpott yeah that happens:

Steps to reproduce:
- Apply the patch
- Make two content types
- Make a grouped filter for content type
- Add one group 'is one of' for one content type
- Add one group 'is one of' for the second content type
- Save
- Filter using one of these groups and you wil see IN('node_type_1' ,'0', '0') in the query.

This sounds like work for a follow-up though. This doesn't sound like it is caused by this patch, but it looks like this is an existing bug that just gets exposed by fixing grouped filters in general. I actually expect more issues with grouped filters to float to the surface after this is committed because this will actually allow you to create them....

Lendude’s picture

@alexpott quick check, and yeah this is caused by \Drupal\views\Plugin\views\filter\InOperator::opSimple not cleaning the empty checkboxes values:

This:
$this->query->addWhere($this->options['group'], "$this->tableAlias.$this->realField", array_values($this->value), $this->operator);
Should be something like:
$this->query->addWhere($this->options['group'], "$this->tableAlias.$this->realField", array_filter(array_values($this->value)), $this->operator);

But since 0 is a valid value, it needs more logic then that. (and tests of course)

But like I said, I feel this is unrelated and should not hold up this issue.

alexpott’s picture

@Lendude well I think we need to filter the disabled checkboxes before we save the view to config rather than when we are constructing the query - can you open a followup for this?

tacituseu’s picture

They are added in Drupal\views\Plugin\views\filter\FilterPluginBase::buildGroupSubmit(), there seems to be a provision for filtering empty values, check: Drupal\views\Plugin\views\filter\FilterPluginBase::hasValidGroupedValue():

$actual_values = count(array_filter($group['value'], 'static::arrayFilterZero'));

so it could be as simple as using it in buildGroupSubmit(), one line fix attached, decide if a separate issue is needed.

tacituseu’s picture

On second thought... ;)

Lendude’s picture

Follow up for the InOperator issue: #2857740: Views grouped filters that use InOperator add '0' to the query for every not selected option

@tacituseu please move your proposed fix there, it looks like a great start off point. But it would require tests and it's unrelated to the config change we are doing here, so lets not scope creep it in.

alexpott’s picture

Rebased post array-all-the-things

Lendude’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

Committed 594fdb7 and pushed to 8.4.x. Thanks!

I've discussed this issue with @catch, @cilefen and @xjm and we agreed to set this to patch to be ported and consider for inclusion into the 8.3.0 release. Given the addition of an update function and fixes to configuration schema it'd be nice to not have to consider this for 8.3.1 and the number of comments on the issue reflect the fact that a lot of users are running into this issue.

It is really exciting to have javascript testing of the grouped exposed filter functionality. This is incredibly complex and fragile so it is no surprise we broke it.

  • alexpott committed 594fdb7 on 8.4.x
    Issue #2369119 by Lendude, olli, mpdonadio, alexpott, dagmar, mikeker,...
catch’s picture

Just to make sure, isn't this the only update function?

+ * Rebuild caches to ensure schema changes are read in.
+ */
+function views_post_update_grouped_filters() {
+  // Empty update to cause a cache rebuild so that the schema changes are read.
+}

If so I'm much less concerned about this going into 8.3.x during the rc than if there was an actual data change. The schema changes make me not want to consider it for 8.3.1 though.

Lendude’s picture

@catch yes the actual data that got saved was always correct, it just didn't match the config schema, hence the fatal.

bkosborne’s picture

Is there anything that needs to be done to get the patch ready for 8.3, or is it just waiting for a core committer to apply it to 8.3? I'm not sure what the procedure is for the "patch (to be ported)" status.

nemethf’s picture

FileSize
27.23 KB

#189 didn't apply to me because of a recent api doc group removal

alexpott’s picture

Status: Patch (to be ported) » Fixed

Committed 6ca8ccc and pushed to 8.3.x. Thanks!

  • alexpott committed 6ca8ccc on 8.3.x
    Issue #2369119 by Lendude, olli, mpdonadio, alexpott, dagmar, mikeker,...
andypost’s picture

Status: Fixed » Closed (fixed)

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

sukanya.ramakrishnan’s picture

Anyone having issues saving a view that has taxonomy filters (Not grouped, just regular ones) after applying the patch for 8.2 ?

I am getting this error - The configuration property display.default.display_options.filters..value.4 doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 74 of core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

Thanks,
Sukanya

sukanya.ramakrishnan’s picture

I have been breaking my head over this issue today and i am finding that any view that has a taxonomy filter with the widget as autocomplete or SHS fails with the error.
Dropdown works fine with taxonomy filters. Any inputs appreciated!

Update: I was wrong about dropdown working fine with taxonomy filters. I am still not clear about the usecase, but i am finding this issue with some taxonomy filters , exposed or not exposed and any kind of widget. Looks like some configuration is missing, but just unable to figure it out. !

Update: the filter field for which the view save is failing is a many to one field and the config schema mapping is falling back to views.filter_value.* . I am not too sure what should go for views.filter_value.many_to_one config?

Thanks,
Sukanya