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

[uiSettings]: Migrate items owned by app arch to new platform #66040

Merged
merged 46 commits into from
Jun 5, 2020

Conversation

VladLasitsa
Copy link
Contributor

@VladLasitsa VladLasitsa commented May 11, 2020

Closes #63480

Summary

Migrated uiSettings items to new platform:

  • query:queryString:options 👉 data
  • query:allowLeadingWildcards 👉 data
  • search:queryLanguage 👉 data
  • sort:options 👉 data
  • defaultIndex 👉data
  • courier:ignoreFilterIfFieldNotInIndex 👉data
  • courier:setRequestPreference 👉data
  • courier:customRequestPreference 👉data
  • courier:maxConcurrentShardRequests 👉data
  • courier:batchSearches 👉data
  • search:includeFrozen 👉data
  • histogram:barTarget 👉data
  • histogram:maxBars 👉data
  • visualize:enableLabs 👉visualizations
  • csv:separator 👉share
  • csv:quoteValues 👉share
  • history:limit 👉data
  • shortDots:enable 👉data
  • format:defaultTypeMap 👉data
  • format:number:defaultPattern 👉data
  • format:bytes:defaultPattern 👉data
  • format:percent:defaultPattern 👉data
  • format:currency:defaultPattern 👉data
  • format:number:defaultLocale 👉data
  • timepicker:refreshIntervalDefaults 👉data
  • timepicker:quickRanges 👉data
  • indexPattern:placeholder 👉data
  • filters:pinnedByDefault 👉data
  • filterEditor:suggestValues 👉data

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@VladLasitsa VladLasitsa requested a review from alexwizp May 11, 2020 14:27
@VladLasitsa VladLasitsa self-assigned this May 11, 2020
@elasticmachine
Copy link
Contributor

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

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

setRequestReferenceSetting: '<strong>courier:setRequestPreference</strong>',
customSettingValue: '"custom"',
requestPreferenceLink:
'<a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-preference.html" target="_blank" rel="noopener">' +
Copy link
Contributor

Choose a reason for hiding this comment

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

@lukeelmers not sure that it's related to current PR, but it looks like an issue which should be addressed in future.
We should get links from DocLinks service (src/core/public/doc_links/doc_links_service.ts) instead of hardcoding it.

Copy link
Member

Choose a reason for hiding this comment

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

We should get links from DocLinks service (src/core/public/doc_links/doc_links_service.ts) instead of hardcoding it.

Yes, agreed. The problem right now is that docLinks is not available in CoreSetup. I have a PR up to expose it in setup so we could use it here.

Once that's merged we can add these links to the docLinks service, as I don't see them there right now.

Copy link
Member

Choose a reason for hiding this comment

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

I created #67026 to track updating docLinks as a follow-up task.

@VladLasitsa VladLasitsa marked this pull request as ready for review May 12, 2020 15:29
@VladLasitsa VladLasitsa requested a review from a team as a code owner May 12, 2020 15:29
@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

1 similar comment
@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

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

Canvas code looks good 👍

@lukeelmers
Copy link
Member

@elasticmachine merge upstream

elasticmachine and others added 4 commits June 3, 2020 07:32
# Conflicts:
#	x-pack/plugins/reporting/server/export_types/csv/server/execute_job.test.ts
#	x-pack/plugins/reporting/server/export_types/csv/server/execute_job.ts
#	x-pack/plugins/reporting/server/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts
#	x-pack/plugins/translations/translations/ja-JP.json
#	x-pack/plugins/translations/translations/zh-CN.json
# Conflicts:
#	src/plugins/data/public/public.api.md
#	src/plugins/data/server/server.api.md
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Assuming UI_SETTINGS.* are just strings being swapped in for our hard-coded ones, this LGTM!

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Stack Monitoring looks good from code pov 👍

@VladLasitsa
Though I couldn't test the branch locally since I kept getting:

 FATAL  Error: Setup lifecycle of "reporting" plugin wasn't completed in 30sec. Consider disabling the plugin and re-start.

 server crashed  with status code 1

@alexwizp alexwizp merged commit b87eefc into elastic:master Jun 5, 2020
alexwizp pushed a commit to alexwizp/kibana that referenced this pull request Jun 5, 2020
…c#66040)

* Migrated uiSettings items to new platform

* API changes

* Fixed translations

* Fixed comment and i18n

* Fixed tests

* Fixed internalization

* Fix karma tests

* made code more explicit

* Fixed plugin

* Added consts for ui settings ids.

* Added id for another settings

* Fixed tests.

* Improved imports

* Fix imports to public which were happening from the server.

* Fixed paths

* Moved styles to plugin.ts

* Moved styles

* Fixed docs

* Fix ci

* Fix ci

* fix documentation

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Alexey Antonov <[email protected]>
Co-authored-by: Luke Elmers <[email protected]>
# Conflicts:
#	src/legacy/core_plugins/kibana/server/ui_setting_defaults.js
alexwizp added a commit that referenced this pull request Jun 5, 2020
#68354)

* Migrated uiSettings items to new platform

* API changes

* Fixed translations

* Fixed comment and i18n

* Fixed tests

* Fixed internalization

* Fix karma tests

* made code more explicit

* Fixed plugin

* Added consts for ui settings ids.

* Added id for another settings

* Fixed tests.

* Improved imports

* Fix imports to public which were happening from the server.

* Fixed paths

* Moved styles to plugin.ts

* Moved styles

* Fixed docs

* Fix ci

* Fix ci

* fix documentation

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Alexey Antonov <[email protected]>
Co-authored-by: Luke Elmers <[email protected]>
# Conflicts:
#	src/legacy/core_plugins/kibana/server/ui_setting_defaults.js

Co-authored-by: Uladzislau Lasitsa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[uiSettings]: Migrate items owned by app arch to new platform