[Discover] Extend DiscoverNoResults component to show different message on error #79671

kertal · 2020-10-06T14:15:28Z

Summary

With the alignment of error notification in PR #77788 all error messages in Discover were migrated to be a toast. This could be misleading when there's an error and the no-results page is displayed:

This PR displays a different message in the error case

Furthermore this PR converts DiscoverNoResults to TypeScript, modularizes the component, migrates testing to no longer use snapshots of the whole component, and moves the whole component out of the angular folder.

Testing

Add an index pattern containing a invalid painless script by importing the saved object in management export.ndjson.zip
This adds the logs* index pattern. Add data to the logs index

PUT log/_doc/1
{
  "@timestamp":"2020-10-14T07:07:29.952Z",
  "message": "test"
}

Navigate to Discover, select the logs index pattern, adapt time range
Submit and then you should see the error message

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
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

kertal · 2020-10-06T15:49:57Z

@elasticmachine merge upstream

…rror-message

kertal · 2020-10-07T08:00:22Z

@elasticmachine merge upstream

…rror-message

…-06-discover-add-static-error-message

… of github.com:kertal/kibana into kertal-pr-2020-10-06-discover-add-static-error-message

src/plugins/discover/public/application/angular/directives/no_results.tsx

elasticmachine · 2020-10-15T13:03:42Z

Pinging @elastic/kibana-app (Team:KibanaApp)

kertal · 2020-10-15T13:04:00Z

@elasticmachine merge upstream

…rror-message

… of github.com:kertal/kibana into kertal-pr-2020-10-06-discover-add-static-error-message

gchaps · 2020-10-15T15:54:26Z

How about:

We encountered an error retrieving your search results
Or
We encountered an error retrieving search results

Button: Show error message

kertal · 2020-10-16T14:14:45Z

How about:

We encountered an error retrieving your search results
Or
We encountered an error retrieving search results

Button: Show error message
@gchaps thx, this is much better wording, done 👍

lizozom · 2020-10-16T15:46:22Z

The cluster is on fire! :-D

kertal · 2020-10-19T06:31:43Z

@elasticmachine merge upstream

kertal · 2020-10-19T08:50:10Z

@elasticmachine merge upstream

…rror-message

majagrubic

Tested this in Chrome on Mac OS according to instructions. Works as expected. A few code comments below.

src/plugins/discover/public/application/angular/directives/no_results.tsx

kertal · 2020-10-19T09:41:25Z

Tested this in Chrome on Mac OS according to instructions. Works as expected. A few code comments below.

@majagrubic thx, yes, makes sense to clean it up a bit further when touching this code. will adapt!

src/plugins/discover/public/application/angular/directives/no_results.tsx

lizozom · 2020-10-19T13:07:24Z

src/plugins/discover/public/application/angular/discover.js

@@ -814,7 +816,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
        if (error instanceof Error && error.name === 'AbortError') return;

        $scope.fetchStatus = fetchStatuses.NO_RESULTS;
-        $scope.rows = [];
+        $scope.fetchError = error;

        data.search.showError(error);


There's an annoying problem here, that if the error is a SearchTimeoutError - it's won't show again.
I'm not sure it's worth fixing, but at least it might be worth documenting.

It's just displayed once in a session?

@lizozom but the error is displayed in the toast, right?

Yeah, but that popover will be displayed only once. Since you're re-calling shower to, it doesn't get shown. Not sure if I have a good proposition for fixing it, but I don't think it's critical.

I see, so the replay doesn't work, also think it's not critical, the button is an additional option if you have missed the toast. would be interesting why it's just callable once.

lizozom

Tested with all types of errors I have stored.
LGTM.

…-06-discover-add-static-error-message

majagrubic

Looks a lot nicer!

andreadelrio

I think this is better suited for a danger callout/button since it is an error.

src/plugins/discover/public/application/components/no_results/no_results.tsx

…no_results.tsx Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

andreadelrio

Design changes LGTM

kibanamachine · 2020-10-21T08:43:46Z

💚 Build Succeeded

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

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`discover`	243	254	+11

async chunks size

id	before	after	diff
`discover`	439.3KB	436.4KB	-2.9KB

History

💔 Build #82985 failed 6e7256d
💚 Build #82688 succeeded b0215d8
💚 Build #82366 succeeded 3b33603
💚 Build #82135 succeeded 23db6e0
💚 Build #81863 succeeded 3728c23

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

