Move saved objects loader to dashboard plugin by rudolf · Pull Request #124708 · elastic/kibana

rudolf · 2022-02-04T16:22:38Z

Summary

When creating the NP the Core team decided not to port src/legacy/ui/public/saved_objects/ due to concerns with the architecture, code quality and test coverage. In order to unblock teams this code was temporarily moved into a new saved_objects plugin #57452 We've since asked teams to remove dependencies on this plugin #46435

Now that the dashboard plugin is the only consumer of the SavedObjectsLoader I have moved this dependency into the dashboard plugin.

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)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…cts-loader

elasticmachine · 2022-02-08T11:44:01Z

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

elasticmachine · 2022-02-08T11:45:27Z

Pinging @elastic/kibana-presentation (Team:Presentation)

kibana-ci · 2022-02-08T13:16:56Z

💛 Build succeeded, but was flaky

Test Failures

[job] [logs] Default CI Group #15 / machine learning permissions for user with full ML access with no data loaded (ft_ml_poweruser) should display elements on ML Overview page correctly

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	259	261	+2
`savedObjects`	39	37	-2
total			-0

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`savedObjects`	177	151	-26

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`savedObjects`	3	2	-1

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`dashboard`	13	14	+1

Page load bundle

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

id	before	after	diff
`dashboard`	64.5KB	65.8KB	+1.3KB
`savedObjects`	30.5KB	29.1KB	-1.4KB
total			-98.0B

Unknown metric groups

API count

id	before	after	diff
`savedObjects`	221	192	-29

References to deprecated APIs

id	before	after	diff
`dashboard`	84	78	-6

History

💔 Build #22149 failed 13e27003758c2892565e3543b0efe5472a48108c
💔 Build #21886 failed 04782fb

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

ThomThomson

Thanks for doing this @rudolf, we will absolutely be moving away from the Saved Objects Loader class soon - it is on our tech debt roadmap.

Code review only, but all the changes in this PR make sense - LGTM!

pgayvallet

lgtm

pgayvallet · 2022-02-10T09:03:32Z

src/plugins/dashboard/public/services/saved_object_loader.ts

 } from 'kibana/public';
-import { SavedObject } from '../types';
-import { StringUtils } from './helpers/string_utils';
+import { SavedObject } from '../../../saved_objects/public';


NIT: import type (even if the target is an interface)

kibanamachine · 2022-02-10T09:34:18Z

The following labels were identified as gaps in your version labels and will be added automatically:

v8.2.0

If any of these should not be on your pull request, please manually remove them.

kibanamachine · 2022-02-10T09:37:20Z

💚 All backports created successfully

Status	Branch	Result
✅	8.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

* Move SavedObjectLoader into the dashboard plugin * Remove old comment recommending savedobjectloader * Fix types (cherry picked from commit 00fb06c)

rudolf · 2022-02-10T10:02:27Z

Removed Team:Presentation label to see if we can get it to sync to Jira

* Move saved objects loader to dashboard plugin (#124708) * Move SavedObjectLoader into the dashboard plugin * Remove old comment recommending savedobjectloader * Fix types (cherry picked from commit 00fb06c) * Fix types Co-authored-by: Rudolf Meijering <rudolf.meijering@elastic.co> Co-authored-by: Rudolf Meijering <skaapgif@gmail.com>

Move SavedObjectLoader into the dashboard plugin

7c2edab

rudolf changed the title ~~Move saved objects loader~~ Move saved objects loader to dashboard plugin Feb 4, 2022

Remove old comment recommending savedobjectloader

04782fb

rudolf force-pushed the move-saved-objects-loader branch from 24806b9 to 04782fb Compare February 4, 2022 16:27

rudolf added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.1.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes labels Feb 4, 2022

Fix types

ce8d004

rudolf force-pushed the move-saved-objects-loader branch from 13e2700 to ce8d004 Compare February 8, 2022 11:40

Merge branch 'main' of github.com:elastic/kibana into move-saved-obje…

36f98f9

…cts-loader

rudolf marked this pull request as ready for review February 8, 2022 11:43

rudolf requested a review from a team as a code owner February 8, 2022 11:44

rudolf added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// label Feb 8, 2022

ThomThomson approved these changes Feb 8, 2022

View reviewed changes

pgayvallet approved these changes Feb 10, 2022

View reviewed changes

rudolf merged commit 00fb06c into elastic:main Feb 10, 2022

kibanamachine added the backport:skip This PR does not require backporting label Feb 10, 2022

rudolf deleted the move-saved-objects-loader branch February 10, 2022 09:33

kibanamachine added the v8.2.0 label Feb 10, 2022

kibanamachine mentioned this pull request Feb 10, 2022

[8.1] Move saved objects loader to dashboard plugin (#124708) #125203

Merged

rudolf removed the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// label Feb 10, 2022

ThomThomson mentioned this pull request Sep 21, 2022

[Portable Dashboards] Remove Saved Object Class #138774

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move saved objects loader to dashboard plugin#124708

Move saved objects loader to dashboard plugin#124708
rudolf merged 4 commits intoelastic:mainfrom
rudolf:move-saved-objects-loader

rudolf commented Feb 4, 2022

Uh oh!

elasticmachine commented Feb 8, 2022

Uh oh!

elasticmachine commented Feb 8, 2022

Uh oh!

kibana-ci commented Feb 8, 2022

API count

References to deprecated APIs

Uh oh!

ThomThomson left a comment

Uh oh!

pgayvallet left a comment

Uh oh!

pgayvallet Feb 10, 2022

Uh oh!

kibanamachine commented Feb 10, 2022

Uh oh!

kibanamachine commented Feb 10, 2022

Uh oh!

rudolf commented Feb 10, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

rudolf commented Feb 4, 2022

Summary

Checklist

Risk Matrix

For maintainers

Uh oh!

elasticmachine commented Feb 8, 2022

Uh oh!

elasticmachine commented Feb 8, 2022

Uh oh!

kibana-ci commented Feb 8, 2022

💛 Build succeeded, but was flaky

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Public APIs missing exports

Page load bundle

API count

References to deprecated APIs

History

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet Feb 10, 2022

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 10, 2022

Uh oh!

kibanamachine commented Feb 10, 2022

💚 All backports created successfully

Questions ?

Uh oh!

rudolf commented Feb 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rudolf commented Feb 10, 2022 •

edited

Loading