[NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy #60276

sulemanof · 2020-03-16T15:56:53Z

Summary

Based on #57695 .

This removes all imports from ui/agg_types and uses NP data plugin instead.
This also moves paginated table to kibana_legacy plugin.

For reviewers:

It shouldn't be hard to review this, since the most cases are:

use absolute paths src/plugins/data/public instead of ui/agg_types where only types are used;
use relative paths ../**/plugins/data/public instead of ui/agg_types where static stuff is used;
use data plugin contract to access common data services;

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
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts # src/legacy/core_plugins/kibana/public/visualize/np_ready/types.d.ts # src/legacy/core_plugins/kibana/public/visualize/plugin.ts # src/legacy/core_plugins/vis_default_editor/public/components/agg_params.tsx # src/legacy/core_plugins/vis_default_editor/public/components/agg_params_helper.test.ts # src/legacy/core_plugins/vis_default_editor/public/components/agg_params_helper.ts # src/legacy/core_plugins/vis_default_editor/public/components/sidebar/state/reducers.ts # src/legacy/core_plugins/vis_default_editor/public/default_editor.tsx # src/legacy/core_plugins/vis_default_editor/public/legacy_imports.ts # src/legacy/core_plugins/vis_type_table/public/legacy_imports.ts # src/legacy/core_plugins/vis_type_vislib/public/legacy_imports.ts # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

# Conflicts: # src/legacy/core_plugins/kibana/public/discover/kibana_services.ts # src/legacy/core_plugins/vis_type_tagcloud/public/legacy_imports.ts # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

elasticmachine · 2020-03-18T12:32:20Z

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

sulemanof · 2020-03-18T12:36:14Z

src/dev/jest/config.js

@@ -49,8 +49,6 @@ export default {
    '!packages/kbn-ui-framework/src/services/**/*/index.js',
    'src/legacy/core_plugins/**/*.{js,jsx,ts,tsx}',
    '!src/legacy/core_plugins/**/{__test__,__snapshots__}/**/*',
-    'src/legacy/ui/public/{agg_types,vis}/**/*.{ts,tsx}',
-    '!src/legacy/ui/public/{agg_types,vis}/**/*.d.ts',


There is no necessity to collect coverage from these folders (agg_types doesn't exist anymore, vis contains only legacy mocka tests since the most of code was moved)

# Conflicts: # src/legacy/core_plugins/kibana/public/discover/kibana_services.ts

spalger

~~Please re-request review from @elastic/kibana-operations when this is passing CI~~ oops, misinterpreted the latest test results

spalger

LGTM from an operations perspective

lukeelmers

Code changes LGTM for app arch! Reviewed everything other than the paginate directive updates, which I am unfamiliar with.

And thanks for deleting ui/agg_types here. Nice to see that going away 🙂

lukeelmers · 2020-03-18T22:46:52Z

src/legacy/core_plugins/vis_default_editor/public/components/agg_params.test.tsx

+      data: {
+        search: {
+          aggs: {
+            types: {
+              getAll: () => [],
+            },
+          },
+          __LEGACY: {
+            aggTypeFieldFilters: {
+              filter: () => [],
+            },
+          },
+        },
+      },


nit - It would probably make maintenance easier if you passed the mocks from src/plugins/data/public/mocks.ts here (assuming that works for these specific tests)

done.
Wrapped KibanaContextProvider with dataPluginMock service .

lukeelmers · 2020-03-18T22:57:24Z

src/legacy/core_plugins/vis_default_editor/public/components/agg_params_helper.ts

@@ -136,13 +132,14 @@ function getAggParamsToRender({
 }

 function getAggTypeOptions(
+  aggTypes: any,


We should probably expose a proper interface for this from the data plugin, because I don't think there's an easy way to type aggTypes otherwise. I've added it to our roadmap list in #60126

lukeelmers · 2020-03-18T23:02:51Z

src/legacy/ui/public/agg_types/index.ts

-  siblingPipelineType,
-  termsAggFilter,
-} = search.aggs;
-


kertal · 2020-03-19T06:29:14Z

src/legacy/core_plugins/vis_type_table/public/legacy_imports.ts

@@ -19,13 +19,10 @@

 export { npSetup, npStart } from 'ui/new_platform';
 export { getFormat } from 'ui/visualize/loader/pipeline_helpers/utilities';
-export { IAggConfig, AggGroupNames, Schemas } from 'ui/agg_types';
 // @ts-ignore
 export { PaginateDirectiveProvider } from 'ui/directives/paginate';
 // @ts-ignore
 export { PaginateControlsDirectiveProvider } from 'ui/directives/paginate';


you could also adapt imports PaginateDirectiveProvider and PaginateControlsDirectiveProvider from kibana_legacy. then ui/directives/paginate is only used in src/legacy/core_plugins/timelion/public/directives/saved_object_finder.js, so it could be inlined there

It's a really good catch, removed reference here and completely removed ui/directives/paginate references.

src/legacy/core_plugins/vis_default_editor/public/components/agg_params.tsx

# Conflicts: # x-pack/legacy/plugins/rollup/public/legacy_imports.ts

sulemanof · 2020-03-20T07:17:09Z

@elasticmachine merge upstream

kibanamachine · 2020-03-20T09:01:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 40c15a0

History

💔 Build #34609 failed 57ea6fa
💔 Build #34555 failed 937777f
💔 Build #34471 failed 79b7b88
💛 Build #34219 was flaky b68ab0b
💔 Build #34175 failed fc3c455

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

kertal

Code LGTM, tested locally with chrome, Kibana continues to loose weight, 154 LOC this time 👍

…ibana_legacy (elastic#60276) * fix agg type shims and move paginated table to kibana_legacy * fix types * fix i18n ids * fix unit tests * Update imports * Remove ui/agg_types imports * Clean up vis_default_editor plugin * Remove agg_types imports in vis_type_table * Clean up x-pack * Clean up vis_type_vislib * Last cleanups * Update docs * Mock Schemas in vis_type_metric * Use data plugin mocks * Remove ui/directives/paginate reference * Remove snapshot * Remove shallow Co-authored-by: Joe Reuter <[email protected]> Co-authored-by: Elastic Machine <[email protected]> # Conflicts: # src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts

* master: (55 commits) Update dependency @elastic/charts to v18.1.0 (elastic#60578) Only set timezone when user setting is a valid timezone (elastic#57850) [NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy (elastic#60276) [SIEM] Fix types in rules tests (elastic#60736) [Alerting] prevent flickering when fields are updated in an alert (elastic#60666) License checks for actions plugin (elastic#59070) Implemented ability to clear and properly validate alert interval (elastic#60571) WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568) [Maps] Update layer dependencies to NP (elastic#59585) [Discover] Remove StateManagementConfigProvider (elastic#60221) [ML] Listing all categorization wizard checks (elastic#60502) [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887) [SIEM] Export timeline (elastic#58368) [SIEM] Add support for actions and throttle in Rules (elastic#59641) Fix ace a11y listener (elastic#60639) Add addInfo toast to core notifications service (elastic#60574) fix test description (elastic#60638) [SIEM] Cypress screenshots upload to google cloud (elastic#60556) [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653) [SIEM] Fixes Modification of ML Rules (elastic#60662) ...

* master: Only set timezone when user setting is a valid timezone (elastic#57850) [NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy (elastic#60276) [SIEM] Fix types in rules tests (elastic#60736) [Alerting] prevent flickering when fields are updated in an alert (elastic#60666) License checks for actions plugin (elastic#59070) Implemented ability to clear and properly validate alert interval (elastic#60571) WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568) [Maps] Update layer dependencies to NP (elastic#59585) [Discover] Remove StateManagementConfigProvider (elastic#60221)

…ibana_legacy (#60276) (#60769) * fix agg type shims and move paginated table to kibana_legacy * fix types * fix i18n ids * fix unit tests * Update imports * Remove ui/agg_types imports * Clean up vis_default_editor plugin * Remove agg_types imports in vis_type_table * Clean up x-pack * Clean up vis_type_vislib * Last cleanups * Update docs * Mock Schemas in vis_type_metric * Use data plugin mocks * Remove ui/directives/paginate reference * Remove snapshot * Remove shallow Co-authored-by: Joe Reuter <[email protected]> Co-authored-by: Elastic Machine <[email protected]> # Conflicts: # src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts

flash1293 and others added 7 commits February 14, 2020 16:06

fix agg type shims and move paginated table to kibana_legacy

7eed77e

Merge remote-tracking branch 'upstream/master' into fix-visualize-shims

3e992c2

fix types

f880e2d

fix i18n ids

7abddc4

fix unit tests

555e19e

Merge remote-tracking branch 'upstream/master' into fix-visualize-shims

d70fac3

sulemanof mentioned this pull request Mar 16, 2020

Fix agg type shims and move paginated table to kibana_legacy #57695

Closed

sulemanof added 2 commits March 16, 2020 19:09

Update imports

f390d01

Remove ui/agg_types imports

d4688bf

sulemanof changed the title ~~[NP] Fix agg type shims and move paginated table to kibana_legacy~~ [NP] Remove ui/agg_types dependencies and move paginated table to kibana_legacy Mar 17, 2020

Clean up vis_default_editor plugin

97f40f4

flash1293 mentioned this pull request Mar 17, 2020

Meta: Kibana app NP cutover #60097

Closed

69 tasks

sulemanof added 5 commits March 17, 2020 19:21

Remove agg_types imports in vis_type_table

111d502

Clean up x-pack

7a9b879

Clean up vis_type_vislib

87bc3dc

Last cleanups

bbd5256

sulemanof requested review from lukeelmers and kertal March 18, 2020 12:31

sulemanof marked this pull request as ready for review March 18, 2020 12:31

sulemanof requested a review from a team March 18, 2020 12:31

sulemanof requested review from a team as code owners March 18, 2020 12:31

sulemanof added Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0 labels Mar 18, 2020

sulemanof commented Mar 18, 2020

View reviewed changes

sulemanof added 3 commits March 18, 2020 17:01

Update docs

fc3c455

Merge remote-tracking branch 'upstream/master' into fix-visualize-shims

895c69b

# Conflicts: # src/legacy/core_plugins/kibana/public/discover/kibana_services.ts

Mock Schemas in vis_type_metric

b68ab0b

spalger reviewed Mar 18, 2020

View reviewed changes

spalger approved these changes Mar 18, 2020

View reviewed changes

lukeelmers approved these changes Mar 18, 2020

View reviewed changes

kertal reviewed Mar 19, 2020

View reviewed changes

src/legacy/core_plugins/vis_default_editor/public/components/agg_params.tsx Show resolved Hide resolved

sulemanof added 5 commits March 19, 2020 12:51

Use data plugin mocks

7c358a9

Remove ui/directives/paginate reference

b258b21

Merge remote-tracking branch 'upstream/master' into fix-visualize-shims

79b7b88

# Conflicts: # x-pack/legacy/plugins/rollup/public/legacy_imports.ts

Remove snapshot

937777f

Remove shallow

57ea6fa

Merge branch 'master' into fix-visualize-shims

40c15a0

kertal self-requested a review March 20, 2020 15:00

kertal approved these changes Mar 20, 2020

View reviewed changes

sulemanof merged commit 1a1e2e7 into elastic:master Mar 20, 2020

sulemanof deleted the fix-visualize-shims branch March 20, 2020 15:39

sulemanof mentioned this pull request Mar 20, 2020

[7.x] [NP] Remove ui/agg_types dependencies and move paginated table to kibana_legacy (#60276) #60769

Merged

sulemanof added the backported label Mar 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy #60276

[NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy #60276

sulemanof commented Mar 16, 2020 •

edited

Loading

elasticmachine commented Mar 18, 2020

sulemanof Mar 18, 2020 •

edited

Loading

spalger Mar 18, 2020

spalger left a comment •

edited

Loading

spalger left a comment

lukeelmers left a comment

lukeelmers Mar 18, 2020

sulemanof Mar 19, 2020

lukeelmers Mar 18, 2020

lukeelmers Mar 18, 2020

kertal Mar 19, 2020

sulemanof Mar 19, 2020

sulemanof commented Mar 20, 2020

kibanamachine commented Mar 20, 2020

kertal left a comment

[NP] Remove ui/agg_types dependencies and move paginated table to kibana_legacy #60276

[NP] Remove ui/agg_types dependencies and move paginated table to kibana_legacy #60276

Conversation

sulemanof commented Mar 16, 2020 • edited Loading

Summary

For reviewers:

Checklist

For maintainers

elasticmachine commented Mar 18, 2020

sulemanof Mar 18, 2020 • edited Loading

Choose a reason for hiding this comment

spalger Mar 18, 2020

Choose a reason for hiding this comment

spalger left a comment • edited Loading

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

lukeelmers left a comment

Choose a reason for hiding this comment

lukeelmers Mar 18, 2020

Choose a reason for hiding this comment

sulemanof Mar 19, 2020

Choose a reason for hiding this comment

lukeelmers Mar 18, 2020

Choose a reason for hiding this comment

lukeelmers Mar 18, 2020

Choose a reason for hiding this comment

kertal Mar 19, 2020

Choose a reason for hiding this comment

sulemanof Mar 19, 2020

Choose a reason for hiding this comment

sulemanof commented Mar 20, 2020

kibanamachine commented Mar 20, 2020

💚 Build Succeeded

History

kertal left a comment

Choose a reason for hiding this comment

[NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy #60276

[NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy #60276

sulemanof commented Mar 16, 2020 •

edited

Loading

sulemanof Mar 18, 2020 •

edited

Loading

spalger left a comment •

edited

Loading