[Dashboards] Add getSerializedState method to Dashboard API by nickpeihl · Pull Request #204140 · elastic/kibana

nickpeihl · 2024-12-12T21:49:20Z

Summary

Adds a getSerializedState method to the Dashboard API.

This can be used to retrieve a serialized form of the Dashboard that can be used with the Dashboard HTTP API. The "Export" flyout is a possible future consumer of this method.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

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
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
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 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

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

See some risk examples
...

elasticmachine · 2024-12-12T21:49:22Z

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

nreese · 2024-12-13T13:25:56Z

src/plugins/dashboard/public/dashboard_api/get_dashboard_api.ts

      unifiedSearchManager.internalApi.controlGroupReload$,
      unifiedSearchManager.internalApi.panelsReload$
    ).pipe(debounceTime(0)),
+    getSerializedState: async () => {


#203662 made getState sync so this method could now become sync as well.

Oh good call! Updated in b58c33d.

Unfortunately, the getSerializedState method still needs to be async for now since the getDashboardState method on the CM service relies on a Promise to extract searchSource references. But this might change when we move reference handling to the server in #192758.

Unfortunately, the `getSerializedState` method still needs to be async since the `getDashboardState` method on the CM service is a promise. But this might change when we move reference handling to the server in elastic#192758.

nreese · 2024-12-13T15:27:20Z

...ns/dashboard/public/services/dashboard_content_management_service/lib/get_dashboard_state.ts

+  }
+};
+
+export const getDashboardState = async ({


Why is this part of content management service instead of just a function that can be imported where needed? Does it need to be part of content management service?

It does not need to be on content management service. I moved the module into the dashboard_api directory in ed68550.

nreese · 2024-12-13T15:29:36Z

...s/dashboard/public/services/dashboard_content_management_service/lib/save_dashboard_state.ts

-    ...(prefixedPanelReferences ?? []),
-    ...(controlGroupReferences ?? []),
-  ];
+  const { attributes, references } = await getDashboardState({


Should save methods be updated to just take output of getDashboardState instead of calling getDashboardState internally.

I think it would be better if getDashboardState lived in dashboard_api folder and any place that needed serialized state would just get passed { attributes: DashboardAttributes; references: SavedObjectReference[]; }

I like this idea a lot, but I think I would like to scope that in a different PR. Making those changes in this PR would greatly increase the number of changes required for review.

nreese · 2024-12-17T17:25:54Z

src/plugins/dashboard/public/dashboard_api/get_dashboard_state.ts

+  }
+};
+
+export const getDashboardState = ({


Can this be renamed to getSerializedState (and rename file to get_serialized_state)? That will avoid ambiguity of what state this is returning.

nreese · 2024-12-17T17:31:02Z

src/plugins/dashboard/public/dashboard_api/get_dashboard_state.test.ts

+      panelReferences: [],
+    });
+
+    expect(result.attributes.panels).toEqual([]);


How about asserting the entire result.attributes

nreese · 2024-12-17T17:33:21Z

src/plugins/dashboard/public/dashboard_api/get_dashboard_state.test.ts

+    const dashboardState = {
+      ...getSampleDashboardState(),
+      panels: { oldPanelId: { type: 'visualization' } },
+    } as unknown as DashboardState;


Can you move the cast from the entire dashboard state to just panel

const dashboardState = { ...getSampleDashboardState(), panels: { oldPanelId: { type: 'visualization' } as DashboardPanelState }, };

nreese · 2024-12-17T17:36:25Z

src/plugins/dashboard/public/dashboard_api/get_dashboard_state.test.ts

+      ],
+    });
+
+    expect(result.attributes.panels).toEqual(


Why don't you mock v4 so you know the new id. Then you can assert attributes.panels has the right shape.

jest.mock('uuid', () => ({ v4: jest.fn().mockReturnValue('12345'), }));

expect(result.attributes.panels).toEqual({ ...expectedValue});

nreese · 2024-12-17T17:41:41Z

src/plugins/dashboard/public/dashboard_api/open_save_modal.tsx

            panelReferences,
            saveOptions,
-            currentState: dashboardStateToSave,
+            dashboardState: dashboardStateToSave,


searchSourceReferences is not passed to saveDashboardState call

…ializedState

nreese

LGTM
code review only

nreese · 2024-12-19T15:01:36Z

src/plugins/dashboard/public/dashboard_api/get_serialized_state.test.ts

+
+  it('should return the current state attributes and references', async () => {
+    const dashboardState = getSampleDashboardState();
+    const result = await getSerializedState({


Calls to getSerializedState should not be async. Also, all test cases do not need to be async

…-fix'

ThomThomson · 2024-12-19T18:30:28Z

@elasticmachine merge upstream

elasticmachine · 2024-12-19T20:17:49Z

💚 Build Succeeded

Buildkite Build
Commit: 16e29ae

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	696	697	+1

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
`dashboard`	111	113	+2

Async chunks

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

id	before	after	diff
`dashboard`	667.8KB	668.1KB	+335.0B

Unknown metric groups

API count

id	before	after	diff
`dashboard`	114	117	+3

History

💔 Build #262430 failed 535ffd2
💔 Build #262400 failed 3dc3af4
💔 Build #262389 failed 3bcba56
💔 Build #261112 failed ed68550
💚 Build #260334 succeeded b58c33d
💛 Build #260114 was flaky a9159a4

kibanamachine · 2024-12-19T20:25:06Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12420230354

kibanamachine · 2024-12-19T20:30:21Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 204140

Questions ?

Please refer to the Backport tool documentation

nickpeihl · 2024-12-19T20:40:17Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

…204140) Adds a `getSerializedState` method to the Dashboard API. (cherry picked from commit d7280a1)

…04140) (#205010) # Backport This will backport the following commits from `main` to `8.x`: - [[Dashboards] Add getSerializedState method to Dashboard API (#204140)](#204140)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)

…204140) Adds a `getSerializedState` method to the Dashboard API.

nickpeihl added 2 commits December 12, 2024 14:53

Add Export flyout to dashboards

cca4f65

Add getSerializedState method to dashboard API

a9159a4

nickpeihl added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.0.0 v8.18.0 labels Dec 12, 2024

nickpeihl requested a review from a team as a code owner December 12, 2024 21:49

nickpeihl added backport:prev-minor release_note:skip Skip the PR/issue when compiling release notes labels Dec 12, 2024

Merge branch 'main' into dashboardApi-getSerializedState

6b6818f

nreese reviewed Dec 13, 2024

View reviewed changes

getState is synchronous

b58c33d

Unfortunately, the `getSerializedState` method still needs to be async since the `getDashboardState` method on the CM service is a promise. But this might change when we move reference handling to the server in elastic#192758.

nreese reviewed Dec 13, 2024

View reviewed changes

Make getSerializedState method synchronous

ed68550

thomasneirynck mentioned this pull request Dec 16, 2024

[Dashboard] View source - DRAFT #201690

Closed

9 tasks

nreese reviewed Dec 17, 2024

View reviewed changes

nickpeihl added 2 commits December 19, 2024 09:46

Review feedback

457f24d

Merge remote-tracking branch 'upstream/main' into dashboardApi-getSer…

3bcba56

…ializedState

nickpeihl requested a review from nreese December 19, 2024 14:58

nreese approved these changes Dec 19, 2024

View reviewed changes

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

3dc3af4

…-fix'

elastic-vault-github-plugin-prod bot requested a review from a team as a code owner December 19, 2024 15:17

Fix types and change test to synchronous

535ffd2

Merge branch 'main' into dashboardApi-getSerializedState

16e29ae

ThomThomson removed the request for review from a team December 19, 2024 18:39

ThomThomson merged commit d7280a1 into elastic:main Dec 19, 2024

nickpeihl mentioned this pull request Dec 19, 2024

[8.x] [Dashboards] Add getSerializedState method to Dashboard API (#204140) #205010

Merged

nickpeihl added a commit to nickpeihl/kibana that referenced this pull request Dec 19, 2024

[Dashboards] Add getSerializedState method to Dashboard API (elastic#…

776d28f

…204140) Adds a `getSerializedState` method to the Dashboard API. (cherry picked from commit d7280a1)

stratoula pushed a commit to stratoula/kibana that referenced this pull request Jan 2, 2025

[Dashboards] Add getSerializedState method to Dashboard API (elastic#…

c4531c9

…204140) Adds a `getSerializedState` method to the Dashboard API.

benakansara pushed a commit to benakansara/kibana that referenced this pull request Jan 2, 2025

[Dashboards] Add getSerializedState method to Dashboard API (elastic#…

0586e97

…204140) Adds a `getSerializedState` method to the Dashboard API.

viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025

[Dashboards] Add getSerializedState method to Dashboard API (elastic#…

1c867f2

…204140) Adds a `getSerializedState` method to the Dashboard API.

Conversation

nickpeihl commented Dec 12, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Identify risks

Uh oh!

elasticmachine commented Dec 12, 2024

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!

nreese Dec 13, 2024 • 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!

nreese Dec 17, 2024 • 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!

nreese Dec 17, 2024 • 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!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomThomson commented Dec 19, 2024

Uh oh!

elasticmachine commented Dec 19, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

Uh oh!

kibanamachine commented Dec 19, 2024

Uh oh!

kibanamachine commented Dec 19, 2024

💔 All backports failed

Manual backport

Questions ?

Uh oh!

nickpeihl commented Dec 19, 2024

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nickpeihl commented Dec 12, 2024 •

edited by kibanamachine

Loading

nreese Dec 13, 2024 •

edited

Loading

nreese Dec 17, 2024 •

edited

Loading

nreese Dec 17, 2024 •

edited

Loading