[GS] add tag and dashboard suggestion results by pgayvallet · Pull Request #85144 · elastic/kibana

pgayvallet · 2020-12-07T15:10:56Z

Summary

PR Adds 'filter by tag' and 'filter by type' suggestions when the user searches for a term matching a search type or a tag name.

Screenshots

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
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

pgayvallet · 2020-12-07T15:17:45Z

@ryankeairns @alexfrancoeur the PR is still in draft, but I think it is functionally complete. Could you take a quick look see if everything is looking good?

Some remarks:

I can't easily retrieve the icon associated with a type for the filter by type suggestion. We will need to go with a static icon that doesn't depend on the suggested type (dashboard, vis, etc). I wasn't sure which icon was best here. Could you tell me which one to use?
I did as much as I could regarding the option display / rendering. I can't change the go to label to apply for example.
Now that we have this, do we want to remove the 'Filter by type or tag' in the footer? I think having both is fine.

ryankeairns · 2020-12-07T16:00:00Z

@ryankeairns @alexfrancoeur the PR is still in draft, but I think it is functionally complete. Could you take a quick look see if everything is looking good?

Some remarks:

I can't easily retrieve the icon associated with a type for the filter by type suggestion. We will need to go with a static icon that doesn't depend on the suggested type (dashboard, vis, etc). I wasn't sure which icon was best here. Could you tell me which one to use?

I did as much as I could regarding the option display / rendering. I can't change the go to label to apply for example.

Now that we have this, do we want to remove the 'Filter by type or tag' in the footer? I think having both is fine.

I just did a quick pass, this is looking really nice!

The only thing I've noticed thus far is that the tag names seem to be case sensitive (e.g. I tested with 'Ryan).

In response to your questions:

Icon for type: - We've used savedObjectsApp in some places for generically referring to objects, but that may not be accurate here (?). The other options that come to mind are empty or apps until we can possibly better determine the true type. Do any of those resonate with you?
The display looks good, generally speaking. @myasonik , did the latest changes to EuiSelectableSitewideTemplate enable us to change the 'Go to' badge label, or should I create a separate issue to make that customizable?
I agree with having both. The footer text helps to increase discoverability of the syntax.

Thanks for working on this, its quite helpful.

alexfrancoeur · 2020-12-07T16:08:34Z

I can't easily retrieve the icon associated with a type for the filter by type suggestion. We will need to go with a static icon that doesn't depend on the suggested type (dashboard, vis, etc). I wasn't sure which icon was best here. Could you tell me which one to use?

I'll defer to Ryan here, but I'm +1 on a static icon to simplify

I did as much as I could regarding the option display / rendering. I can't change the go to label to apply for example.

This feels like a nice to have to me. Technically, you're still going to a filtered list?

Now that we have this, do we want to remove the 'Filter by type or tag' in the footer? I think having both is fine.

I agree, a constant reminder of filter types might be useful.

The PR is looking great! A few quick observations and questions below

Does exact match have to be case sensitive? Because it's an exact match, I'm guessing yes, but thought I'd post the question.

I think there might be some issues filtering on tags with white space by using the default suggestion. Adding quotes around the string will provide the appropriate filter

pgayvallet · 2020-12-07T16:28:24Z

Does exact match have to be case sensitive? Because it's an exact match, I'm guessing yes, but thought I'd post the question.

Na, just forget to make the match case-insensitive. Will fix.

I think there might be some issues filtering on tags with white space by using the default suggestion. Adding quotes around the string will provide the appropriate filter

Yea, already have a TODO for this 😄

kibana/x-pack/plugins/global_search_bar/public/suggestions/index.ts

Line 38 in 7776444

suggestedSearch: `type:${searchTerm}`, // TODO: escape if necessary

myasonik · 2020-12-07T19:53:22Z

The display looks good, generally speaking. @myasonik , did the latest changes to EuiSelectableSitewideTemplate enable us to change the 'Go to' badge label, or should I create a separate issue to make that customizable?

I didn't cover it in the previous PR because the badge question is a bigger effort to fix and has a few technical issues in the way of implementing it. I meant to open a ticket for it earlier but forgot to. Done now!
elastic/eui#4361

…ch-suggestions

elasticmachine · 2020-12-08T11:32:01Z

Pinging @elastic/kibana-core (Team:Core)

flash1293

KibanaApp changes LGTM, code review only

ryankeairns

Works as expected; tested the whitespace and case fixes.
The filter icon makes sense, as well 🥇

I can't wait to hear the feedback, this search feature is really powerful.

MichaelMarcialis · 2020-12-08T21:25:49Z

The other options that come to mind are empty or apps until we can possibly better determine the true type. Do any of those resonate with you?

+1. I like the idea of using apps as the static icon for type filter suggestions.

alexfrancoeur

It's all coming together, this looks fantastic. Whitespace and case sensitivity tested on my end. Thanks Pierre, this is amazing!

joshdover

@pgayvallet is a global search machiiine! The implementation looks great, only small question/suggestion.

