Skip to content

[Discover Sessions as Code] Fix false dashboard unsaved changes#261055

Merged
lukasolson merged 10 commits into
elastic:mainfrom
lukasolson:discover_sessions_as_code/unsaved_changes
Apr 8, 2026
Merged

[Discover Sessions as Code] Fix false dashboard unsaved changes#261055
lukasolson merged 10 commits into
elastic:mainfrom
lukasolson:discover_sessions_as_code/unsaved_changes

Conversation

@lukasolson
Copy link
Copy Markdown
Contributor

Summary

Closes #260804.
Related to #260945.

Problem

With discover.embeddableTransforms enabled, opening a dashboard that includes Discover session panels could mark the dashboard as dirty immediately, even though nothing was edited (issue #260804).

Solution

  • Pass defaultState into initializeUnsavedChanges so comparator inputs fall back to the same defaults when a key is absent on last saved or current serialized state:
    • Transforms / sessions shape: selected_tab_id → first tab id when present.
    • Legacy saved-search shape: selectedTabId plus other embeddable defaults (sort, grid, and values from getSearchEmbeddableDefaults(uiSettings)) so omitted persisted fields align with what we serialize at runtime.
  • Remove the bespoke compareSelectedTabId helper; referenceEquality on the tab id fields is sufficient once defaultState supplies the default tab id.
  • Refactor Discover-specific comparator maps into get_search_embeddable_comparators.ts (getSearchEmbeddableComparators / getDiscoverSessionEmbeddableComparators) and add unit tests

How to verify

  1. Enable discover.embeddableTransforms.
  2. Open a dashboard that contains Discover session (by-reference) panels that do not persist an explicit selected tab (or match the scenario from the issue).
  3. Confirm the dashboard does not show unsaved changes on load until something is actually edited.

@lukasolson lukasolson self-assigned this Apr 2, 2026
@lukasolson lukasolson added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// Feature:Embeddables Relating to the Embeddable system Project:Dashboards API labels Apr 2, 2026
@lukasolson lukasolson marked this pull request as ready for review April 3, 2026 20:12
@lukasolson lukasolson requested a review from a team as a code owner April 3, 2026 20:12
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

| Omit<SearchEmbeddableByReferenceState, keyof SearchEmbeddableBaseState>
);

export function getSearchEmbeddableComparators(
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.

q: just by looking at the structure it's pretty clear to me how this is going to be evaluated, but where/when is that evaluation done?

@AlexGPlay
Copy link
Copy Markdown
Contributor

i've done some tests and from the dashboards list it looks good, i was getting the "You have unsaved changes" banner, but when opening a dashboard with a discover session i'm still getting the blue dot that marks that there are changes, is it expected?

dashboards.mov

Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Tested the latest changes and it works great, thanks! Confirmed the dashboard I was testing with in #255213 (review) no longer triggers the unsaved changes indicator on load.

Comment on lines -182 to -189
// While the selected tab is missing or inline editing is in progress,
// skip tab-dependent comparators so unsaved-changes badges don't appear
// until the user explicitly applies a tab change.
...(shouldSkipTabComparators
? Object.fromEntries(
Object.keys(searchEmbeddable.comparators).map((k) => [k, 'skip'])
)
: {}),
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.

Do you know if removing this logic has any actual impact?

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/platform/test/serverless/functional/configs/security/config.group1.ts / Serverless Common UI - Management Data View Management creating and deleting default data view validation can resolve errors and submit

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
discover 158 160 +2

Async chunks

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

id before after diff
discover 1.6MB 1.6MB +78.0B
Unknown metric groups

API count

id before after diff
discover 215 217 +2

History

cc @lukasolson

@lukasolson lukasolson merged commit 03aa146 into elastic:main Apr 8, 2026
19 checks passed
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 Feature:Embeddables Relating to the Embeddable system Project:Dashboards API release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Discover Sessions as Code] Unsaved changes on dashboard load when discover.embeddableTransforms is enabled

5 participants