…ge on error (elastic#79671) Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

…arm-phase-to-formlib * 'master' of github.com:elastic/kibana: (55 commits) [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106) [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082) [DOCS] Add link for monitoring ssl settings (elastic#81057) [test] Await loading indicator in monitoring test (elastic#81279) [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512) [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287) [Discover] Deangularize context.app (elastic#80851) [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873) [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671) Fix tagcloud word overlapping (elastic#81161) [Security Solution] Fixes flaky test rules (elastic#81040) Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183) [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301) Add namespaced version of the API call (elastic#81278) [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123) [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236) [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084) [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239) skip flaky suite (elastic#81264) [Maps] fix top-level Map page is called 'Kibana' (elastic#81238) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx

…ge on error (#79671) (#81310) Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

Extend DiscoverNoResults component to show different message on error

f26dbf4

kertal added the Feature:Discover Discover Application label Oct 6, 2020

kertal self-assigned this Oct 6, 2020

Merge branch 'master' into kertal-pr-2020-10-06-discover-add-static-e…

e65a9af

…rror-message

Merge branch 'master' into kertal-pr-2020-10-06-discover-add-static-e…

c8c4434

…rror-message

kertal added the v7.11.0 label Oct 8, 2020

kertal added 2 commits October 15, 2020 14:35

Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…

30f39fa

…-06-discover-add-static-error-message

Merge branch 'kertal-pr-2020-10-06-discover-add-static-error-message'…

a27559d

… of github.com:kertal/kibana into kertal-pr-2020-10-06-discover-add-static-error-message

kertal marked this pull request as ready for review October 15, 2020 13:00

kertal requested a review from a team October 15, 2020 13:00

kertal commented Oct 15, 2020

View reviewed changes

src/plugins/discover/public/application/angular/directives/no_results.tsx Outdated Show resolved Hide resolved

kertal requested a review from majagrubic October 15, 2020 13:02

kertal added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Oct 15, 2020

Merge branch 'master' into kertal-pr-2020-10-06-discover-add-static-e…

f6a51b3

…rror-message

kertal added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Oct 15, 2020

kertal requested a review from lizozom October 15, 2020 13:04

Merge branch 'kertal-pr-2020-10-06-discover-add-static-error-message'…

3728c23

… of github.com:kertal/kibana into kertal-pr-2020-10-06-discover-add-static-error-message

Address review comments

23db6e0

Merge branch 'master' into kertal-pr-2020-10-06-discover-add-static-e…

3b33603

…rror-message

majagrubic reviewed Oct 19, 2020

View reviewed changes

lizozom reviewed Oct 19, 2020

View reviewed changes

src/plugins/discover/public/application/angular/directives/no_results.tsx Outdated Show resolved Hide resolved

lizozom reviewed Oct 19, 2020

View reviewed changes

lizozom approved these changes Oct 19, 2020

View reviewed changes

kertal added 3 commits October 19, 2020 21:12

Improve code, refactor to helper file

786cc34

Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…

3cca99e

…-06-discover-add-static-error-message

Refactor improving everything

b0215d8

kertal requested a review from majagrubic October 20, 2020 08:18

majagrubic approved these changes Oct 20, 2020

View reviewed changes

kertal requested review from a team and andreadelrio October 20, 2020 15:00

andreadelrio requested changes Oct 21, 2020

View reviewed changes

src/plugins/discover/public/application/components/no_results/no_results.tsx Outdated Show resolved Hide resolved

src/plugins/discover/public/application/components/no_results/no_results.tsx Outdated Show resolved Hide resolved

kertal and others added 2 commits October 21, 2020 07:09

Update src/plugins/discover/public/application/components/no_results/…

94d346e

…no_results.tsx Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

Update src/plugins/discover/public/application/components/no_results/…

6e7256d

…no_results.tsx Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

andreadelrio self-requested a review October 21, 2020 05:25

andreadelrio approved these changes Oct 21, 2020

View reviewed changes

Satisfy linting

b07f58d

kertal merged commit 3a969f4 into elastic:master Oct 21, 2020

kertal mentioned this pull request Oct 21, 2020

[7.x] [Discover] Extend DiscoverNoResults component to show different message on error (#79671) #81310

Merged

kertal added a commit to kertal/kibana that referenced this pull request Oct 21, 2020

[Discover] Extend DiscoverNoResults component to show different messa…

3ef161c

…ge on error (elastic#79671) Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

kertal added a commit that referenced this pull request Oct 21, 2020

[Discover] Extend DiscoverNoResults component to show different messa…

687f96c

…ge on error (#79671) (#81310) Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Extend DiscoverNoResults component to show different message on error #79671

[Discover] Extend DiscoverNoResults component to show different message on error #79671

kertal commented Oct 6, 2020 •

edited

Loading

kertal commented Oct 6, 2020

kertal commented Oct 7, 2020

elasticmachine commented Oct 15, 2020

kertal commented Oct 15, 2020

gchaps commented Oct 15, 2020

kertal commented Oct 16, 2020

lizozom commented Oct 16, 2020

kertal commented Oct 19, 2020

kertal commented Oct 19, 2020

majagrubic left a comment

kertal commented Oct 19, 2020

lizozom Oct 19, 2020

kertal Oct 20, 2020

kertal Oct 20, 2020 •

edited

Loading

lizozom Oct 21, 2020

kertal Oct 21, 2020

lizozom left a comment

majagrubic left a comment

andreadelrio left a comment

andreadelrio left a comment

kibanamachine commented Oct 21, 2020

[Discover] Extend DiscoverNoResults component to show different message on error #79671

[Discover] Extend DiscoverNoResults component to show different message on error #79671

Conversation

kertal commented Oct 6, 2020 • edited Loading

Summary

Testing

Checklist

kertal commented Oct 6, 2020

kertal commented Oct 7, 2020

elasticmachine commented Oct 15, 2020

kertal commented Oct 15, 2020

gchaps commented Oct 15, 2020

kertal commented Oct 16, 2020

lizozom commented Oct 16, 2020

kertal commented Oct 19, 2020

kertal commented Oct 19, 2020

majagrubic left a comment

Choose a reason for hiding this comment

kertal commented Oct 19, 2020

lizozom Oct 19, 2020

Choose a reason for hiding this comment

kertal Oct 20, 2020

Choose a reason for hiding this comment

kertal Oct 20, 2020 • edited Loading

Choose a reason for hiding this comment

lizozom Oct 21, 2020

Choose a reason for hiding this comment

kertal Oct 21, 2020

Choose a reason for hiding this comment

lizozom left a comment

Choose a reason for hiding this comment

majagrubic left a comment

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 21, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

History

kertal commented Oct 6, 2020 •

edited

Loading

kertal Oct 20, 2020 •

edited

Loading