[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working #153816

nreese · 2023-03-27T21:11:19Z

Background

When advanced setting courier:ignoreFilterIfFieldNotInIndex is enabled, filters are ignored when data view does not contain the filtering field. The logic on how this works is captured below for context.

https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83

      .filter((filter) => {
        const indexPattern = findIndexPattern(inputDataViews, filter.meta?.index);
        return !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern);
      })

https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20

export function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase | null) {
  if (!filter.meta?.key || !indexPattern) {
    return true;
  }

  // Fixes https://github.com/elastic/kibana/issues/89878
  // Custom filters may refer multiple fields. Validate the index id only.
  if (filter.meta?.type === 'custom') {
    return filter.meta.index === indexPattern.id;
  }

  return indexPattern.fields.some((field) => field.name === filter.meta.key);
}

The problem is that mapSpatialFilter is setting meta.key property. This causes filterMatchesIndex check to fail for spatial filters.

Spatial filters are designed to work across data views and avoid the problems that courier:ignoreFilterIfFieldNotInIndex solves. As such, spatial filters should not set meta.key property and not get removed when courier:ignoreFilterIfFieldNotInIndex is enabled.

Test

set advanced setting courier:ignoreFilterIfFieldNotInIndex and refresh browser
install 2 or more sample data sets
create a new map
add documents layer from one sample data set
add documents layer from another sample data set
create spatial filter on map, https://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters
Verify filter is applied to both layers

…phic filters no longer working

nreese · 2023-03-27T21:17:44Z

x-pack/plugins/maps/public/routes/map_page/map_app/map_app.tsx

