Use saved object references for dashboard drilldowns by Dosant · Pull Request #82602 · elastic/kibana

Dosant · 2020-11-04T13:36:12Z

Summary

This PR integrates embeddables persistable state service with dashboard app to handle references to other dashboards created by dashboard drilldowns.

In addition to extracting and injecting references on a client-side dashboard, this pr also adds a 7.11.0 dashboard migration that extracts dashboardIds from existing drilldowns and puts them to dashboard's references. To achieve this bunch of client-side dashboard/embeddable code had to be moved to common to be reused on the client-side.

Release Notes

Dashboards connected via drilldowns are now exported or copied to a different space together.

Checklist

Delete any items that are not applicable to this PR.

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

Dosant · 2020-11-05T12:16:23Z

src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts

Not sure about this, but this seem to be working.
I had to decouple injectReferences from client side DashboardSavedObject class to reuse this on server.
Don't see any other way

Dosant · 2020-11-05T12:17:36Z

...oard_enhanced/common/drilldowns/dashboard_drilldown/dashboard_drilldown_persistable_state.ts

Do we have a naming convention for reference names?

not yet, i agree it would be useful to have one

ppisljar · 2020-11-05T14:15:45Z

src/plugins/dashboard/common/saved_dashboard_references.ts

can we create an issue to move this inside embeddable reference extraction (rather than having it here on dashboard) ?

I added a comment and created a placeholder issue: 9cd13a8

I am not 100% sure though that it is possible without breaking changes and if this is required. This extract/inject now operates on dashboard panel specific data model and not on embeddable model.

Dosant · 2020-11-06T15:18:32Z

src/plugins/dashboard/common/embeddable/embeddable_saved_object_converters.ts

    gridData: panelState.gridData,
    panelIndex: panelState.explicitInput.id,
-    embeddableConfig: omit(panelState.explicitInput, ['id', 'savedObjectId']),
+    embeddableConfig: omit(panelState.explicitInput, ['id', 'savedObjectId', 'title']),


nit:
I noticed that title was not omitted even though it is copied on a root level of the object.
This is causing duplicate title in each panel of dashboard SO which caught my eye in new unit tests.

Dosant · 2020-11-06T15:21:12Z

src/plugins/dashboard/common/saved_dashboard_references.test.ts

