Show error if field is not found during filter rendering #59298

lizozom · 2020-03-04T11:48:11Z

Summary

Fixes #57428 by adding a filter errored 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

elasticmachine · 2020-03-04T11:48:14Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

lizozom · 2020-03-04T11:53:27Z

@elasticmachine merge upstream

src/plugins/data/common/es_query/filters/get_display_value.ts

lukasolson

LGTM, added a couple of minor comments

flash1293

I'm not super happy with this solution. The filter will still be there in the saved object (or wherever it's coming from) but it won't be rendered anymore in the UI, so the user can't really remove it (e.g. if the broken filter is part of a visualization). In the end you have a broken visualization which isn't showing any data (because the filter is still in the request) and will produce tons of warnings every time you interact with it - so you still have to start from scratch building it. Of course there are ways to recover it (editing the saved object, deleting all filters with the cogwheel popover menu, editing the url), but the average user won't be able to achieve this.

I'm just realizing this is probably my bad because of a badly worded issue:

Expected behavior:

The filter pill should either get filtered out or be rendered in an error state.

With filtered out I meant getting filtered out completely (ie not showing up in the saved object as well as the search request anymore).

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

@flash1293 thanks for the review.
I'll try rendering it in an errored state

…x-57428

cchaos

Thanks for the ping @lizozom, My initial thoughts are that the dark red background can get confused with the old "excluded" version of the filter pills. So my suggestion is that we need a new class for inValid state on the FilterView in which it looks disabled (so not being included in filters) but with some extra styles like this:

I can pass you up a PR later today.

lizozom · 2020-03-05T13:31:37Z

@elasticmachine merge upstream

lizozom · 2020-03-05T13:45:25Z

@cchaos I agree that your design is much clearer!

…x-57428

cchaos

Styles look good to me ;)

flash1293

Tested and works as expected, LGTM.

Left two suggestions for error messages, but nothing important.

src/plugins/data/common/es_query/filters/get_display_value.ts

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

kibanamachine · 2020-03-06T19:45:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2cb73ee

History

💚 Build #31212 succeeded 75451fb
💚 Build #31070 succeeded e7bc359
💔 Build #31040 failed 3783733
💚 Build #30715 succeeded d51c3e9

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

* Show error if field is not found * Errored filter state * Design adjustments * Fixing class names and making look similar to disabled * code review fixes Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]>

…9610) * Show error if field is not found * Errored filter state * Design adjustments * Fixing class names and making look similar to disabled * code review fixes Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]>

