[Discover] Inline shard failures warnings #161271

jughosta · 2023-07-05T15:05:32Z

Closes [Discover] Inline shard error warnings #155216

Summary

This PR replaces shard falures toasts with inline warnings in Discover.

Intercept shard failures in Discover main app
Show inline warnings above the grid instead
Handle NoResultsFound case too
Implement for Discover context app
Implement for saved search embeddable on Dashboard
Can we inline timeouts too?
Check SQL view
Add tests

Discover view with shard failures

Discover view with shard failures (and no results)

Dashboard view with shard failures

Surrounding documents view with shard failures

Discover view with timeouts

Dashboard view with timeouts

Surrounding documents view with timeouts

Testing

For testing please uncomment

kibana/src/plugins/data/common/search/search_source/search_source.ts

Line 922 in 3f102cf

// body.query = {

or

kibana/src/plugins/data/common/search/search_source/search_source.ts

Line 547 in 3f102cf

// response.rawResponse.timed_out = true;

and switch to kibana* data view.

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
Unit or functional tests were updated or added to match the most common scenarios
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

…-error

jughosta · 2023-08-02T09:58:34Z

Thanks @andreadelrio!

1

The button showing the error shouldn't be overlapping the panels' content. We should show it below in a separate row

Updated the positioning of the badge:

2

Remove gray border this empty prompt

You mean to make it look like the following? Might be not enough contrast. Is there anything else we can use from eui for this case? It's when 0 hits returned and there were some shards failures.

3

Can we group duplicated messages?

Yes, they are grouped based on title and warnings content. The ones from the screenshot have the same title but different details inside.

4

Why the bolt icon? what is it trying to convey and how does it related to the message?

This part I didn't touch in the PR. I guess the idea was to bring users attention to the fact that they reached the earliest or the latest document and there is nothing else to load. Would you suggest to change this callout?

kertal · 2023-08-02T10:22:55Z

4

Why the bolt icon? what is it trying to convey and how does it related to the message?

This part I didn't touch in the PR. I guess the idea was to bring users attention to the fact that they reached the earliest or the latest document and there is nothing else to load. Would you suggest to change this callout?

That's legacy. There could be an info icon + the matching color for that callout ... because I don't think this needs to be a kind of warning.

…-error # Conflicts: # src/plugins/discover/public/embeddable/saved_search_embeddable.tsx # src/plugins/discover/tsconfig.json

andreadelrio · 2023-08-02T17:40:25Z

Thanks @jughosta for making the updates.

2

You mean to make it look like the following? Might be not enough contrast. Is there anything else we can use from eui for this case? It's when 0 hits returned and there were some shards failures.

Yes. I don't think the border does a lot to improve contrast tbh and no-border is the recommended pattern in EUI. See https://elastic.github.io/eui/#/display/empty-prompt/guidelines

3

Yes, they are grouped based on title and warnings content. The ones from the screenshot have the same title but different details inside.

Makes sense, thanks

4

This part I didn't touch in the PR. I guess the idea was to bring users attention to the fact that they reached the earliest or the latest document and there is nothing else to load. Would you suggest to change this callout?

Can you update following Matthias' suggestion and send a screenshot? So a callout in color primary. We also need a bit of spacing above this banner (so more spacing below the Load 5 newer documents line).

…-error

jughosta · 2023-08-03T11:56:07Z

@andreadelrio

2

Yes. I don't think the border does a lot to improve contrast tbh and no-border is the recommended pattern in EUI. See https://elastic.github.io/eui/#/display/empty-prompt/guidelines

Thanks!

From eui example:

Based on these guidelines, an icon should be at the top then, no?

Should we also update the icon position for Discover error view? Currently it looks like this:

4

Can you update following Matthias' suggestion and send a screenshot? So a callout in color primary. We also need a bit of spacing above this banner (so more spacing below the Load 5 newer documents line).

Updated the old callout:

