[Discover] Fix default app state handling when detecting unsaved changes#246664
Merged
davismcphee merged 6 commits intoelastic:mainfrom Dec 18, 2025
Merged
Conversation
davismcphee
added
release_note:fix
Team:DataDiscovery
davismcphee
force-pushed
the
fix-unsaved-changes-default-values
branch
from
5d69c06 to
6ea1677
Compare
davismcphee
commented
Dec 18, 2025
Comment on lines
+368
to
+377
| container.savedSearchState.set( | ||
| updateSavedSearch({ | ||
| savedSearch: finalSavedSearch, | ||
| dataView, | ||
| initialInternalState: undefined, | ||
| appState: currentTab.appState, | ||
| globalState: currentTab.globalState, | ||
| services, | ||
| }) | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
Calling updateSavedSearch now because some useUnsavedChanges tests were breaking after updating getInitialAppState, since savedSearchState was technically out of sync when using getDiscoverStateMock.
Wasn't necessary in the other PR because the tests were updated to use getDiscoverInternalStateMock there.
| const dataViewCreateSpy = jest.spyOn(services.dataViews, 'create'); | ||
| const dataViewsClearCacheSpy = jest.spyOn(services.dataViews, 'clearInstanceCache'); | ||
| const savedSearch = { ...savedSearchMock, timeRestore: false }; | ||
| savedSearch.searchSource.setField('filter', []); |
Contributor
Author
There was a problem hiding this comment.
This line was added because calling updateSavedSearch in getDiscoverStateMock causes filter to default to [], which breaks tests in this file that use savedSearch for comparisons.
Won't be an issue when these tests are migrated to use getDiscoverInternalStateMock which properly initializes state in the same way the app does.
Comment on lines
+218
to
+221
| // If changing tabs, stop syncing the current tab before updating any URL state | ||
| if (selectedTabHasChanged) { | ||
| currentTabStateContainer?.actions.stopSyncing(); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
If tabsStorageManager.pushSelectedTabIdToUrl() triggered a change to appState before we stopped syncing the previous tab, it could technically leak state.
This was not actually causing issues during runtime afaict, but was technically incorrect, and did cause a Jest test to fail when state was out of sync due to getDiscoverStateMock. Probably best to fix it regardless.
8 tasks
Contributor
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#10150[✅] src/platform/test/functional/apps/discover/tabs2/config.ts: 25/25 tests passed. |
Contributor
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
PhilippeOberti
approved these changes
Dec 18, 2025
Contributor
💔 Build Failed
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Async chunks
History
cc @davismcphee |
Contributor
|
Starting backport for target branches: 9.2, 9.3 |
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
davismcphee
added a commit
to davismcphee/kibana
that referenced
this pull request
Dec 18, 2025
…ges (elastic#246664) ## Summary This PR fixes an issue in Discover where default app state could trigger unsaved changes in saved Discover sessions, such as default columns applied via the `defaultColumns` advanced setting. It was mostly extracted from the changes in elastic#244686 (with a few additions I noticed while moving the code) so that it can be backported. Unfortunately it required more changes than expected in order for `getInitialAppState` to be useable within `selectHasUnsavedChanges`, since we don't have access to `SavedSearch` for persisted tabs when comparing. Fixes elastic#246798. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) 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](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] 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. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks The PR has more changes than I'd generally like for a fix with backports, but the actual changes to logic are fairly minimal. Most of the changes come from switching to `DiscoverSessionTab` in `getInitialAppState` and updating tests. (cherry picked from commit 34abb2d) # Conflicts: # src/platform/plugins/shared/discover/public/application/main/state_management/utils/get_state_defaults.ts
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
davismcphee
added a commit
to davismcphee/kibana
that referenced
this pull request
Dec 19, 2025
…ges (elastic#246664) ## Summary This PR fixes an issue in Discover where default app state could trigger unsaved changes in saved Discover sessions, such as default columns applied via the `defaultColumns` advanced setting. It was mostly extracted from the changes in elastic#244686 (with a few additions I noticed while moving the code) so that it can be backported. Unfortunately it required more changes than expected in order for `getInitialAppState` to be useable within `selectHasUnsavedChanges`, since we don't have access to `SavedSearch` for persisted tabs when comparing. Fixes elastic#246798. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) 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](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] 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. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks The PR has more changes than I'd generally like for a fix with backports, but the actual changes to logic are fairly minimal. Most of the changes come from switching to `DiscoverSessionTab` in `getInitialAppState` and updating tests. (cherry picked from commit 34abb2d) # Conflicts: # src/platform/plugins/shared/discover/public/__mocks__/discover_state.mock.ts # src/platform/plugins/shared/discover/public/application/main/state_management/discover_state.test.ts # src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/reset_discover_session.ts # src/platform/plugins/shared/discover/public/application/main/state_management/redux/utils.test.ts # src/platform/plugins/shared/discover/public/application/main/state_management/utils/get_initial_app_state.test.ts # src/platform/plugins/shared/discover/public/application/main/state_management/utils/get_initial_app_state.ts # src/platform/plugins/shared/discover/public/application/main/state_management/utils/get_state_defaults.ts # src/platform/plugins/shared/discover/public/customizations/customization_provider.ts
davismcphee
added a commit
that referenced
this pull request
Dec 19, 2025
…d changes (#246664) (#246982) # Backport This will backport the following commits from `main` to `9.3`: - [[Discover] Fix default app state handling when detecting unsaved changes (#246664)](#246664) <!--- Backport version: 10.2.0 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Davis McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2025-12-18T21:12:10Z","message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1","branchLabelMapping":{"^v9.4.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:DataDiscovery","backport:version","v9.3.0","v9.4.0","v9.2.3"],"title":"[Discover] Fix default app state handling when detecting unsaved changes","number":246664,"url":"https://github.com/elastic/kibana/pull/246664","mergeCommit":{"message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1"}},"sourceBranch":"main","suggestedTargetBranches":["9.3","9.2"],"targetPullRequestStates":[{"branch":"9.3","label":"v9.3.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.4.0","branchLabelMappingKey":"^v9.4.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/246664","number":246664,"mergeCommit":{"message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1"}},{"branch":"9.2","label":"v9.2.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
kibanamachine
added
the
backport missing
Contributor
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
1 similar comment
Contributor
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
davismcphee
added a commit
that referenced
this pull request
Dec 23, 2025
…d changes (#246664) (#246988) # Backport This will backport the following commits from `main` to `9.2`: - [[Discover] Fix default app state handling when detecting unsaved changes (#246664)](#246664) <!--- Backport version: 10.2.0 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Davis McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2025-12-18T21:12:10Z","message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1","branchLabelMapping":{"^v9.4.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:DataDiscovery","backport:version","v9.3.0","v9.4.0","v9.2.3"],"title":"[Discover] Fix default app state handling when detecting unsaved changes","number":246664,"url":"https://github.com/elastic/kibana/pull/246664","mergeCommit":{"message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1"}},"sourceBranch":"main","suggestedTargetBranches":["9.3","9.2"],"targetPullRequestStates":[{"branch":"9.3","label":"v9.3.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.4.0","branchLabelMappingKey":"^v9.4.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/246664","number":246664,"mergeCommit":{"message":"[Discover] Fix default app state handling when detecting unsaved changes (#246664)\n\n## Summary\n\nThis PR fixes an issue in Discover where default app state could trigger\nunsaved changes in saved Discover sessions, such as default columns\napplied via the `defaultColumns` advanced setting.\n\nIt was mostly extracted from the changes in #244686 (with a few\nadditions I noticed while moving the code) so that it can be backported.\nUnfortunately it required more changes than expected in order for\n`getInitialAppState` to be useable within `selectHasUnsavedChanges`,\nsince we don't have access to `SavedSearch` for persisted tabs when\ncomparing.\n\nFixes #246798.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nThe PR has more changes than I'd generally like for a fix with\nbackports, but the actual changes to logic are fairly minimal. Most of\nthe changes come from switching to `DiscoverSessionTab` in\n`getInitialAppState` and updating tests.","sha":"34abb2d23e012ddbefc4655414727bc1096729f1"}},{"branch":"9.2","label":"v9.2.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Matthias Polman <matthias.wilhelm@elastic.co>
kibanamachine
added
v9.2.4
and removed
backport missing
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Jan 6, 2026
…ges (elastic#246664) ## Summary This PR fixes an issue in Discover where default app state could trigger unsaved changes in saved Discover sessions, such as default columns applied via the `defaultColumns` advanced setting. It was mostly extracted from the changes in elastic#244686 (with a few additions I noticed while moving the code) so that it can be backported. Unfortunately it required more changes than expected in order for `getInitialAppState` to be useable within `selectHasUnsavedChanges`, since we don't have access to `SavedSearch` for persisted tabs when comparing. Fixes elastic#246798. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) 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](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] 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. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks The PR has more changes than I'd generally like for a fix with backports, but the actual changes to logic are fairly minimal. Most of the changes come from switching to `DiscoverSessionTab` in `getInitialAppState` and updating tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes an issue in Discover where default app state could trigger unsaved changes in saved Discover sessions, such as default columns applied via the
defaultColumnsadvanced setting.It was mostly extracted from the changes in #244686 (with a few additions I noticed while moving the code) so that it can be backported. Unfortunately it required more changes than expected in order for
getInitialAppStateto be useable withinselectHasUnsavedChanges, since we don't have access toSavedSearchfor persisted tabs when comparing.Fixes #246798.
Checklist
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.Identify risks
The PR has more changes than I'd generally like for a fix with backports, but the actual changes to logic are fairly minimal. Most of the changes come from switching to
DiscoverSessionTabingetInitialAppStateand updating tests.