Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move ui/value_suggestions ⇒ NP data plugin #45762

Merged

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Sep 16, 2019

Summary

  • Move ui/value_suggestions ⇒ NP data plugin
  • Update value_suggestions to use core.http instead of kfetch and 'uiSettings'
  • Re-write x-pack/legacy/plugins/kuery_autocomplete/public/autocomplete_providers/value.test.js from karma to jest. The tests used to mostly test the underlying value_suggestions. I updated them to test the kuery_autocomplete provider instead.

FilterBar

  • Replace uiSettings that were passed around FilterBar components, with using the useKibana hook.
  • FilterBar to create it's own context with wrapInContextIfMissing if instantiated from angular
  • Pass NP data plugin to KibanaContextProvider to have access to autocomplete and getSuggestions

Dev Docs

Move value_suggestions into NP and expose data.getSuggestions on start contract.

In old platform:

    import { npStart } from 'ui/new_platform';
    const suggestions = await npStart.plugins.data.getSuggestions(indexPattern.title, field, value);

In new platform:

    public start(core, plugins) {
       ...
       const suggestions = await plugins.data.getSuggestions(indexPattern.title, field, value);
    }

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom lizozom marked this pull request as ready for review September 23, 2019 16:25
@lizozom lizozom changed the title Newplatform/data plugin/value suggesions Move ui/value_suggestions ⇒ data plugin Sep 23, 2019
@lizozom lizozom changed the title Move ui/value_suggestions ⇒ data plugin Move ui/value_suggestions ⇒ data plugin Sep 23, 2019
@lizozom lizozom changed the title Move ui/value_suggestions ⇒ data plugin Move ui/value_suggestions ⇒ data plugin Sep 23, 2019
@lizozom lizozom self-assigned this Sep 23, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@lizozom lizozom added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. review labels Sep 23, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Liza K added 13 commits September 26, 2019 14:31
Pass storage and autocomplete to createSearchBar method
Add appName and autocomplete to IDataPluginServices
QueryBarInput to consume autocomplete and appName from context
QueryBarTopRow to consume appName from context
Remove appName from SearchBar
Added AutocompletePublicPluginSetup and AutocompletePublicPluginStart types
@lizozom lizozom requested review from ppisljar and streamich October 3, 2019 11:58
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor Author

lizozom commented Oct 3, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

Comment on lines 38 to +39
autocomplete: this.autocomplete,
getSuggestions: getSuggestionsProvider(core.uiSettings, core.http),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In data plugin we have this services pattern where each service is returned from life-cycles, here autocomplete and getSuggestions are some utilities. Should we collect them into a service, say suggestions?

Copy link
Contributor Author

@lizozom lizozom Oct 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually thought of placing it inside the autocomplete service.
I just wasn't sure and didn't want to create unnecessary code while not sure.
Until we have a good hunch, I think the services pattern is great for more complex things, than a single function.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom lizozom merged commit 0c001e4 into elastic:master Oct 6, 2019
lizozom pushed a commit to lizozom/kibana that referenced this pull request Oct 6, 2019
* Bind search bar

* create prewired data components

* Pass NP data plugin to shim plugin, to access autocomplete
Pass storage and autocomplete to createSearchBar method
Add appName and autocomplete to IDataPluginServices
QueryBarInput to consume autocomplete and appName from context
QueryBarTopRow to consume appName from context
Remove appName from SearchBar
Added AutocompletePublicPluginSetup and AutocompletePublicPluginStart types

* Use KibanaContextProvider in vis editor and graph

* Use KibanaContextProvider in maps

* Use prewirted SearchBar in TopNavMenu

* Use KibanaContextProbider in Lens

* Fix appName usage in query bar input

* fixed query bar top row appName

* update tests

* fixed bind search bar bug

* mock SearchBar

* Removed unnecessary mocks

* Delete unused mock

* Fixed exporting of data plugin types

* Updated maps snapshot

* Fixed some TS issues

* Fixed jest tests

* Context adjustments in TSVB

* componentWillMount

* Code review fixes

* Pass dataTestSubj to query bar input

* Graph data

* - Pass NP data plugin to KibanaReactContext
- Move value_suggestions to NP

* - Pass NP data plugin to KibanaReactContext
- Move value_suggestions to NP

* ts fixes

* Added karma getSuggestions fake

* Refactored kuery autocomplete tests to jest

* Filter bar context for directives

* updated snapshot

* fix diffs

* fixed lens test
lizozom pushed a commit that referenced this pull request Oct 6, 2019
* Bind search bar

* create prewired data components

* Pass NP data plugin to shim plugin, to access autocomplete
Pass storage and autocomplete to createSearchBar method
Add appName and autocomplete to IDataPluginServices
QueryBarInput to consume autocomplete and appName from context
QueryBarTopRow to consume appName from context
Remove appName from SearchBar
Added AutocompletePublicPluginSetup and AutocompletePublicPluginStart types

* Use KibanaContextProvider in vis editor and graph

* Use KibanaContextProvider in maps

* Use prewirted SearchBar in TopNavMenu

* Use KibanaContextProbider in Lens

* Fix appName usage in query bar input

* fixed query bar top row appName

* update tests

* fixed bind search bar bug

* mock SearchBar

* Removed unnecessary mocks

* Delete unused mock

* Fixed exporting of data plugin types

* Updated maps snapshot

* Fixed some TS issues

* Fixed jest tests

* Context adjustments in TSVB

* componentWillMount

* Code review fixes

* Pass dataTestSubj to query bar input

* Graph data

* - Pass NP data plugin to KibanaReactContext
- Move value_suggestions to NP

* - Pass NP data plugin to KibanaReactContext
- Move value_suggestions to NP

* ts fixes

* Added karma getSuggestions fake

* Refactored kuery autocomplete tests to jest

* Filter bar context for directives

* updated snapshot

* fix diffs

* fixed lens test
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 7, 2019
… into console-token-iterator

* 'console-token-iterator' of github.com:jloleysens/kibana: (184 commits)
  [functional/services] update webdriver lib and types (elastic#47381)
  Standardizing IconField implementation across the app (elastic#47196)
  Move ui/value_suggestions ⇒ NP data plugin (elastic#45762)
  Remove ui/persisted_log - Part 2 (elastic#47236)
  Update gulp related packages (elastic#47421)
  Update dependency idx to ^2.5.6 (elastic#47399)
  try running fewer jobs in parallel on the same worker (elastic#47403)
  Update webpack related packages (elastic#47402)
  Update jsonwebtoken related packages (elastic#47400)
  Update gulp related packages (major) (elastic#46665)
  Update dependency prettier to ^1.18.2 (elastic#47340)
  Update dependency @types/puppeteer to ^1.20.1 (elastic#47339)
  Update dependency @elastic/elasticsearch to ^7.4.0 (elastic#47338)
  Update dependency tar-fs to ^1.16.3 (elastic#47341)
  [Code] Code Integrator Component (elastic#47180)
  [Canvas][i18n] Sidebar (elastic#46090)
  Generate uuid in task Manager as Kibana uuid may not yet have been initialised
  [Code] Embedded Code Snippet Component (elastic#47183)
  Revert "Add pipeline for flaky test runner job (elastic#46740)"
  SearchSource: fix docvalue_fields and fields intersection logic (elastic#46724)
  ...
@rayafratkina
Copy link
Contributor

@lizozom can you please add a DevDocs section?

@lizozom lizozom deleted the newplatform/data-plugin/value-suggesions branch November 14, 2019 13:03
@elasticmachine
Copy link
Contributor

💔 Build Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. review v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants