Add error if filter index pattern is missing by lizozom · Pull Request #66979 · elastic/kibana

lizozom · 2020-05-19T11:20:28Z

Summary

Note: Commented out maps test - fix in #64861

See #67177 for full explanation.

Release Notes

This PR adds error and warning statuses to FilterBar filters, according to the following scenarios:

If a filter's index pattern is missing the filter will show in an error state and disable itself.
If a filter's index pattern field is missing the filter will show in an error state and disable itself.
If a filter's uses an unrelated index pattern and the field in that filter does not exist in any of the index patterns used by the app - the filter will show in a warning state. i.e. Trying to apply a filter for the logs data set file extension field to the ecommerce data set dashboard. The filter will not be automatically disabled.
If a filter's uses an unrelated index pattern but the field in that filter exist in at least one of the index patterns used by the app - the filter will show in a normal state. i.e. Trying to apply a filter for the logs data set "name" field to the ecommerce data set dashboard (that also has a name field). If you try to edit this filter, you will still have to re-select the index pattern and re-configure the filter.

Error state:

Warning state:

To reproduce

Error filter

Create an index pattern
Go to discover and create a filter for that index pattern
Save it as a saved query
Delete the index pattern you created
Go to another dashboard and load that saved query

Warning filter

Load two data sets (ecommerce and logs for example).
Go to one of the dashboards and create a filter
Save a Saved Query
Go to the other dashboard and try loading the Saved Query.

The filter should appear in an error state

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
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

lizozom · 2020-05-19T11:47:42Z

src/plugins/data/public/ui/filter_bar/filter_item.tsx