joshdover · 2020-12-09T00:14:01Z

x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.ts

+      return typeRegistry
+        .getVisibleTypes()
+        .filter((type) => type.management?.defaultSearchField && type.management?.getInAppUrl)
+        .map((type) => type.name);


Should we DRY this up with lines 26-29 to ensure the implementations of these two functions are consistent?

…ch-suggestions

kibanamachine · 2020-12-09T10:05:49Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 3963a35
Flaky suites:
- xpack-kibana-ciGroup2

Test Failures

Plugin Functional Tests.x-pack/test/plugin_api_integration/test_suites/task_manager/health_route·ts.task_manager health should return a breakdown of idleTasks in the task manager workload

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: task_manager
[00:00:00]           └-> "before all" hook
[00:00:00]           └-: health
[00:00:00]             └-> "before all" hook
[00:00:00]             └-> should return basic configuration of task manager
[00:00:00]               └-> "before each" hook: global before each
[00:00:00]               └- ✓ pass  (14ms) "task_manager health should return basic configuration of task manager"
[00:00:00]             └-> should return the task manager workload
[00:00:00]               └-> "before each" hook: global before each
[00:00:02]               │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] [.ds-ilm-history-5-000001] creating index, cause [initialize_data_stream], templates [ilm-history], shards [1]/[0]
[00:00:02]               │ info [o.e.c.m.MetadataCreateDataStreamService] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] adding data stream [ilm-history-5] with write index [.ds-ilm-history-5-000001] and backing indices []
[00:00:03]               │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] moving index [.ds-ilm-history-5-000001] from [null] to [{"phase":"new","action":"complete","name":"complete"}] in policy [ilm-history-ilm-policy]
[00:00:03]               │ info [o.e.c.r.a.AllocationService] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] current.health="GREEN" message="Cluster health status changed from [YELLOW] to [GREEN] (reason: [shards started [[.ds-ilm-history-5-000001][0]]])." previous.health="YELLOW" reason="shards started [[.ds-ilm-history-5-000001][0]]"
[00:00:03]               │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] moving index [.ds-ilm-history-5-000001] from [{"phase":"new","action":"complete","name":"complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] in policy [ilm-history-ilm-policy]
[00:00:03]               │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] moving index [.ds-ilm-history-5-000001] from [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-follow-shard-tasks"}] in policy [ilm-history-ilm-policy]
[00:00:03]               │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] [.kibana_task_manager_test_result] creating index, cause [auto(bulk api)], templates [], shards [1]/[1]
[00:00:03]               │ info [o.e.c.m.MetadataMappingService] [kibana-ci-immutable-centos-tests-xxl-1607500060156011205] [.kibana_task_manager_test_result/6-dICffzRdGUHQxhDg-3xQ] create_mapping
[00:00:05]               └- ✓ pass  (5.1s) "task_manager health should return the task manager workload"
[00:00:05]             └-> should return a breakdown of idleTasks in the task manager workload
[00:00:05]               └-> "before each" hook: global before each
[00:00:05]               └- ✖ fail: task_manager health should return a breakdown of idleTasks in the task manager workload
[00:00:05]               │       Error: expected 19 to sort of equal 20
[00:00:05]               │       + expected - actual
[00:00:05]               │ 
[00:00:05]               │       -19
[00:00:05]               │       +20
[00:00:05]               │       
[00:00:05]               │       at Assertion.assert (/dev/shm/workspace/parallel/3/kibana/packages/kbn-expect/expect.js:100:11)
[00:00:05]               │       at Assertion.eql (/dev/shm/workspace/parallel/3/kibana/packages/kbn-expect/expect.js:244:8)
[00:00:05]               │       at Context.<anonymous> (test/plugin_api_integration/test_suites/task_manager/health_route.ts:162:61)
[00:00:05]               │       at Object.apply (/dev/shm/workspace/parallel/3/kibana/packages/kbn-test/src/functional_test_runner/lib/mocha/wrap_function.js:84:16)
[00:00:05]               │ 
[00:00:05]               │

Stack Trace

Error: expected 19 to sort of equal 20
    at Assertion.assert (/dev/shm/workspace/parallel/3/kibana/packages/kbn-expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/3/kibana/packages/kbn-expect/expect.js:244:8)
    at Context.<anonymous> (test/plugin_api_integration/test_suites/task_manager/health_route.ts:162:61)
    at Object.apply (/dev/shm/workspace/parallel/3/kibana/packages/kbn-test/src/functional_test_runner/lib/mocha/wrap_function.js:84:16) {
  actual: '19',
  expected: '20',
  showDiff: true
}

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`globalSearch`	23	24	+1
`globalSearchBar`	35	37	+2
total			+3

Distributable file count

id	before	after	diff
`default`	47739	47742	+3

Page load bundle

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

id	before	after	diff
`globalSearch`	17.0KB	19.8KB	+2.8KB
`globalSearchBar`	43.4KB	48.6KB	+5.2KB
`globalSearchProviders`	8.8KB	8.8KB	+79.0B
`lens`	53.1KB	53.2KB	+39.0B
`savedObjectsTagging`	48.8KB	49.1KB	+305.0B
`savedObjectsTaggingOss`	8.4KB	8.5KB	+95.0B
total			+8.5KB

History

💚 Build #92665 succeeded b290c33
💚 Build #92649 succeeded e324d57
💚 Build #92632 succeeded 0131e71
💔 Build #92373 failed 7776444

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

* initial draft * polish * fix mocks * add tests * tests on suggestions * add comment * add FTR tests * factorize getSearchableTypes * move to bottom

…k-field-to-hot-phase * 'master' of github.com:elastic/kibana: (429 commits) simplify popover open state logic (elastic#85379) [Logs UI][Metrics UI] Move actions to the kibana header (elastic#84648) [Search Source] Do not pick scripted fields if * provided (elastic#85133) [Search] Session SO polling (elastic#84225) [Transform] Replace legacy elasticsearch client (elastic#84932) [Uptime]Refactor header and action menu (elastic#83779) Fix agg select external link (elastic#85380) [ILM] Show forcemerge in hot when rollover is searchable snapshot is enabled (elastic#85292) clear using keyboard (elastic#85042) [GS] add tag and dashboard suggestion results (elastic#85144) [ML] API integration tests - skip GetAnomaliesTableData Add ECS field for event.code. (elastic#85109) [Functional][TSVB] Wait for markdown textarea to be cleaned (elastic#85128) skip flaky suite (elastic#62060) skip flaky suite (elastic#85098) Bump highlight.js to v9.18.5 (elastic#84296) Add `server.publicBaseUrl` config (elastic#85075) [Alerting & Actions ] More debug logging (elastic#85149) [Security Solution][Case] Manual attach alert to a case (elastic#82996) Loosen UUID regex to accept uuidv1 or uuidv4 (elastic#85338) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx # 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_fields/index.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase/warm_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/i18n_texts.ts # x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts

initial draft

7776444

pgayvallet requested review from alexfrancoeur and ryankeairns December 7, 2020 15:12

pgayvallet added 7 commits December 8, 2020 07:59

Merge remote-tracking branch 'upstream/master' into kbn-83888-gs-sear…

e22f594

…ch-suggestions

polish

5690a71

fix mocks

0131e71

add tests

e324d57

tests on suggestions

901ae70

add comment

db3acbc

add FTR tests

b290c33

pgayvallet added release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.11.0 v8.0.0 labels Dec 8, 2020

pgayvallet marked this pull request as ready for review December 8, 2020 11:31

pgayvallet requested a review from a team December 8, 2020 11:31

pgayvallet requested a review from a team as a code owner December 8, 2020 11:31

pgayvallet requested a review from a team December 8, 2020 11:31

flash1293 approved these changes Dec 8, 2020

View reviewed changes

ryankeairns approved these changes Dec 8, 2020

View reviewed changes

alexfrancoeur approved these changes Dec 8, 2020

View reviewed changes

joshdover approved these changes Dec 9, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into kbn-83888-gs-sear…

c6fee45

…ch-suggestions

pgayvallet added 2 commits December 9, 2020 08:46

factorize getSearchableTypes

59cf6a1

move to bottom

3963a35

pgayvallet merged commit 73fbf2a into elastic:master Dec 9, 2020

pgayvallet mentioned this pull request Dec 9, 2020

[7.x] [GS] add tag and dashboard suggestion results (#85144) #85382

Merged

pgayvallet added a commit that referenced this pull request Dec 9, 2020

[GS] add tag and dashboard suggestion results (#85144) (#85382)

42ee8ba

* initial draft * polish * fix mocks * add tests * tests on suggestions * add comment * add FTR tests * factorize getSearchableTypes * move to bottom

This was referenced Dec 9, 2020

Add saved object tags to integrations with Kibana saved objects elastic/integrations#327

Closed

[Discuss] Documentation for Tags and Navigational Search #85630

Closed

Conversation

pgayvallet commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

Checklist

Uh oh!

pgayvallet commented Dec 7, 2020

Uh oh!

ryankeairns commented Dec 7, 2020

Uh oh!

alexfrancoeur commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pgayvallet commented Dec 7, 2020

Uh oh!

myasonik commented Dec 7, 2020

Uh oh!

elasticmachine commented Dec 8, 2020

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

ryankeairns 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!

MichaelMarcialis commented Dec 8, 2020

Uh oh!

alexfrancoeur left a comment

Choose a reason for hiding this comment

Uh oh!

joshdover left a comment

Choose a reason for hiding this comment

Uh oh!

joshdover Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 9, 2020

💛 Build succeeded, but was flaky

Test Failures

Standard Out

Stack Trace

Metrics [docs]

Module Count

Distributable file count

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

pgayvallet commented Dec 7, 2020 •

edited

Loading

alexfrancoeur commented Dec 7, 2020 •

edited

Loading

ryankeairns left a comment •

edited

Loading