andreadelrio · 2023-08-03T20:32:40Z

From eui example:

Based on these guidelines, an icon should be at the top then, no?

Should we also update the icon position for Discover error view? Currently it looks like this:

Yes, I think this makes sense.

…-error

jughosta · 2023-08-04T08:40:28Z

Updated:

@davismcphee @andreadelrio It's ready for review.

andreadelrio

Design changes LGTM. Thanks Julia!

davismcphee

Tested locally again and the latest changes LGTM 👍 Thanks for updating the error callout too!

kertal · 2023-08-10T09:40:26Z

@andreadelrio I've got a theory why the bold was being used, maybe Harry Potter accidentally applied a spell creating a PR to Kibana and used a bolt similar to his scar to claim ownership?

kibana-ci · 2023-08-10T10:29:36Z

💚 Build Succeeded

Buildkite Build
Commit: 2e3731e

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	608	613	+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/search-response-warnings`	-	8	+8

Async chunks

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

id	before	after	diff
`discover`	549.1KB	556.3KB	+7.2KB
`unifiedHistogram`	46.7KB	46.7KB	+3.0B
total			+7.2KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/search-response-warnings`	-	16	+16

History

💚 Build #148367 succeeded ed630be
💚 Build #148196 succeeded 4c0358a
💚 Build #147431 succeeded 7a3331f
💚 Build #147114 succeeded da0bd76
💚 Build #147049 succeeded b7dd1ef
💚 Build #146813 succeeded fb6eafe

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

cc @jughosta

* main: (108 commits) [Telemetry Schema Validation] Allow `null` on `string` (elastic#163499) [Search] Add Slack and Gmail connectors (elastic#163321) [ML] Provide hints for empty fields in dropdown options in Anomaly detection & Transform creation wizards, Change point detection view (elastic#163371) chore(slo): Add response required fields (elastic#163430) [AO] Fix add_to_case functional test (elastic#163155) unskip license type functional test (elastic#163199) fix(NA): yarn env vars for node_modules mirrors (elastic#163549) [Response Ops][Task Manager] Expose SLI metrics in HTTP API (elastic#162178) [Logs UI] Adapt test to ES highlighting changes and unskip (elastic#163592) [Infra UI] Implement Telemetry on 'Show' buttons within Inventory (elastic#163587) [Enterprise Search]Migrate all usages of EuiPage*_Deprecated (elastic#163482) fix(slo): settings and access for serverless (elastic#163514) [Infra UI] Implement telemetry for the asset details flyout (elastic#163078) [Fleet] Add a banner to the top of the Kafka Output UI to say that Elastic Defend integration is not supported (elastic#163579) [Fleet] Re-enable and fix Fleet policy secret integration tests (elastic#163428) [Fleet] add managed to imported saved object (elastic#163526) [Index Management] Disable index actions using contextRef (elastic#163475) [Discover] Inline shard failures warnings (elastic#161271) [Security Solution][Detection engine] skips geo_point non-ecs validation (elastic#163487) Update EUI layout components in bfetch example plugin (elastic#163490) ...

[Discover] Intercept shard failures

eab86d5

jughosta added release_note:enhancement backport:skip This commit does not require backporting Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) labels Jul 5, 2023

jughosta self-assigned this Jul 5, 2023

jughosta changed the title ~~[Discover] Intercept shard failures~~ [Discover] Inline shard failures warnings Jul 5, 2023

jughosta added 24 commits July 5, 2023 17:33

[Discover] Update deps

bfd44e3

[Discover] Add a todo

38e17fc

[Discover] Rearrange the code

86e7e16

[Discover] Revert changes

1af6f16

[Discover] Render warnings

d7b95ad

[Discover] Show a warning badge on dashboard

5ebe38c

[Discover] Handle NoResults case too

d37dcf0

[Discover] Rearrange components

338559b

[Discover] Support long list

4854473

