Don't prevent filterable rows from being filterable

[lukasolson]

Fixes #8678.

For some reason, the data table has only ever allowed filtering on rows that have some sort of field associated, which meant that when you aggregate using the "filters" bucket agg, you were not able to filter on the associated row. This was strange because if you had created the same visualization as a vertical bar chart, you could click on the bar to filter.

This PR simply removes the check that the row has a field associated with it, which means you can now create filters from the "filters" bucket aggregation!

Summary: Data tables using the "Filters" aggregation now allow you to click on a row to create a filter.

Before:

After:

[lukasolson]

yo jenkins, i'd really appreciate it if you'd test this

[Bargs]

Ran into a couple issues but I think they have an easy fix.

• This adds filter icons to agg buckets that don't support filtering, like geohash_grid:

We can check whether an agg type supports filtering with the following condition:

_.get(contents, 'aggConfig.type.createFilter')

• This also adds filter icons to fields that are aggregatable, but not filterable, which will thrown an error if you try to filter on them. It's probably not common, but it's possible if you had a mapping like this:

PUT aggnofilter
{
  "mappings": {
    "aggnofilter": {
      "properties": { 
      "notindexed": {
        "type": "integer",
        "index": false
      }
      }
    }
  }
}

We can also check for this with the following condition:

(contents.aggConfig.getField() ? contents.aggConfig.getField().filterable : true);

Putting it altogether, we can write this condition as follows:

const isCellContentFilterable =
  contents.type === 'bucket'
  && _.get(contents, 'aggConfig.type.createFilter')
  && (contents.aggConfig.getField() ? contents.aggConfig.getField().filterable : true);

[lukasolson]

Awesome, thanks @Bargs

[lukasolson]

Do I really need to check if contents.type === 'bucket' if I'm checking _.get(contents, 'aggConfig.type.createFilter')?

[Bargs]

Not 100% sure, but that sounds right.

[Bargs]

Oh, one other thing, it would be really cool if the agg's filter labels became filter aliases. But that's just a nice to have, we could add it some time in a separate PR.

Made the requested changes

[lukasolson]

@Bargs Ready for another look.

[thomasneirynck]

This is a neat addition!

minor comment, for your consideration

[thomasneirynck]

I'd consider moving this check to the AggConfig type instead. Then you could do something like if (aggConfig.isFilterable()), and we don't need to ducktype this here.

[Bargs]

@lukasolson lemme know if you wanna take a crack at this, if so I'll hold off on giving it another look. Otherwise I'll do another review first thing tomorrow.

[lukasolson]

@Bargs Just fixed this. Feel free to take a look now. Thanks!

[Bargs]

Looked to me like the previous test run passed but s3 upload failed.

LGTM! Great stuff

[lukasolson]

Backported to 5.x (5.5.0) in f25f118.