[dashboards as code] drilldown schemas#249445
Conversation
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
|
/ci |
|
/ci |
|
@elasticmachine merge upstream |
|
|
||
| const drilldownsFromEnhancements = enhancements.dynamicActions.events | ||
| .map((event) => { | ||
| if (event.action.factoryId === 'DASHBOARD_TO_DASHBOARD_DRILLDOWN') { |
There was a problem hiding this comment.
I wonder if we should be using constants instead of hard-coding the strings here? I see this constant is defined in the dashboard_enhanced plugin, but I expect that plugin will be removed in a subsequent PR?
There was a problem hiding this comment.
I see this constant is defined in the dashboard_enhanced plugin, but I expect that plugin will be removed in a subsequent PR?
That is correct. BWC code should be isolated and not import from other places to ensure the functionality remains the same. In this case, dashboard_enhanced, will be removed when client gets migrated to drilldown schema.
For BWC, I think hard coded strings are more clear and since its BWC, we know the string value will never change so there is no advantage for a constant.
src/platform/plugins/shared/embeddable/public/bwc/legacy_url_transform.ts
Show resolved
Hide resolved
…1337) There is currently an issue in main where `findLensReference` does not find a lens reference because the reference name is `panel_5` instead of `savedObjectRef`. This causes the transform out to throw because `savedObjectRef && isByRefLensState(state)` check fails and call to `const migratedAttributes = migrateAttributes(state.attributes);` throws. I discovered this because x-pack/platform/test/functional/apps/dashboard/group2/migration_smoke_tests/lens_migration_smoke_test.ts started failing in #249445. With #249445, throwing in lens transforms prevents enhancements from getting converted to drilldowns so the functional test fails because the lens state is `{ enhancements }` instead of `{ drilldowns }`. The root of the problem is that `transformPanelProperties` injects by-reference reference. This is left over from client side injection. The problem is resolved by transforming the reference to what transforms expect and stop injecting references in `transformPanelProperties`. --------- Co-authored-by: Devon Thomson <devon.thomson@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
x-pack/platform/plugins/private/drilldowns/url_drilldown/common/constants.ts
Outdated
Show resolved
Hide resolved
| // Do not change constan value - part of public REST API | ||
| export const DISCOVER_DRILLDOWN_TYPE = 'discover_drilldown'; | ||
| // Only additive changes are allowed, part of public REST API | ||
| export const DISCOVER_DRILLDOWN_SUPPORTED_TRIGGERS = [APPLY_FILTER_TRIGGER]; |
There was a problem hiding this comment.
Great to have these comments.
⏳ Build in-progress, with failures
Failed CI StepsTest Failures
History
|
nickpeihl
left a comment
There was a problem hiding this comment.
lgtm! code review only.
davismcphee
left a comment
There was a problem hiding this comment.
Data Discovery changes LGTM 👍
Dosant
left a comment
There was a problem hiding this comment.
i smoked checked image, url drilldowns. lgtm
andrimal
left a comment
There was a problem hiding this comment.
Visualization changes LGTM!
There was a problem hiding this comment.
nit: I think I would keep the index.ts to the transforms directory and export getTransformIn and getTransformOut from there. This would be consistent with other directories within lens and give us a cleaner import path.
There was a problem hiding this comment.
I opted to directly import getTransformOut only because getTransform requires both transformDrilldownOut and trasnformDrilldownIn. In public, only transformDrilldownOut is available and only transformDrilldownOut is needed.
…hema types (#252976) Follow up to #249445 Part of #239425 Kibana-presentation team is working on "Dashboards as code" project. #249445 created a schema for drilldowns and updated Dashboard REST API to use new schema defined shape for drilldowns. #249445 did not touch client code - leaving client code using old shape. This PR takes the next step and refactors client code to use new schema defined drilldowns shape. ### Overview of changes * Decouples drilldowns from dynamic actions * Moves registryDrilldown from uiActionsEnhanced plugin to embeddables plugin * Moves DrilldownsManager UI components from uiActionsEnhanced plugin to embeddables plugin. The effort tried to change as little as possible in DrilldownsManager UI components. * Removes dashboardEnhanced plugin * Moves createDrilldowns action and manageDrilldowns action registration into embeddable plugin * Moves dashboard drilldown registation into drilldown plugin * Removes emebbeddableEnhanced plugin * Replaces `HasDynamicActions` interface with `HasDrilldown` interface and moves this interface into embeddable plugin * Replaces `initializeDynamicActionsManager` with `initializeDrilldownsManager` * Moves `initializeDrilldownsManager` as parameter passed to `buildEmbeddable` instead of exposing via plugin start contract. * Updates all embeddables to `initializeDrilldownsManager` * Moves DiscoverDrilldown to embeddables plugin `registerDrilldown` * Moves URLDrilldown to embeddables plugin `registerDrilldown` ### What this PR does not do The uiActionsEnhanced plugin can be removed now that drilldowns no longer use dynamic actions. This is clean up work that will occur in future efforts. URLDrilldown is still very intertwined with uiActionsEnhancement plugin. These 2 plugins will need to be separated in future work. ### Embeddable owner test instructions * Create new dashboard * add your embeddable panel * add drilldowns. Ensure they work as expected ### Drilldown owner test instructions * Create new dashboard * add an embeddable panel that supports your drilldown type * add drilldowns. Ensure they work as expected --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Anton Dosov <dosantappdev@gmail.com>
…hema types (elastic#252976) Follow up to elastic#249445 Part of elastic#239425 Kibana-presentation team is working on "Dashboards as code" project. elastic#249445 created a schema for drilldowns and updated Dashboard REST API to use new schema defined shape for drilldowns. elastic#249445 did not touch client code - leaving client code using old shape. This PR takes the next step and refactors client code to use new schema defined drilldowns shape. ### Overview of changes * Decouples drilldowns from dynamic actions * Moves registryDrilldown from uiActionsEnhanced plugin to embeddables plugin * Moves DrilldownsManager UI components from uiActionsEnhanced plugin to embeddables plugin. The effort tried to change as little as possible in DrilldownsManager UI components. * Removes dashboardEnhanced plugin * Moves createDrilldowns action and manageDrilldowns action registration into embeddable plugin * Moves dashboard drilldown registation into drilldown plugin * Removes emebbeddableEnhanced plugin * Replaces `HasDynamicActions` interface with `HasDrilldown` interface and moves this interface into embeddable plugin * Replaces `initializeDynamicActionsManager` with `initializeDrilldownsManager` * Moves `initializeDrilldownsManager` as parameter passed to `buildEmbeddable` instead of exposing via plugin start contract. * Updates all embeddables to `initializeDrilldownsManager` * Moves DiscoverDrilldown to embeddables plugin `registerDrilldown` * Moves URLDrilldown to embeddables plugin `registerDrilldown` ### What this PR does not do The uiActionsEnhanced plugin can be removed now that drilldowns no longer use dynamic actions. This is clean up work that will occur in future efforts. URLDrilldown is still very intertwined with uiActionsEnhancement plugin. These 2 plugins will need to be separated in future work. ### Embeddable owner test instructions * Create new dashboard * add your embeddable panel * add drilldowns. Ensure they work as expected ### Drilldown owner test instructions * Create new dashboard * add an embeddable panel that supports your drilldown type * add drilldowns. Ensure they work as expected --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Anton Dosov <dosantappdev@gmail.com>
Part of #239425
Kibana-presentation team is working on "Dashboards as code" project. Dashboard REST API state contains drilldown state. This PR refactors drilldown state and provides schemas for drilldown state.
Before
In the image below, notice how drilldown state is passed as

{ enhancements: { dynamicActions: events: [ { ...messyStateWithClassStructures } ] } }. This shape leaks Kibana registry design and class structures into dashboards as code REST API.After
In the image below, notice how drilldown state is passed as

{ drilldowns: [ {...cleanState } ] }. Now the dashboards as code REST API does not leak Kibana registry design or class structures.Overview of changes
registerDrilldown, allowing drilldowns to register schemas and transforms.registerTransformsshape from{ transformIn, transformOut, ...otherProps }to{ getTransforms, ...otherProps }. This change is done to passdrilldownTransformsto embeddable transform implementations.drilldownTransformsis stateful because it uses the contents of drilldown registry.transformEnhancementsInandtransformEnhancementsOutfrom interface. Instead of exposing drilldown tranforms directly from EmbeddableServerSetup,drilldownTransformsis provided to embeddables as a prop togetTransformsregisterTransformsto provide{ getTransforms, ...otherProps }instead of{ transformIn, transformOut, ...otherProps }transformEnhancementsInandtransformEnhancementsOutto usedrilldownTransformsWhat this PR does not do
To limit scope as much as possible, this PR does not refactor client code to use the new drilldown format. As a stop gap, a translation layer is in place that converts
drilldownstoenhancements. Then, on save,enhancementsare converted back todrilldownsbefore sending state across the dashboard REST API. In a follow up PR(s), this translation layer will be removed and drilldown client code will be refactored to work directly withDrilldownsStateExternal team testing
If your embebbable type does not support drilldowns then there is nothing to test.
If your embeddable type does support drilldowns, then run the following
/api/dashboards). Notice that drilldowns are passed asdrilldownsinstead ofenhancements.