[Embeddable Rebuild] Refactor enhanced embeddables by Heenawter · Pull Request #178896 · elastic/kibana

Heenawter · 2024-03-18T17:43:52Z

Summary

In the old embeddable system, when an embeddable is created through its factory, it is "enhanced" via enhanceEmbeddableWithDynamicActions in EmbeddableEnhancedPlugin - this enhancement is responsible for adding the pieces necessary (for example, the DynamicActionManager) to the embeddable to manage drilldowns. In the new system, we are no longer using these class-based factories - so there is no longer a way to enhance embeddables on creation. Therefore, this PR does three main things:

It adds a new method initializeDynamicActions to the EnhancedEmbeddablePlugin and returns it through its start contract (in order to bypass limitations around importing stuff from xpack) - this method can be used to enhance all React-based embeddables with the stuff necessary for drilldowns to work. As part of this, I had to add a few extra pieces to the HasDynamicActions API:
- dynamicActionsState$: This is a publishing subject that will keep track of the events currently attached to the given embeddable (DynamicActionsState)
- setDynamicActions: A setter for the above publishing subject
It modifies all actions tied to dashboard drilldowns - PanelNotificationsAction, EmbeddableToDashboardDrilldown, FlyoutEditDrilldownAction, and FlyoutCreateDrilldownAction.
It modifies the old enhanceEmbeddableWithDynamicActions to add the publishing subject + setter from (1) above to all legacy embeddables when they are enhanced - that way, they will still pass the apiHasDynamicActions type check + all modified actions continue to work for legacy embeddables.

How to Test
To test with a React embeddable, you can checkout the draft PR containing the new image embeddable.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

Heenawter · 2024-03-18T22:15:34Z

/ci

Heenawter · 2024-03-19T14:35:06Z

/ci

Heenawter · 2024-03-19T17:46:41Z

/ci

Heenawter · 2024-03-19T20:13:49Z

/ci

Heenawter · 2024-03-19T20:21:17Z

/ci

Heenawter · 2024-03-19T21:22:08Z

/ci

…fix'

Heenawter · 2024-03-20T16:05:09Z

/ci

Heenawter · 2024-03-20T18:45:19Z

...ns/dashboard_enhanced/public/services/drilldowns/actions/flyout_edit_drilldown/menu_item.tsx

+}: {
+  context: { embeddable: FlyoutEditDrilldownActionApi };
+}) => {
+  const dynamicActionsState = useStateFromPublishingSubject(embeddable.dynamicActionsState$);


Using the state container from DynamicActionManager was falling out of sync, so the badge beside the "edit" menu option would often reflect the wrong number (especially after the dashboard was reset) - switching to the dynamicActionsState$ publishing subject, which should be our source of truth, fixed this.

Heenawter · 2024-03-20T18:56:11Z

x-pack/plugins/embeddable_enhanced/public/actions/panel_notifications_action.ts

+    if (!isApiCompatible(embeddable)) return;
+
+    return merge(
+      getViewModeSubject(embeddable) ?? new BehaviorSubject(ViewMode.VIEW),


Not sure how to better handle optional behaviour subjects?

Heenawter · 2024-03-20T18:57:03Z

/ci

elasticmachine · 2024-03-20T20:11:33Z

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

nreese

LGTM with a few nits about typings. This is a heavy lift and your changes are really streamlined and easy to follow. Great job bringing drilldowns across the line for react embeddables.
code review, tested in chrome

...nced/public/services/drilldowns/actions/flyout_edit_drilldown/flyout_edit_drilldown.test.tsx

...shboard_enhanced/public/services/drilldowns/actions/flyout_edit_drilldown/menu_item.test.tsx

x-pack/plugins/embeddable_enhanced/public/actions/panel_notifications_action.test.ts

x-pack/plugins/embeddable_enhanced/public/embeddables/dynamic_action_storage.test.ts

x-pack/plugins/embeddable_enhanced/public/embeddables/dynamic_action_storage.ts

x-pack/plugins/embeddable_enhanced/public/plugin.ts

kibana-ci · 2024-03-21T23:04:50Z

💚 Build Succeeded

Buildkite Build
Commit: c07c945

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`embeddableEnhanced`	11	32	+21

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
`embeddableEnhanced`	18	23	+5

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
`embeddableEnhanced`	0	1	+1

Page load bundle

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

id	before	after	diff
`dashboardEnhanced`	14.3KB	14.6KB	+348.0B
`embeddableEnhanced`	7.2KB	9.9KB	+2.8KB
total			+3.1KB

Unknown metric groups

API count

id	before	after	diff
`embeddableEnhanced`	18	23	+5

ESLint disabled line counts

id	before	after	diff
`embeddableEnhanced`	3	2	-1

References to deprecated APIs

id	before	after	diff
`dashboardEnhanced`	15	13	-2

Total ESLint disabled count

id	before	after	diff
`embeddableEnhanced`	5	4	-1

Unreferenced deprecated APIs

id	before	after	diff
`embeddableEnhanced`	0	1	+1

History

💔 Build #198842 failed d7c452a872443d5e8f309bd8e134dc2ba52327cd
💔 Build #198800 failed 5710d14
💚 Build #198599 succeeded 56411ca
💛 Build #198562 was flaky 391d8d9
💔 Build #198402 failed 77a26efcbba472876eb09191c8100094a7ab8a5c
💔 Build #198387 failed 22d0fe54d36942b1879ef74ef2152918fd507b23

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

cc @Heenawter

…11678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in #180536 (related PR where drilldowns / dynamic actions were refactored: #178896). A new functional test has also been added to prevent future regressions. Fixes #211677. ### Checklist - [ ] 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/src/platform/packages/shared/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 - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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/src/platform/packages/shared/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 - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d)

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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/src/platform/packages/shared/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 - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d) # Conflicts: # src/platform/plugins/shared/discover/public/types.ts # src/platform/plugins/shared/discover/tsconfig.json

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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/src/platform/packages/shared/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 - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d) # Conflicts: # src/plugins/discover/public/build_services.ts # src/plugins/discover/public/embeddable/get_search_embeddable_factory.tsx # src/plugins/discover/public/embeddable/types.ts # src/plugins/discover/public/embeddable/utils/serialization_utils.test.ts # src/plugins/discover/public/types.ts # src/plugins/discover/tsconfig.json

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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/src/platform/packages/shared/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 - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