* master: (154 commits) Add an optional authentication mode for HTTP resources (elastic#58589) Implement embeddable drilldown menu options (elastic#59232) [Alerting] "Create alert" graph visualization design improvements (elastic#59399) Alerting update route throttle property is missing (elastic#59580) [SIEM] Adds 'Load prebuilt rules' Cypress test (elastic#59529) Show error if field is not found during filter rendering (elastic#59298) Navigate back to discover app during test, because the saved search from the preceding test has major performance problems when used with this test (elastic#59571) Check for alert dialog when doing a force logout (elastic#59329) ensure fs deletes are not cwd dependent (elastic#59570) Empty message for APM service map (elastic#59518) [Drilldowns] <ActionWizard/> Component (elastic#59032) [Reporting] Improve the page exit error messages (elastic#59351) Ensure logged out starting state for tests that need it (elastic#59322) Hide input value from kbn-config-schema error messages (elastic#58843) [ML] Transforms: Migrate client plugin to NP. (elastic#59443) [ML] Disable failing functional tests [SIEM] Update Timeline to use the latest euiFlyoutBody style (elastic#59524) Temporarily remove the project mappings for PR labels (elastic#59493) [Alerting] replace index threshold graph usage of watcher APIs with new API (elastic#59385) [ML] Show view series link in anomalies table for machine_learning_user role (elastic#59549) ...

* Show error if field is not found * Errored filter state * Design adjustments * Fixing class names and making look similar to disabled * code review fixes Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: cchaos <[email protected]> # Conflicts: # src/plugins/data/public/ui/filter_bar/filter_editor/lib/filter_label.tsx # src/plugins/data/public/ui/filter_bar/filter_item.tsx

Show error if field is not found

43291be

lizozom added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 4, 2020

lizozom requested a review from flash1293 March 4, 2020 11:48

lizozom requested a review from a team as a code owner March 4, 2020 11:48

lizozom self-assigned this Mar 4, 2020

Merge branch 'master' into bug-fix-57428

d51c3e9

lizozom mentioned this pull request Mar 4, 2020

Label and filter problems in dashboards after upgrading several times from 4.5.x/5.6.x to 7.4.x #58196

Closed

lukasolson reviewed Mar 4, 2020

View reviewed changes

src/plugins/data/common/es_query/filters/get_display_value.ts Outdated Show resolved Hide resolved

lukasolson reviewed Mar 4, 2020

View reviewed changes

src/plugins/data/common/es_query/filters/get_display_value.ts Outdated Show resolved Hide resolved

lukasolson approved these changes Mar 4, 2020

View reviewed changes

flash1293 reviewed Mar 4, 2020

View reviewed changes

Liza K added 3 commits March 5, 2020 12:25

Merge branch 'master' of github.com:elastic/kibana into bug-fix-57428

073324b

Errored filter state

4caccef

Merge branch 'bug-fix-57428' of github.com:lizozom/kibana into bug-fi…

3783733

…x-57428

lizozom requested a review from cchaos March 5, 2020 11:07

cchaos reviewed Mar 5, 2020

View reviewed changes

Merge branch 'master' into bug-fix-57428

e7bc359

Liza K added 2 commits March 5, 2020 18:42

Merge branch 'master' of github.com:elastic/kibana into bug-fix-57428

fa71ced

Design adjustments

6dd3ae1

Liza K and others added 2 commits March 5, 2020 19:19

Merge branch 'bug-fix-57428' of github.com:lizozom/kibana into bug-fi…

91b9547

…x-57428

Fixing class names and making look similar to disabled

75451fb

lizozom requested a review from a team as a code owner March 5, 2020 19:24

cchaos approved these changes Mar 5, 2020

View reviewed changes

flash1293 approved these changes Mar 6, 2020

View reviewed changes

src/plugins/data/common/es_query/filters/get_display_value.ts Outdated Show resolved Hide resolved

src/plugins/data/public/ui/filter_bar/filter_item.tsx Outdated Show resolved Hide resolved

Liza K added 2 commits March 6, 2020 20:13

code review fixes

56cbf58

Merge remote-tracking branch 'upstream/master' into bug-fix-57428

2cb73ee

lizozom added the v7.6.2 label Mar 6, 2020

lizozom merged commit 5db1b0a into elastic:master Mar 6, 2020

lizozom mentioned this pull request Mar 7, 2020

[7.x] Show error if field is not found during filter rendering (#59298) #59610

Merged

lizozom mentioned this pull request Apr 1, 2020

[Dashboard] Filter bar disappears as a result of an exception #59411

Closed

lizozom removed the v7.6.2 label May 19, 2020

lizozom mentioned this pull request May 19, 2020

[7.6] Show error if field is not found during filter rendering (#59298) #66982

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show error if field is not found during filter rendering #59298

Show error if field is not found during filter rendering #59298

lizozom commented Mar 4, 2020 •

edited

Loading

elasticmachine commented Mar 4, 2020

lizozom commented Mar 4, 2020

lukasolson left a comment

flash1293 left a comment

lizozom commented Mar 5, 2020

cchaos left a comment

lizozom commented Mar 5, 2020

lizozom commented Mar 5, 2020

cchaos left a comment

flash1293 left a comment

kibanamachine commented Mar 6, 2020

Show error if field is not found during filter rendering #59298

Show error if field is not found during filter rendering #59298

Conversation

lizozom commented Mar 4, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 4, 2020

lizozom commented Mar 4, 2020

lukasolson left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

lizozom commented Mar 5, 2020

cchaos left a comment

Choose a reason for hiding this comment

lizozom commented Mar 5, 2020

lizozom commented Mar 5, 2020

cchaos left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 6, 2020

💚 Build Succeeded

History

lizozom commented Mar 4, 2020 •

edited

Loading