[NP] Migrate uiSettings owned by Kibana app #64321

maryia-lapata · 2020-04-23T14:17:52Z

Summary

Fixes #63597.

This PR migrates the following ui settings to the new platform:

to discover:
- defaultColumns
- discover:sampleSize
- discover:aggs:terms:size
- discover:sort:defaultOrder
- discover:searchOnPageLoad
- doc_table:hideTimeColumn
- fields:popularLimit
- context:defaultSize
- context:step
- context:tieBreakerFields
to data:
- metaFields
- doc_table:highlight
to charts:
- visualization:colorMapping
to vis_type_vislib:
- visualization:dimmingOpacity
- visualization:heatmap:maxBuckets
to saved_objects:
- savedObjects:perPage
- savedObjects:listingLimit
to vis_type_timeseries:
- metrics:max_buckets
remove visualization:loadingDelay since it's unused.

i18n IDs were updated respectively.

maryia-lapata · 2020-04-24T11:57:40Z

@elasticmachine merge upstream

elasticmachine · 2020-04-24T11:57:42Z

merge conflict between base and head

kertal · 2020-04-27T11:31:00Z

@elasticmachine merge upstream

elasticmachine · 2020-04-27T11:31:02Z

merge conflict between base and head

kertal · 2020-04-28T09:34:05Z

@elasticmachine merge upstream

elasticmachine · 2020-04-28T09:34:07Z

merge conflict between base and head

nreese

Maps changes LGTM
code review

src/plugins/saved_objects_management/public/management_section/mount_section.tsx

…to setting accessors

joshdover · 2020-05-11T19:45:53Z

I agree with that plugins should only have access to the settings that either (1) Core exposes (2) plugins they depend on expose (3) they own themselves.

I also think that to accomplish that we need a much larger change to how this service works in order to enforce those boundaries. Same goes for saved object types. We probably shouldn't allow arbitrary access to any SO type.

So maybe for now we try to follow the pattern @pgayvallet laid out above where it's easy & feasible, but if there are some cases that don't fit nicely into that pattern we can solve that separately later?

kertal · 2020-05-11T20:28:34Z

So maybe for now we try to follow the pattern @pgayvallet laid out above where it's easy & feasible, but if there are some cases that don't fit nicely into that pattern we can solve that separately later?

I do agree and I'm currently adapting the PR to the pattern @pgayvallet suggested, where possible

src/plugins/discover/server/ui_settings.ts

mshustov · 2020-05-12T15:20:06Z

src/plugins/data/server/ui_settings.ts

+    schema: schema.arrayOf(schema.string()),
+  },


Optional: I'm wondering whether we know all of them to declare validation as a union of well-known strings.

here are mapping fields. but not all e.g. _score of the query context is missing, think it should stay flexible, just in case
https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-fields.html

src/plugins/dashboard/public/application/legacy_app.js

src/plugins/saved_objects/server/plugin.ts

src/plugins/vis_type_vislib/server/plugin.ts

src/plugins/visualize/public/application/listing/visualize_listing.js

flash1293

Tested against current master and all settings are still there. LGTM.

While looking through the code I noticed we need to do the same thing for a bunch of timelion settings, I will create a separate issue for that.

src/plugins/charts/server/plugin.ts

kibanamachine · 2020-05-15T12:31:24Z

💚 Build Succeeded

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

History

💚 Build #47891 succeeded 8d4b8c2
💔 Build #47690 failed ca7915a
💔 Build #47601 failed 185d29c
💚 Build #47097 succeeded 785c3a5
💔 Build #46937 failed 72e29b7

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

alexwizp · 2020-05-15T12:42:52Z

...re_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/tabs/tabs.tsx

@@ -33,6 +33,7 @@ import { i18n } from '@kbn/i18n';
 import { fieldWildcardMatcher } from '../../../../../../../../../plugins/kibana_utils/public';
 import { IndexPatternManagementStart } from '../../../../../../../../../plugins/index_pattern_management/public';
 import { IndexPattern, IndexPatternField } from '../../../../../../../../../plugins/data/public';
+import { META_FIELDS_SETTING } from '../../../../../../../../../plugins/data/common';


If I'm not wrong we are trying to avoid imports from /common folder.

Also I think that this approach introduce a lot of fields in our static contract which also not good.

Maybe we can move all into one variable:

const UI_SETTINGS = { META_FIELDS_SETTING: 'meta_field', DEFAULT_QUERY_LANGUAGE: 'kuery', META_FIELDS_SETTING: `'metaFields',` }

sorry, seems we had a comment/merge race condition. seen this comment after the merge. since this part of Kibana was in the legacy world, and it's currently migrated, this could be done in or after #65026

* Move META_FIELDS_SETTING, DOC_HIGHLIGHT_SETTING to data plugin.ts * Refactor table_list_view.tsx to no longer get PER_PAGE_SETTING by uiSettings * Migrate graph, visualize, dashboard usage of saved_objects module constants to accessor functions * Remove redundant logging in plugins start and setup methods Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> # Conflicts: # src/plugins/data/public/index_patterns/index_patterns/index_pattern.ts

* Move META_FIELDS_SETTING, DOC_HIGHLIGHT_SETTING to data plugin.ts * Refactor table_list_view.tsx to no longer get PER_PAGE_SETTING by uiSettings * Migrate graph, visualize, dashboard usage of saved_objects module constants to accessor functions * Remove redundant logging in plugins start and setup methods Co-authored-by: Maryia Lapata <mary.lopato@gmail.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