Heenawter self-assigned this Mar 18, 2024

Heenawter force-pushed the refactor-dashboard-drilldowns_2024-03-18 branch 3 times, most recently from 6c67ac5 to fb1ba5e Compare March 18, 2024 22:05

Heenawter force-pushed the refactor-dashboard-drilldowns_2024-03-18 branch 2 times, most recently from 6150063 to 0cd10cd Compare March 19, 2024 17:23

Heenawter added the project:embeddableRebuild label Mar 19, 2024

Heenawter force-pushed the refactor-dashboard-drilldowns_2024-03-18 branch from b7bb7a6 to 22d0fe5 Compare March 19, 2024 20:20

Heenawter and others added 12 commits March 20, 2024 10:04

First draft of refactoring embeddable enhanced

c4ada0a

Keep edit action in sync

48b1abd

Fix undefined events errors

19b27b1

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

9c764ca

…fix'

Do some cleanup work

b7eddad

Try to fix typing

c2acba8

Fix action storage tests

6b09de6

Fix flyout tests

8f21c19

Rename embeddable action storage

0857da7

Fix export

4787801

More test fixes + cleanup

4bf8240

Fix mock

391d8d9

Heenawter commented Mar 20, 2024

View reviewed changes

Final cleanup

56411ca

Heenawter marked this pull request as ready for review March 20, 2024 20:11

Heenawter requested a review from a team as a code owner March 20, 2024 20:11

nreese self-requested a review March 21, 2024 14:37

nreese approved these changes Mar 21, 2024

View reviewed changes

botelastic bot added the Feature:Drilldowns Embeddable panel Drilldowns label Mar 21, 2024

Heenawter added 3 commits March 21, 2024 12:14

First round of feedback

343e464

Rename getEventsFromEmbeddable -> getEvents

5710d14

Fix type error

c07c945

Heenawter force-pushed the refactor-dashboard-drilldowns_2024-03-18 branch from d7c452a to c07c945 Compare March 21, 2024 21:58

Heenawter enabled auto-merge (squash) March 21, 2024 22:01

Heenawter merged commit 3f1dcff into elastic:main Mar 21, 2024

kibanamachine added v8.14.0 backport:skip This PR does not require backporting labels Mar 21, 2024

Heenawter deleted the refactor-dashboard-drilldowns_2024-03-18 branch March 21, 2024 23:06

ThomThomson mentioned this pull request Mar 22, 2024

[Embeddables Rebuild] Enhance BuildEmbeddable function #179205

Merged

thomheymann mentioned this pull request Mar 26, 2024

[Observability Onboarding] Link all “Add data” buttons in observability to onboarding landing page #179459

Merged

davismcphee mentioned this pull request Feb 19, 2025

[Discover] [Embeddable] Fix Discover session embeddable drilldown #211678

Merged

7 tasks

Conversation

Heenawter commented Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

Heenawter commented Mar 18, 2024

Uh oh!

Heenawter commented Mar 19, 2024

Uh oh!

Heenawter commented Mar 19, 2024

Uh oh!

Heenawter commented Mar 19, 2024

Uh oh!

Heenawter commented Mar 19, 2024

Uh oh!

Heenawter commented Mar 19, 2024

Uh oh!

Heenawter commented Mar 20, 2024

Uh oh!

Heenawter Mar 20, 2024

Choose a reason for hiding this comment

Uh oh!

Heenawter Mar 20, 2024

Choose a reason for hiding this comment

Uh oh!

Heenawter commented Mar 20, 2024

Uh oh!

elasticmachine commented Mar 20, 2024

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibana-ci commented Mar 21, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

Unreferenced deprecated APIs

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Heenawter commented Mar 18, 2024 •

edited

Loading