[Discover] Update tests

2400388

[Discover] Catch timeouts too

9be17a9

[Discover] Better layout for timeouts

30ed661

Merge branch 'main' into 155216-inline-shard-error

9c685f6

[Discover] Allow to close warnings

d11195f

[Discover] Inline context app shard warnings

993ea5c

Merge remote-tracking branch 'upstream/main' into 155216-inline-shard…

e26cc30

…-error

[Discover] Update jest tests

71190b6

[Discover] Add tests

4940876

[Discover] Simplify imports

e04aaca

[Discover] Add tests

bdac782

[Discover] Update warning mocks

46361a0

[Discover] Add more tests

f5485b2

[Discover] Add a test for the dashboard badge

3f102cf

[Discover] Update snapshots

932e345

[Discover] Show warning badge outside of grid in a saved search panel

c9ad004

jughosta added 2 commits August 2, 2023 16:10

Merge remote-tracking branch 'upstream/main' into 155216-inline-shard…

e640e68

…-error # Conflicts: # src/plugins/discover/public/embeddable/saved_search_embeddable.tsx # src/plugins/discover/tsconfig.json

[Discover] Add comments

fb6eafe

jughosta added 2 commits August 3, 2023 10:14

Merge remote-tracking branch 'upstream/main' into 155216-inline-shard…

5249333

…-error

[Discover] Update old callout

b7dd1ef

jughosta added 2 commits August 3, 2023 14:14

[Discover] Update warning empty prompt

e0a581b

[Discover] Update tests

da0bd76

jughosta added 2 commits August 4, 2023 09:56

Merge remote-tracking branch 'upstream/main' into 155216-inline-shard…

99b98c8

…-error

[Discover] Update error callout

7a3331f

jughosta requested review from andreadelrio and davismcphee August 4, 2023 08:40

andreadelrio approved these changes Aug 4, 2023

View reviewed changes

Merge branch 'main' into 155216-inline-shard-error

4c0358a

kertal mentioned this pull request Aug 8, 2023

Handling of cross cluster search errors #163381

Closed

Merge branch 'main' into 155216-inline-shard-error

ed630be

davismcphee approved these changes Aug 10, 2023

View reviewed changes

Merge branch 'main' into 155216-inline-shard-error

2e3731e

jughosta merged commit c97d496 into elastic:main Aug 10, 2023
20 of 21 checks passed

jughosta deleted the 155216-inline-shard-error branch August 10, 2023 11:49

kibanamachine added the v8.10.0 label Aug 10, 2023

davismcphee mentioned this pull request Sep 20, 2023

[Discover] Improve shard error message formatting #161098

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Inline shard failures warnings #161271

[Discover] Inline shard failures warnings #161271

jughosta commented Jul 5, 2023 •

edited

jughosta commented Aug 2, 2023 •

edited

kertal commented Aug 2, 2023

4

andreadelrio commented Aug 2, 2023

jughosta commented Aug 3, 2023

andreadelrio commented Aug 3, 2023

jughosta commented Aug 4, 2023

andreadelrio left a comment

davismcphee left a comment

kertal commented Aug 10, 2023

kibana-ci commented Aug 10, 2023

API count

[Discover] Inline shard failures warnings #161271

[Discover] Inline shard failures warnings #161271

Conversation

jughosta commented Jul 5, 2023 • edited

Summary

Testing

Checklist

jughosta commented Aug 2, 2023 • edited

1

2

3

4

kertal commented Aug 2, 2023

4

andreadelrio commented Aug 2, 2023

2

3

4

jughosta commented Aug 3, 2023

2

4

andreadelrio commented Aug 3, 2023

jughosta commented Aug 4, 2023

andreadelrio left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

kertal commented Aug 10, 2023

kibana-ci commented Aug 10, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

jughosta commented Jul 5, 2023 •

edited

jughosta commented Aug 2, 2023 •

edited