-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Dashboard] Keep managed panels out of unmanaged dashboards #176006
[Dashboard] Keep managed panels out of unmanaged dashboards #176006
Conversation
…anaged-panels-on-clone
/ci |
/ci |
…ewdaemon/kibana into 172383-unlink-managed-panels-on-clone
/ci |
…anaged-panels-on-clone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DataDiscovery code changes LGTM, though it wouldn't hurt to get a second pair of eyes from someone a little more familiar with these areas.
…anaged-panels-on-clone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-only review, just serving as a second set of eyes for @lukasolson. The Data Discovery changes LGTM overall. I left a couple small pieces of feedback, but nothing I'd consider blocking or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! Tested by cloning a managed Dashboard, and by adding managed panels to an unmanaged Dashboard, and checking in the SO Management screen that all of the expected references are present.
Nice work!
|
||
type SavedObjectToPanelMethod<TSavedObjectAttributes, TByValueInput> = ( | ||
savedObject: SavedObjectCommon<TSavedObjectAttributes> | ||
) => { savedObjectId: string } | Partial<TByValueInput>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice generics here! Thanks for the suggestion @davismcphee.
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fantastic my dear Drew! LGTM 🎉
…176006) ## Summary Close elastic#172383 Close elastic#172384 This PR introduces a [new embeddable-related registry](https://github.com/elastic/kibana/pull/176006/files#diff-1401b355377c76ab6458756aa0e3177beef5ec56796c58b7a52b5e003f85b5cf) which clients can use to define a custom transformation from saved object to embeddable input during the add-panel-from-library sequence. Then, each content type uses this to communicate whether a particular object should be added by-ref or by-val based on the presence of `managed: true` on the saved object ([example](https://github.com/elastic/kibana/pull/176006/files#diff-3baaeaeef5893a5a4db6379a1ed888406a8584cb9d0c7440f273040e4aa28166R157-R167)). ### Managed panels are added by-value to dashboards <img width="400" alt="Screenshot 2024-02-01 at 12 24 06 PM" src="https://github.com/elastic/kibana/assets/315764/42a695d4-fccf-45bf-bd6a-8d8fc606d04e"> ### Cloning a managed dashboard inlines all by-ref panels https://github.com/elastic/kibana/assets/315764/ca6e763c-cc02-46cb-9164-abd91deca081 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials — will happen in elastic#175150 - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed — https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5031 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…176006) ## Summary Close elastic#172383 Close elastic#172384 This PR introduces a [new embeddable-related registry](https://github.com/elastic/kibana/pull/176006/files#diff-1401b355377c76ab6458756aa0e3177beef5ec56796c58b7a52b5e003f85b5cf) which clients can use to define a custom transformation from saved object to embeddable input during the add-panel-from-library sequence. Then, each content type uses this to communicate whether a particular object should be added by-ref or by-val based on the presence of `managed: true` on the saved object ([example](https://github.com/elastic/kibana/pull/176006/files#diff-3baaeaeef5893a5a4db6379a1ed888406a8584cb9d0c7440f273040e4aa28166R157-R167)). ### Managed panels are added by-value to dashboards <img width="400" alt="Screenshot 2024-02-01 at 12 24 06 PM" src="https://github.com/elastic/kibana/assets/315764/42a695d4-fccf-45bf-bd6a-8d8fc606d04e"> ### Cloning a managed dashboard inlines all by-ref panels https://github.com/elastic/kibana/assets/315764/ca6e763c-cc02-46cb-9164-abd91deca081 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials — will happen in elastic#175150 - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed — https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5031 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…176006) ## Summary Close elastic#172383 Close elastic#172384 This PR introduces a [new embeddable-related registry](https://github.com/elastic/kibana/pull/176006/files#diff-1401b355377c76ab6458756aa0e3177beef5ec56796c58b7a52b5e003f85b5cf) which clients can use to define a custom transformation from saved object to embeddable input during the add-panel-from-library sequence. Then, each content type uses this to communicate whether a particular object should be added by-ref or by-val based on the presence of `managed: true` on the saved object ([example](https://github.com/elastic/kibana/pull/176006/files#diff-3baaeaeef5893a5a4db6379a1ed888406a8584cb9d0c7440f273040e4aa28166R157-R167)). ### Managed panels are added by-value to dashboards <img width="400" alt="Screenshot 2024-02-01 at 12 24 06 PM" src="https://github.com/elastic/kibana/assets/315764/42a695d4-fccf-45bf-bd6a-8d8fc606d04e"> ### Cloning a managed dashboard inlines all by-ref panels https://github.com/elastic/kibana/assets/315764/ca6e763c-cc02-46cb-9164-abd91deca081 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials — will happen in elastic#175150 - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed — https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5031 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
Close #172383
Close #172384
This PR introduces a new embeddable-related registry which clients can use to define a custom transformation from saved object to embeddable input during the add-panel-from-library sequence.
Then, each content type uses this to communicate whether a particular object should be added by-ref or by-val based on the presence of
managed: true
on the saved object (example).Managed panels are added by-value to dashboards
Cloning a managed dashboard inlines all by-ref panels
Screen.Recording.2024-02-01.at.3.07.47.PM.mov
Testing material
Here's a dashboard with a bunch of managed by-ref panels.
Checklist
Delete any items that are not applicable to this PR.