* master: (191 commits) [Maps] Get number of categories from palette (elastic#66454) move oss features registration to KP (elastic#66524) [kbn/plugin-helpers] typescript-ify (elastic#66513) Add kibana-operations as codeowners for .ci/es-snapshots and vars/ (elastic#66746) FTR: move basic services under common folder (elastic#66563) Migrate Beats Management UI to KP (elastic#65791) [CI] Add 20 minutes to overall build timeout lint import from restricted zones for export exressions (elastic#66588) [SIEM][Detection Engine] Add validation for Rule Actions (elastic#63332) KP plugins shouldn't need package.json (elastic#66654) Replace agent metrics link with the new one (elastic#66632) [CI] Add one retry to setup step (elastic#66638) [CI] Add slack alerts to tracked branch jobs, change default channel, change formatting (elastic#66580) [docLinks] Add docLinks to CoreSetup. (elastic#66631) [DOCS] Rename monitoring collection from internal to legacy (elastic#65781) unskip newsfeed tests (elastic#66562) [NP] Migrate uiSettings owned by Kibana app (elastic#64321) [ML] Functional tests - stabilize typing in DFA mml input (elastic#66706) [Map] return bounding box for static feature collection without joins (elastic#66607) remove trailing slash in graph sample data links (elastic#66358) ...

elasticmachine · 2020-05-25T18:07:14Z

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

Migrate uiSettings

4efa64e

maryia-lapata added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.8.0 labels Apr 23, 2020

Migrate uiSettings to visTypeVislib

77b75cd

maryia-lapata added 2 commits April 24, 2020 11:43

Refactoring

db60224

Update index_pattern.ts

6761400

Merge remote-tracking branch 'kibana/master' into ui_settings

cf723a9

maryia-lapata assigned sulemanof Apr 24, 2020

kertal added 2 commits April 27, 2020 13:37

Merge master /fix conflicts

83b29f7

Add defaults for fixing failing tests

c0c8327

kertal self-assigned this Apr 28, 2020

Merge master /fix conflicts

1d21edd

kertal marked this pull request as ready for review April 28, 2020 13:27

kertal requested a review from a team April 28, 2020 13:27

kertal requested review from a team as code owners April 28, 2020 13:27

kertal requested a review from flash1293 April 28, 2020 13:28

nreese approved these changes Apr 28, 2020

View reviewed changes

mshustov reviewed Apr 28, 2020

View reviewed changes

src/plugins/saved_objects_management/public/management_section/mount_section.tsx Outdated Show resolved Hide resolved

Merge master /fix conflicts

33587b0

Migrate graph usage of saved_objects module constants for uiSettings …

785c3a5

…to setting accessors

mshustov reviewed May 12, 2020

View reviewed changes

kertal added 4 commits May 13, 2020 12:52

Adapt Visualize code

185d29c

Cleanups

17c291a

Adapt dashboard

5df721d

Merge master /fix conflicts

ca7915a

mshustov approved these changes May 13, 2020

View reviewed changes

Fix types

8d4b8c2

kertal requested a review from mattkime May 14, 2020 15:43

mattkime approved these changes May 14, 2020

View reviewed changes

lukeelmers mentioned this pull request May 14, 2020

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

Merged

7 tasks

flash1293 approved these changes May 15, 2020

View reviewed changes

src/plugins/charts/server/plugin.ts Outdated Show resolved Hide resolved

kertal added 2 commits May 15, 2020 12:20

Merge remote-tracking branch 'upstream/master' into ui_settings

6119854

Remove chart plugin logging

fe8dc06

alexwizp reviewed May 15, 2020

View reviewed changes

kertal merged commit 7c8eea1 into elastic:master May 15, 2020

kertal added v7.9.0 and removed v7.8.0 labels May 15, 2020

kertal mentioned this pull request May 15, 2020

[7.x] [NP] Migrate uiSettings owned by Kibana app (#64321) #66730

Merged

lukeelmers mentioned this pull request May 19, 2020

[data] Expose uiSettings in plugin contract #67060

Closed

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

VladLasitsa mentioned this pull request Oct 7, 2020

Removed visualization:loadingDelay setting docs #79841

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NP] Migrate uiSettings owned by Kibana app #64321

[NP] Migrate uiSettings owned by Kibana app #64321

maryia-lapata commented Apr 23, 2020 •

edited by kertal

Loading

maryia-lapata commented Apr 24, 2020

elasticmachine commented Apr 24, 2020

kertal commented Apr 27, 2020

elasticmachine commented Apr 27, 2020

kertal commented Apr 28, 2020

elasticmachine commented Apr 28, 2020

nreese left a comment

joshdover commented May 11, 2020

kertal commented May 11, 2020

mshustov May 12, 2020

kertal May 13, 2020

flash1293 left a comment

kibanamachine commented May 15, 2020

alexwizp May 15, 2020

kertal May 15, 2020

elasticmachine commented May 25, 2020

[NP] Migrate uiSettings owned by Kibana app #64321

[NP] Migrate uiSettings owned by Kibana app #64321

Conversation

maryia-lapata commented Apr 23, 2020 • edited by kertal Loading

Summary

maryia-lapata commented Apr 24, 2020

elasticmachine commented Apr 24, 2020

kertal commented Apr 27, 2020

elasticmachine commented Apr 27, 2020

kertal commented Apr 28, 2020

elasticmachine commented Apr 28, 2020

nreese left a comment

Choose a reason for hiding this comment

joshdover commented May 11, 2020

kertal commented May 11, 2020

mshustov May 12, 2020

Choose a reason for hiding this comment

kertal May 13, 2020

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

kibanamachine commented May 15, 2020

💚 Build Succeeded

History

alexwizp May 15, 2020

Choose a reason for hiding this comment

kertal May 15, 2020

Choose a reason for hiding this comment

elasticmachine commented May 25, 2020

maryia-lapata commented Apr 23, 2020 •

edited by kertal

Loading