+      Object {
+        "attributes": Object {
+          "foo": true,
+          "panelsJSON": "[{\\"embeddableConfig\\":{},\\"title\\":\\"Title 1\\",\\"panelRefName\\":\\"panel_0\\"},{\\"embeddableConfig\\":{},\\"title\\":\\"Title 2\\",\\"panelRefName\\":\\"panel_1\\"}]",


In each of these snapshots embeddableConfig:{} is added because now underlying code converts to EmbeddablePanel and then back. We don't clean up embeddableConfig when we convert from EmbeddablePanel to DashboardSavedPanel. I think having embeddableConfig is technically more correct and initial doc is just not in a complete shape.

Other option could be to remove embeddableConfig in convertSavedDashboardPanelToPanelState if there is no keys.

Dosant · 2020-11-06T15:22:29Z

src/plugins/dashboard/common/types.ts

+export type PanelId = string;
+export type SavedObjectId = string;
+
+export interface DashboardPanelState<


Had to move this to common from public/type/i_container
Didn't move the whole file because it contains references to client side types

Dosant · 2020-11-06T15:26:03Z

x-pack/plugins/dashboard_enhanced/server/plugin.ts

+    plugins.uiActionsEnhanced.registerActionFactory({
+      id: EMBEDDABLE_TO_DASHBOARD_DRILLDOWN,
+      inject: createInject({ drilldownId: EMBEDDABLE_TO_DASHBOARD_DRILLDOWN }),
+      extract: createExtract({ drilldownId: EMBEDDABLE_TO_DASHBOARD_DRILLDOWN }),


This is an important bit:
We register a EMBEDDABLE_TO_DASHBOARD_DRILLDOWN server side counter part which provides extract/inject functions which will be used by dashboard migrations via EmbeddablePersistableStateService

Dosant · 2020-11-06T15:29:11Z

x-pack/test/functional/es_archives/spaces/copy_saved_objects/data.json

        "hits": 0,
        "description": "",
-        "panelsJSON": "[{}]",
+        "panelsJSON": "[]",


{} is definitely invalid panel shape :D
New dashboard migration failed because of this.
Previously this would have failed on dashboard app open.

Alternatively we could make sure that migration can swallow {}, but I am not sure what is a recommended approach?

Similarly there is single change in x-pack/test/functional/es_archives/reporting/hugedata/data.json.gz which started failing during import. There was a typo in a reference name: Fanel_1 reference instead of panel_1 which led to test failures.

I just updated the data as I guess we should fail the migration in such case.

…-drilldown-so2

Dosant · 2020-11-07T06:59:49Z

...egration/apis/fixtures/test_packages/all_assets/0.1.0/kibana/dashboard/sample_dashboard.json

    "version": 1
  },
+  "references": [
+    { "id": "sample_visualization", "name": "panel_0", "type": "visualization" },


Same as #82602 (comment) and other x-pack/test/ingest_manager_api_integration/apis/fixtures/test_packages/all_assets/*. The sample dashboard JSON was incorrect as was missing references array and new migration failed when importing these sample dashboards.

proc [kibana] log [19:25:34.199] [error][ingestManager][plugins] Error: Could not find reference "panel_0" [00:05:19] │ proc [kibana] at panels.forEach.panel (/dev/shm/workspace/kibana-build-xpack-9/src/plugins/dashboard/common/saved_dashboard_references.js:105:13)

elasticmachine · 2020-11-07T08:44:25Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

…-drilldown-so2

ppisljar

code LGTM

ThomThomson

Code LGTM - plenty of test coverage! Tested in chrome and everything still works as expected. Ensured saved object references were being registered properly for dashboard drilldowns. Didn't test the migrations, but the tests seem robust!

bhavyarm

LGTM

bhavyarm · 2020-11-12T00:39:48Z

Tested the following cases after creating drilldowns in 7.10 and running ES 8.0.0 with data pointed towards 7.10. Thanks @ThomThomson cc @LeeDr

Dashboards created with drilldowns in 7.10 (exported from 6.8.13) work fine in 8.0.
Exported dashboard saved object is successfully imported and drilldowns are fine
URL drilldowns are working
dashboard drilldowns are working
Regular dashboards without drilldowns are working fine
Drilldowns show up correctly in relationship table on saved objects
Can copy drilldown dashboards between spaces
Can export drill downs ndjson and import it successfully again

Dosant · 2020-11-12T11:41:54Z

@elasticmachine merge upstream

kibanamachine · 2020-11-12T13:37:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8cddea3

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	169	170	+1
`dashboardEnhanced`	28	34	+6
total			+7

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	198.1KB	195.9KB	-2.2KB

Distributable file count

id	before	after	diff
`default`	42775	42789	+14
`oss`	22459	22464	+5

Page load bundle

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

id	before	after	diff
`dashboard`	311.9KB	320.8KB	+8.9KB
`dashboardEnhanced`	31.1KB	34.8KB	+3.7KB
`embeddable`	221.6KB	221.5KB	-22.0B
total			+12.6KB

History

💚 Build #87056 succeeded 57ea0d9
💚 Build #86579 succeeded e1aef10
💛 Build #86539 was flaky 4441cc1
💔 Build #86481 failed 3eb2c39
💔 Build #86426 failed d67b343

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

* master: [Ingest Manager] Lift up registry/{stream,extract} functions (elastic#83239) [Reporting] Move "common" types and constants to allow cross-plugin integration (elastic#83198) [Lens] Add suffix formatter (elastic#82852) [App Search] Version documentation links (elastic#83245) Use saved object references for dashboard drilldowns (elastic#82602) Btsymbala/registered av (elastic#81910) [APM] Errors table for service overview (elastic#83065)

Dosant changed the title ~~Dev/dashboard drilldown so 2~~ Use saved object references for dashboard drilldowns Nov 5, 2020

Dosant commented Nov 5, 2020

View reviewed changes

Dosant requested a review from ppisljar November 5, 2020 12:18

ppisljar reviewed Nov 5, 2020

View reviewed changes

Dosant closed this Nov 6, 2020

Dosant force-pushed the dev/dashboard-drilldown-so-2 branch from 9cd13a8 to 03ee1a6 Compare November 6, 2020 15:06

Use saved object references for dashboard drilldowns

d67b343

Dosant reopened this Nov 6, 2020

Dosant commented Nov 6, 2020

View reviewed changes

Dosant added Feature:Dashboard Dashboard related features Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame release_note:enhancement Team:AppArch v7.11.0 v8.0.0 labels Nov 6, 2020

Dosant added 3 commits November 6, 2020 18:48

Merge branch 'master' of github.com:elastic/kibana into dev/dashboard…

3eb2c39

…-drilldown-so2

fix corrupted dashboard object

4441cc1

Merge branch 'master' of github.com:elastic/kibana into dev/dashboard…

e1aef10

…-drilldown-so2

Dosant commented Nov 7, 2020

View reviewed changes

Dosant marked this pull request as ready for review November 7, 2020 08:44

Dosant requested review from a team as code owners November 7, 2020 08:44

Dosant mentioned this pull request Nov 9, 2020

Dashboard panel references to use embeddable persistable state service #82830

Closed

Dosant requested a review from ThomThomson November 9, 2020 12:29

Merge branch 'master' of github.com:elastic/kibana into dev/dashboard…

57ea0d9

…-drilldown-so2

ppisljar approved these changes Nov 10, 2020

View reviewed changes

ThomThomson approved these changes Nov 10, 2020

View reviewed changes

Dosant requested a review from bhavyarm November 11, 2020 16:35

bhavyarm approved these changes Nov 12, 2020

View reviewed changes

Merge branch 'master' into dev/dashboard-drilldown-so-2

8cddea3

Dosant merged commit eaa6553 into elastic:master Nov 12, 2020

Dosant added a commit to Dosant/kibana that referenced this pull request Nov 12, 2020

Use saved object references for dashboard drilldowns (elastic#82602)

9b517ae

Dosant mentioned this pull request Nov 12, 2020

[7.x] Use saved object references for dashboard drilldowns (#82602) #83293

Merged

Dosant added a commit that referenced this pull request Nov 12, 2020

Use saved object references for dashboard drilldowns (#82602) (#83293)

aec05c4

crob611 mentioned this pull request Feb 19, 2021

[Dashboard] Export appropriate references from byValue panels #91567

Merged

Dosant mentioned this pull request Mar 11, 2021

Fix importing dashboards created before ~6.1.0 #94332

Merged

2 tasks

Conversation

Dosant commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release Notes

Checklist

For maintainers

Uh oh!

Dosant Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dosant Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dosant Nov 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 7, 2020

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

bhavyarm left a comment

Choose a reason for hiding this comment

Uh oh!

bhavyarm commented Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dosant commented Nov 12, 2020

Uh oh!

kibanamachine commented Nov 12, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Dosant commented Nov 4, 2020 •

edited

Loading

Dosant Nov 5, 2020 •

edited

Loading

Dosant Nov 6, 2020 •

edited

Loading

Dosant Nov 7, 2020 •

edited

Loading

bhavyarm commented Nov 12, 2020 •

edited

Loading