Feature fix 99239

[maihde]

Summary

This PR provides a proof-of-concept fix for #99239. It likely needs iteration before being merged into the baseline. It makes the following usability improvements:

• Only include geo-fields that are actually being used in layers. This may be a debatable point (i.e. I want to render fieldA but filter on fieldB) when drawing shapes; if that is a desired feature, then I suggest that the "Filtering field" list be ordered such that those other fields are lower on the list and in a visually distinct manner.
• When multiple index patterns have the same field name, it's redundant to show each of them in the list because the ultimate filter that is created is agnostic to the index. Instead, the control now shows the geoFieldType and deduplicates based off name/type.

This PR was developed against v7.11.2.

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[kibanamachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[jbudz]

@maihde Hi! Thanks for the PR. Is this supposed to be targeting a different branch? The diff looks quite large.

[maihde]

The PR was based off the 7.11.2 tag (since that the version we are running). Sorry about that. I can't find a way to change that...if you can update the PR that would help, otherwise I can create a new PR.

…

On Thu, May 6, 2021 at 11:49 AM Jonathan Budzenski ***@***.***> wrote: @maihde <https://github.com/maihde> Hi! Thanks for the PR. Is this supposed to be targeting a different branch? The diff looks quite large. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#99482 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYEIVCCDQL2NG32DH3OCTTMK3BTANCNFSM44HHUS2Q> .

[elasticmachine]

Pinging @elastic/kibana-gis (Team:Geo)

[jbudz]

I updated the target branch and tagged this with the geo team for review. Thanks again.

[thomasneirynck]

hi @maihde , thx for the proposal. Overall, I think these changes make sense to me.

The first thing that jumps out is logistical really. This PR targets 7.11. Could you open this PR up against master instead? After a PR is merged into master, it will then get backported to the 7.x release branch.

Let me know if you have questions or comments around this.

[maihde]

@thomasneirynck I have updated the PR to target the master branch.

[kmartastic]

Hey @maihde.

We had a discussion (@thomasneirynck @nreese @nickpeihl) about interactive spatial filtering and how we might want to improve it today. I'll summarize my perspective here.

I think the most common way to use a spatial filter is to filter everything that is visible. If you draw a circle, and there are 3 layers intersecting/within the circle, each layer should be filtered.

I think the second most common filtering is to apply the filter to a specific layer -- but -- do not apply the filter to all other layers. You can do this today, but you need to set all other layers to ignore global filters.

The least common scenario to me is the default behavior today -- you specify a layer in the map field, and draw a filter, which only applies to the layers based off the field/index pattern.

Although I agree your proposal is reasonable. It's not really the direction I want to invest in. I would rather change (break) the default behavior and adapt it to my preferences above. I wouldn't want to do anything, without broader customer feedback (which I will try to gather).

What do you think?

[maihde]

@kmartastic yes, I agree with your perspective. In our case, the most common and least common scenarios generally overlap because all of the layers are using the same geo-spatial field. This PR attempts to address a narrow issue quickly with minimal change rather than the broader usability concerns. If you think there is a timely path toward improving things more broadly we are happy to test things out.

[nreese]

If you think there is a timely path toward improving things more broadly we are happy to test things out.

@maihde The ability to generate a filter that will work for any geo field on the map was needed for filtering dashboard panels by map bounds. From experimenting with this PR, we found that we can generated a bool.should filter that contains nested bool.must filters that test to ensure the geoField exists prior to testing if the geoField is contained in the spatial feature. In the example below, the filter looks for documents where either geoFieldAlpha exists and is within the bounding box or geoFieldBravo exists and is within the bounding box allowing this filter to match documents with either geo field regardless of if the other geo field exists or not.

{
  query: {
    bool: {
      should: [
        {
          bool: {
            must: [
              {
                exists: {
                  field: 'geoFieldAlpha',
                },
              },
              {
                geo_bounding_box: {
                  [geoFieldAlpha]: {
                    top_left: [120, 60],
                    bottom_right: [140, 40],
                  },
                },
              },
            ],
          },
        },
        {
          bool: {
            must: [
              {
                exists: {
                  field: 'geoFieldBravo',
                },
              },
              {
                geo_bounding_box: {
                  [geoFieldBravo]: {
                    top_left: [120, 60],
                    bottom_right: [140, 40],
                  },
                },
              },
            ],
          },
        },
      ]
    },
  }

This method was not really possible in prior releases because the query syntax were so different between geo_point and geo_shape (with geo_point query constructs and geo_shape query constructs not supporting the other's geo field type). Elasticsearch has really helped out by supporting both geo_shape and geo_point fields for most geospatial queries now, making this effort much easier.

We should be able to use this patter when creating spatial filters and all creating a spatial filter for all geo fields in the map.

[nreese]

@maihde

I have created #100735 to remove the need to select a geo field when creating a spatial filter. The generated filter is a bool.should filter that contains nested bool.must filters that test to ensure each geoField exists prior to testing if the geoField is contained in the spatial feature. That way, the filter will work for each index and geo field in the map.

Let me know if this functionality works for your use cases.

[thomasneirynck]

Replaced by #99482. Thanks @maihde for creating this POC and getting the ball rolling!