-        }),
-      });
-      valueLabel = this.props.intl.formatMessage({
+      errorMessage = valueLabel = this.props.intl.formatMessage({


Notification was removed as it renders multiple times - showing the error message on the filter is clear enough.

Dosant

Tested using steps from description. Dashboard is displayed and see error in filter 👍

One note: I was trying to fix broken filter from UI, it allowed me to take new existing index pattern, which is great. But is looks it didn't save changes, please see:

I think, if this is a quick fix, it worth fixing?

Also, I'd consider adding a test for this bug.
Maybe even a functional one,
I'd setup it the following way:

Prepare a saved query with broken filter upfront (using API or esarchiver). (just do not click UI for this)
Open any dashboard and apply the saved query
Assert that dashboard is displayed

docs/development/plugins/data/public/kibana-plugin-plugins-data-public.baseformatterspublic.md

Dosant · 2020-05-19T13:16:49Z

FYI, when tested noticed regression in master: #67003

Dosant · 2020-05-19T13:36:48Z

I tried to reproduce "Empty/broken dashboard" in master using the steps from the description, but I still can see the dashboard. Just filter isn't working.

Is there a way to get into state with empty dashboard? or it is not possible in Master?

…ndex-pattern-filter

lizozom · 2020-05-20T11:48:42Z

@Dosant I suspect that the reason you didn't see it update is because of a problem with the build system at the moment (it seems that plugins are built, but they are actually still building for a few mins after kibana seems to start).

Also working on adding the test you requested and fixing the broken test as well.

…/kibana into bug/missing-index-pattern-filter

lizozom · 2020-05-21T16:01:58Z

src/plugins/data/public/index_patterns/index_patterns/index_patterns.ts

    ).savedObjects;
  }

+  getAll = async (refresh: boolean = false) => {


@mattkime what do you think of this?

lizozom · 2020-05-21T16:02:52Z

src/plugins/data/public/ui/filter_bar/filter_bar.tsx

+     * Filters may be applied from an index pattern other than the active index patterns list provided.
+     * Retrieve the actual index pattern from the filter for validation.
+     */
+    getIndexPatterns()


Get index patterns once in filter bar and pass them to each FilterItem

lizozom · 2020-05-21T16:03:43Z

src/plugins/data/public/ui/filter_bar/filter_item.tsx

-      this.props.filter.meta.disabled ? 'disabled' : 'enabled'
-    }`;
-    const dataTestSubjPinned = `filter-${isFilterPinned(filter) ? 'pinned' : 'unpinned'}`;
+  private isValidLabel(labelConfig: LabelOptions) {


I broke down the logic in this component to helper functions for easier debugging and reviewing

lizozom · 2020-06-02T15:34:01Z

@Dosant validated that 7.8 works with the new data.json.gz file

lizozom · 2020-06-02T15:34:06Z

@elasticmachine merge upstream

kibanamachine · 2020-06-02T17:22:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f25a7fb

History

💚 Build #51025 succeeded da75691
💚 Build #50809 succeeded 10a3441
💚 Build #50727 succeeded 0bc2fbd
💔 Build #50269 failed 7cd51cb
💔 Build #50268 failed cccae76

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

* add error if filter index pattern is gone * docs change - why? * Fix i18n * Added a functional test for broken filters (field + index pattern) * Clarify readme * Moved readme * New warning status * Remove getAll * git pull upstream master * Fix translation files * Fix merge * added filterbar texts * disabled correction * Disable check in maps test until elastic#64861 is resolved * Fix tests, warning state is not disabled. * Adjust warning filter - ignore filters from "foreign" index pattern, that are still applicable * Add an additional unrelaeted filter test * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Fixed test data * Revert mapping * Update data with missing test * Update test to match data * Code review Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* add error if filter index pattern is gone * docs change - why? * Fix i18n * Added a functional test for broken filters (field + index pattern) * Clarify readme * Moved readme * New warning status * Remove getAll * git pull upstream master * Fix translation files * Fix merge * added filterbar texts * disabled correction * Disable check in maps test until #64861 is resolved * Fix tests, warning state is not disabled. * Adjust warning filter - ignore filters from "foreign" index pattern, that are still applicable * Add an additional unrelaeted filter test * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Fixed test data * Revert mapping * Update data with missing test * Update test to match data * Code review Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* add error if filter index pattern is gone * docs change - why? * Fix i18n * Added a functional test for broken filters (field + index pattern) * Clarify readme * Moved readme * New warning status * Remove getAll * git pull upstream master * Fix translation files * Fix merge * added filterbar texts * disabled correction * Disable check in maps test until elastic#64861 is resolved * Fix tests, warning state is not disabled. * Adjust warning filter - ignore filters from "foreign" index pattern, that are still applicable * Add an additional unrelaeted filter test * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src/plugins/data/public/ui/filter_bar/_global_filter_item.scss Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Fixed test data * Revert mapping * Update data with missing test * Update test to match data * Code review Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # test/functional/fixtures/es_archiver/dashboard/current/kibana/data.json.gz

kibanamachine · 2020-06-09T19:38:45Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

bhavyarm · 2020-07-14T13:42:10Z

This fix didn't go into 7.8.1

add error if filter index pattern is gone

666aa4b

lizozom requested a review from a team as a code owner May 19, 2020 11:20

lizozom self-assigned this May 19, 2020

lizozom added v7.6.3 v7.7.1 v7.8.0 v8.0.0 bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes labels May 19, 2020

lizozom commented May 19, 2020

View reviewed changes

docs change - why?

21f662e

Dosant reviewed May 19, 2020

View reviewed changes

lizozom commented May 19, 2020

View reviewed changes

docs/development/plugins/data/public/kibana-plugin-plugins-data-public.baseformatterspublic.md Outdated Show resolved Hide resolved

Merge branch 'master' of github.com:elastic/kibana into bug/missing-i…

3e4d0f1

…ndex-pattern-filter

Liza K and others added 4 commits May 20, 2020 14:50

Fix i18n

fceecd1

Added a functional test for broken filters (field + index pattern)

75764a9

Clarify readme

6f9ba3b

Moved readme

a33a037

lizozom removed v7.6.3 v7.7.1 labels May 21, 2020

Liza K added 2 commits May 21, 2020 18:51

New warning status

1018766

Merge branch 'bug/missing-index-pattern-filter' of github.com:lizozom…

45881b7

…/kibana into bug/missing-index-pattern-filter

lizozom requested a review from a team as a code owner May 21, 2020 15:59

lizozom requested a review from Dosant May 21, 2020 16:00

lizozom commented May 21, 2020

View reviewed changes

Merge branch 'master' into bug/missing-index-pattern-filter

f25a7fb

lizozom merged commit fbcb74c into elastic:master Jun 3, 2020

lizozom mentioned this pull request Jun 3, 2020

[7.x] Add error if filter index pattern is missing (#66979) #68050

Merged

This was referenced Jun 4, 2020

[FIX] Allow filters without index #68225

Merged

[7.8] Add error if filter index pattern is missing (#66979) #68350

Closed

patrykkopycinski mentioned this pull request Jun 8, 2020

[Security Solution] Add cypress tests for global search bar #68535

Merged

1 task

bhavyarm added v7.8.1 v7.8.0 and removed v7.8.0 v7.8.1 labels Jun 8, 2020

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 9, 2020

lizozom added v7.8.1 and removed v7.8.0 labels Jun 10, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 11, 2020

lizozom mentioned this pull request Jul 5, 2020

Stale index pattern references in dashboard filters #43347

Closed

Dosant mentioned this pull request Jul 13, 2020

Vega is missing filters bar when initially loaded #62119

Closed

bhavyarm added v7.9.0 and removed v7.8.1 labels Jul 14, 2020

This was referenced Jul 21, 2020

[Dashboard][Filters] Warning filters in non default index pattern dashboard #72601

Closed

fix dashboard index pattern race condition #72899

Merged

Flickering warning filters state on a dashboard #73286

Closed

Conversation

lizozom commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release Notes

To reproduce

Error filter

Warning filter

Checklist

For maintainers

Uh oh!

lizozom May 19, 2020

Choose a reason for hiding this comment

Uh oh!

Dosant left a comment • 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!

Uh oh!

Dosant commented May 19, 2020

Uh oh!

Dosant commented May 19, 2020

Uh oh!

lizozom commented May 20, 2020

Uh oh!

lizozom May 21, 2020

Choose a reason for hiding this comment

Uh oh!

lizozom May 21, 2020

Choose a reason for hiding this comment

Uh oh!

lizozom May 21, 2020

Choose a reason for hiding this comment

Uh oh!

lizozom commented Jun 2, 2020

Uh oh!

lizozom commented Jun 2, 2020

Uh oh!

kibanamachine commented Jun 2, 2020

💚 Build Succeeded

History

Uh oh!

kibanamachine commented Jun 9, 2020

Uh oh!

bhavyarm commented Jul 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lizozom commented May 19, 2020 •

edited

Loading

Dosant left a comment •

edited

Loading