Views filtering on boolean fields doesn't use right formatter

After digging a little further, it seems what the old 'properties' of D7, now called base fields, are handled somewhat differently from custom Entity Fields.

The base field 'status' is correctly labeled as a filter of type 'boolean' in the switch statement in the function mapSingleFieldViewsData in core/modules/views/src/EntityViewsData.php. The code there is using the field type to determine the views configuration.

A newly created field of type boolean however, is processed in the function views_field_default_views_data in core/modules/views/views.views.inc. Unfortunately that code is using the db column type to decide on the views configuration for the field. Boolean fields are created as tiny int in the database, which ends in the fields getting incorrectly labeled as of the filter type 'numeric'.

Why base fields are treated differently from custom fields is a mystery to me :)

To me this is a big issue, since it effectively blocks you from using a boolean field as a filter.

Why base fields are treated differently from custom fields is a mystery to me :)

The answer is simply time.

OK, i could have guessed that ;)

It works by mimicking the switch-case code from the mapSingleFieldViewsData
Is it an option that i provide something like this as a patch ?
There's plenty of discussion on related subjects that might make this code temporary, but i assume it's better to have something that works now than code that is broken ?

The discussion involve topics and solution that don't seem to be on the horizon right now ...
(f.i. https://www.drupal.org/node/2337515)

Yeah that issue is indeed dealing with that kind of problem.
Making those mappings more parallel is certainly the right first step to get some small level of sanity there.

Ok, attached is a first try at integrating the code from the base fields into the hook for the custom fields.

I can confirm the patch works for Boolean fields, didn't check other field types.

Before:

After:

1. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +    // Set up the sort, argument, and filters, based on
   +    // the column and/or field data type.
   

   The wrapping is 80 characters. You should continue the first line till reach 80 char.

2. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +    // @todo Allow field types to customize this.
   +    // @see https://www.drupal.org/node/2337515
   

   According to our coding standards @see must be before @todo. See https://www.drupal.org/coding-standards/docs#order.

3. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +    switch ($field_type) {
   

   If we are introducing this switch then I guess we'll need to provide tests for each $field_type, right?

4. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +        $field = 'field';
   

   What's $field? Seems that is never used.

5. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +        // For most fields, the field type is generic enough to just use
   +        // the column type to determine the filters etc.
   

   'the' goes one line up.

6. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +
   

   No need for the empty line.

Fixed the issues mentioned in #8 and added some testing (we should probably add more though)

1. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +    // @todo Allow field types to customize this.
   +    // @see https://www.drupal.org/node/2337515
   

   I think that todo is only applicable for schema fields.

2. +++ b/core/modules/views/views.views.inc
   @@ -489,28 +490,84 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
   +      case 'timestamp':
   +      case 'created':
   +      case 'changed':
   ...
   +      case 'language':
   ...
   +      case 'uri':
   

   These cases don't really make sense for fields, this, I think it only applies to schema fields.

I'm all for restoring sanity, but hanging this bug up on this seems strange to me.

Can't we just fix this issue and open another issue to get custom fields to be treated the same as base fields?

The patch in #9 for example breaks the 'numeric' field (manual testing revealed this), so like @claudiu.cristea pointed out we would need coverage for all the field types for that to move forward.

Adding a patch that just makes an exception for the boolean field, so the test coverage provided by @geertvd would be sufficient for that change I feel.

I tested #11 patch on simplytest.me
Result looks bit weird to me.
As you can see in 2469553-Node-creation-form.png there are radio buttons for the field.
However, for views' filter criteria field setting i can see select list instead of radio buttons 2469553-views-filter-criteria-form.png
May be others should also review it.

Update:
While testing this field, I kept field type as "List (text)" and from "Manage form display" I changed widget to "Checkboxes/Radio buttons"

While testing this field, I kept field type as "List (text)"

Yeah this only applies to field type 'Boolean'. Retested this, and looks fine to me.

Edit: 'Fine' meaning, it looks exactly like it does in the screenshot in the Issue summary.

+++ b/core/modules/field/src/Tests/Views/FieldUITest.php
@@ -112,4 +114,55 @@ public function testHandlerUIAggregation() {
+   */
+  public function _testTextFilterHandler() {
+    $field_name = 'field_name_0';
+    $url = "admin/structure/views/nojs/add-handler/test_view_fieldapi/default/filter";
+    $this->drupalPostForm($url, ['name[node__' . $field_name . '.' . $field_name . '_value]' => TRUE], t('Add and configure @handler', array('@handler' => t('filter criteria'))));
+    $this->assertResponse(200);
+
+    $this->xpath('//input[@type=:type and @id=:id]', array(':type' => 'text', ':id' => 'edit-options-value'));
+
+    $this->drupalPostForm(NULL, [], t('Apply'));
+
+  }

Is there a reason this test function doesn't assert anything?

I think we can just remove that bit of the tests. That was probably intended as a start to test all filter types, but if we want to take this the route of only fixing it for the boolean field, then that is no longer needed.

Confirming patch solves boolean field issue.

Before

After

#15 looks reasonable for me.

Do we want a follow-up for the refactor in #9?

@catch
Well, ideally this entire bit would be merged with EntityViewsData itself.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!