+    }
+
+    // ignore results for outdated requests
+    if (!_.isEqual(nextIndexPatternIds, this._prevIndexPatternIds)) {


This is an unrelated fix for an issue where timing of getIndexPatterns response are out of order, resulting in setting state to a stale value.

…-ref HEAD~1..HEAD --fix'

…-fix'

nreese · 2023-03-28T12:45:23Z

@elasticmachine merge upstream

nreese · 2023-03-28T13:51:36Z

@elasticmachine merge upstream

elasticmachine · 2023-03-28T17:29:52Z

Pinging @elastic/kibana-presentation (Team:Presentation)

nickpeihl

lgtm! code review and tested with and without courier:ignoreFilterIfFieldNotInIndex enabled.

It is not possible to specify a data view using the Edit filter option in the filter pill context menu, however this is an issue in main and not related to this PR.

nreese · 2023-03-29T20:53:59Z

@elasticmachine merge upstream

kibana-ci · 2023-03-29T22:34:45Z

💚 Build Succeeded

Buildkite Build
Commit: 7a22c03

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`maps`	2.7MB	2.7MB	-213.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	403.8KB	403.6KB	-192.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💛 Build #116079 was flaky be42f1f
💔 Build #116036 failed 98a30dd
💔 Build #115883 failed be3f57a
💔 Build #115880 failed f251950
💔 Build #115853 failed 413956e
💔 Build #115846 failed 48661e5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

stratoula

Changes LGTM!

…phic filters no longer working (#153816) Fixes #153595 ### Background When advanced setting `courier:ignoreFilterIfFieldNotInIndex` is enabled, filters are ignored when data view does not contain the filtering field. The logic on how this works is captured below for context. https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/from_filters.ts#L83 ``` .filter((filter) => { const indexPattern = findIndexPattern(inputDataViews, filter.meta?.index); return !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern); }) ``` https://github.com/elastic/kibana/blob/main/packages/kbn-es-query/src/es_query/filter_matches_index.ts#L20 ``` export function filterMatchesIndex(filter: Filter, indexPattern?: DataViewBase | null) { if (!filter.meta?.key || !indexPattern) { return true; } // Fixes #89878 // Custom filters may refer multiple fields. Validate the index id only. if (filter.meta?.type === 'custom') { return filter.meta.index === indexPattern.id; } return indexPattern.fields.some((field) => field.name === filter.meta.key); } ``` The problem is that [mapSpatialFilter](https://github.com/elastic/kibana/blob/8.7/src/plugins/data/public/query/filter_manager/lib/mappers/map_spatial_filter.ts#L12) is setting `meta.key` property. This causes `filterMatchesIndex` check to fail for spatial filters. Spatial filters are designed to work across data views and avoid the problems that `courier:ignoreFilterIfFieldNotInIndex` solves. As such, spatial filters should not set `meta.key` property and not get removed when `courier:ignoreFilterIfFieldNotInIndex` is enabled. ### Test * set advanced setting `courier:ignoreFilterIfFieldNotInIndex` and refresh browser * install 2 or more sample data sets * create a new map * add documents layer from one sample data set * add documents layer from another sample data set * create spatial filter on map, https://www.elastic.co/guide/en/kibana/8.6/maps-create-filter-from-map.html#maps-spatial-filters * Verify filter is applied to both layers --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 4504cd1)

kibanamachine · 2023-03-31T14:03:26Z

💚 All backports created successfully

Status	Branch	Result
✅	8.7

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

…geographic filters no longer working (#153816) (#154162) # Backport This will backport the following commits from `main` to `8.7`: - [[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working (#153816)](#153816)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nathan Reese <reese.nathan@elastic.co>

…filter disappear from map (#154087) Fixes #107044 and #109340 ### Background In Maps, users can [create spatial filters](https://www.elastic.co/guide/en/kibana/current/maps-create-filter-from-map.html). The filter is added to the Filter bar. <img width="300" alt="Screen Shot 2021-09-03 at 4 07 33 PM" src="https://user-images.githubusercontent.com/373691/133142628-9b03dec2-69ee-4c7c-ac8d-841168c88a94.png"> There are 2 important properties about spatial filters created from a Map: * `filter.meta.type` is set to `spatial_filter`. Maps uses `filter.meta.type` to identify spatial filters. Spatial filters are displayed on the map to provide a visual reference of all applied spatial filters. * `filter.meta.isMultiIndex` is set to true since Elasticsearch Query DSL for the filter is not tied to a single index pattern or field. The Elasticsearch Query DSL is generated in such a way that the filter works for all map layers, regardless of index pattern or spatial field. In the screenshot above a single filter is filtering both layers, even though they are from different index patterns and different spatial fields. * Each layer identifies one or more geo_point or geo_shape fields that is used to display features for that layer * The Elasticsearch Query DSL combines these into a `bool.should` clause where each field provides a `bool.must` clause ensuring the spatial field exists and that the document satisfies the spatial filter. Any document matching any of the `bool.must` clauses allows the bool.should clause to produce a match. ### Solution [Earlier solution](#111897) proposed Sept 2021. This original effort was abandoned as priorities shifted. I investigated re-opening original solution, which included a filter operator registry. This solution is no longer appropriate with `combined` filters. Instead, this PR takes a less complex approach and when `filter.meta.isMultiIndex`, the filter editor displays the following: * Only displays "custom" editor. "Values" editor is not available * "Edit as filter values" toggle button is not visible * Data view select is not visible * onSubmit does not change filter to "custom" unless DSL is edited <img width="500" alt="Screen Shot 2023-03-30 at 10 33 58 AM" src="https://user-images.githubusercontent.com/373691/228918145-61ddda8b-43a3-44e8-8316-a803473f5a36.png"> ### Test instructions * Install web logs sample data set and open "[Logs] Total Requests and Bytes" map * Click wrench and select "Draw bounds to filter data". Draw bounds and create a filter. * In the Filter bar, Click "Edit filter". * Change the label and click save. The filter should still be displayed on the map and the filter pill should have a new label. <strike>Note for testing, until #153816 is merged, spatial filters with a single geofield will not set `isMultiIndex` to true and will still have the errors referenced in the issues. Once #153816 is merged all spatial filters will set isMultiIndex.</strike> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

nreese added 3 commits March 27, 2023 15:10

[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geogra…

1b9fd49

…phic filters no longer working

clean up

7186fb3

simplify test

8a57941

nreese commented Mar 27, 2023

View reviewed changes

kibanamachine and others added 4 commits March 27, 2023 21:18

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

48661e5

…-ref HEAD~1..HEAD --fix'

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

413956e

…-fix'

tslint

f251950

update jest expects

be3f57a

nreese added v8.8.0 v8.7.1 labels Mar 28, 2023

Merge branch 'main' into issue_153595

98a30dd

Merge branch 'main' into issue_153595

be42f1f

nreese marked this pull request as ready for review March 28, 2023 17:29

nreese requested review from a team as code owners March 28, 2023 17:29

nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// auto-backport Deprecated - use backport:version if exact versions are needed Feature:Maps labels Mar 28, 2023

nreese added the release_note:fix label Mar 28, 2023

nickpeihl approved these changes Mar 28, 2023

View reviewed changes

Merge branch 'main' into issue_153595

7a22c03

lukasolson approved these changes Mar 29, 2023

View reviewed changes

nreese mentioned this pull request Mar 31, 2023

[unified search] fix Changing label of a geospatial filter on causes filter disappear from map #154087

Merged

stratoula approved these changes Mar 31, 2023

View reviewed changes

nreese merged commit 4504cd1 into elastic:main Mar 31, 2023

kibanamachine mentioned this pull request Mar 31, 2023

[8.7] [maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working (#153816) #154162

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working #153816

[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working #153816

Uh oh!

nreese commented Mar 27, 2023 •

edited

Loading

Uh oh!

nreese Mar 27, 2023 •

edited

Loading

Uh oh!

nreese commented Mar 28, 2023

Uh oh!

nreese commented Mar 28, 2023

Uh oh!

elasticmachine commented Mar 28, 2023

Uh oh!

nickpeihl left a comment

Uh oh!

nreese commented Mar 29, 2023

Uh oh!

kibana-ci commented Mar 29, 2023

ESLint disabled line counts

Total ESLint disabled count

Uh oh!

stratoula left a comment

Uh oh!

kibanamachine commented Mar 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working #153816

[maps] fix When courier:ignoreFilterIfFieldNotInIndex enabled, geographic filters no longer working #153816

Uh oh!

Conversation

nreese commented Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Test

Uh oh!

nreese Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nreese commented Mar 28, 2023

Uh oh!

nreese commented Mar 28, 2023

Uh oh!

elasticmachine commented Mar 28, 2023

Uh oh!

nickpeihl left a comment

Choose a reason for hiding this comment

Uh oh!

nreese commented Mar 29, 2023

Uh oh!

kibana-ci commented Mar 29, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 31, 2023

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nreese commented Mar 27, 2023 •

edited

Loading

nreese Mar 27, 2023 •

edited

Loading