Skip to content

[Dashboard] State extraction as a consistent override#259839

Merged
ThomThomson merged 3 commits intoelastic:mainfrom
ThomThomson:dashboard/OverrideExtractionCleanup
Apr 1, 2026
Merged

[Dashboard] State extraction as a consistent override#259839
ThomThomson merged 3 commits intoelastic:mainfrom
ThomThomson:dashboard/OverrideExtractionCleanup

Conversation

@ThomThomson
Copy link
Copy Markdown
Contributor

Summary

Closes #257722

Root cause of the issue was that the extraction functions treated the panels and pinned_panels keys inconsistently, not providing panels as an override if a blank array was given.

Instead, this function should act as a pure override. I.e. if panels or pinned panels are given to this function it should return what it's given regardless of the count of elements in the array.

@ThomThomson ThomThomson added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. release_note:skip Skip the PR/issue when compiling release notes v9.4.0 backport:skip This PR does not require backporting labels Mar 26, 2026
@ThomThomson
Copy link
Copy Markdown
Contributor Author

/ci

@ThomThomson ThomThomson marked this pull request as ready for review March 27, 2026 14:29
@ThomThomson ThomThomson requested review from a team as code owners March 27, 2026 14:29
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elastic elastic deleted a comment from elasticmachine Mar 27, 2026
};

const { panels, savedObjectReferences } = extractPanelsState(stateAsObject);
if (panels?.length) dashboardState.panels = panels;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may cause problems if URL return empty array and then that empty array removes all panels as override state.

I think you should add a length check to loadDashboardApi so panels is only overridden if there is a length
https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/dashboard/public/dashboard_api/load_dashboard_api/load_dashboard_api.ts#L77

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is how it was before, but IMO an override should override. If you pass in empty panels to the URL, that should only happen because the user deleted all panels before sharing the Dashboard. There is a difference between
panels: undefined; // no panels in this URL, fall back to what you had and
panels: []; // explicitly setting panels to an empty array

@ThomThomson
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review only

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
code review only

@ThomThomson
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #29 / serverless observability UI - ML and Discover discover/observabilitySolution/context_awareness extension getRowIndicatorProvider should render log.level row indicators on Surrounding documents page

Metrics [docs]

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 29 30 +1

Async chunks

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

id before after diff
dashboard 847.6KB 847.6KB -19.0B
Unknown metric groups

API count

id before after diff
dashboard 125 126 +1

@ThomThomson ThomThomson merged commit 8741ad7 into elastic:main Apr 1, 2026
18 checks passed
mbondyra added a commit to mbondyra/kibana that referenced this pull request Apr 1, 2026
…heck

* commit '6f040b29a5220ce12886a9731f656613e50aff06': (34 commits)
  [Entity Analytics] Add entity resolution UI to service flyout (elastic#260504)
  [Dashboard] Fix setState in embeddables (elastic#260082)
  [EDR Workflows] Unskip FTR tests that failed due to transient Fleet service unavailability (elastic#260519)
  [Observability:Streams] Fix query streams error handling test (elastic#260777)
  [Alerting v2] Dispatcher grouping modes, throttle strategies, and matcher autosuggestion (elastic#260249)
  [Dashboard] State extraction as a consistent override (elastic#259839)
  [Alerting v2] [Rule authoring] Fix rule name validation and error visibility in create/edit flow (elastic#260337)
  [Fix] re-introduce sln breadcrumbs to unified rules (elastic#260289)
  [Security Solution][Endpoint] Updated kibana docs to include `xpack.securitySolution.maxEndpointScriptFileSize` as configurable in cloud (elastic#260568)
  [Alerting v2] updated the alerting-v2-constants package with artifacts constants, fix to the runbook max characters (elastic#260342)
  [Automatic Import V2] Provide user tooltips (elastic#260725)
  [One Workflow] Deduplicate step types by base type in workflow list (elastic#260763)
  [Security Solution] Execution results UI: Enable the feature flag (elastic#260711)
  [Metrics][Discover] internal/search/esql_async returns 200 but METRICS_INFO responds with error (elastic#260746)
  Collapse redundant anyOf/oneOf array unions in OAS query params (elastic#260585)
  [Unified rules] Hide stack rules from global search (elastic#260088)
  [Agent Builder] Sidebar navigation updates (elastic#260728)
  [* As Code] Use PUT for upserts (elastic#260318)
  Update EUI to v114.0.0 (elastic#259497)
  [Entity Resolution] Add contextual-security-apps as co-owner of resolution paths (elastic#260659)
  ...

# Conflicts:
#	src/platform/plugins/shared/dashboard/public/index.ts
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Apr 2, 2026
Makes the Dashboard URL state extraction function act as a pure override.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.4.0

Projects

None yet

4 participants