-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Discover] View mode changes should trigger Unsaved changes badge in ES|QL #187244
Conversation
/ci |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
To update your PR or re-run it, just comment with: cc @jughosta |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
if (fieldName === 'viewMode') { | ||
// By default, viewMode: undefined is equivalent to documents view | ||
// So they should be treated as same | ||
return savedSearch.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and this PR fixes the issue 👍. one ask, this is the actual fix of the problem, right? Could you add a unit test for it in discover_saved_search_container.test.ts
? it would be great to increase coverage in this case 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kertal The actual fix is in src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts
. Here it's only a stylistic choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it, thx a lot for adding more tests anyway 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thx a lot for fixing this, and adding more tests on top, even if they were not necessary for catching the bug 👍
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
cc @jughosta |
…ES|QL (elastic#187244) - Closes elastic#184624 ## Summary This PR updates the logic to show Unsaved changes badge when view mode changes in ES|QL mode. ### Checklist - [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 --------- Co-authored-by: Matthias Wilhelm <[email protected]> (cherry picked from commit 25d7346)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…dge in ES|QL (#187244) (#187687) # Backport This will backport the following commits from `main` to `8.15`: - [[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)](#187244) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Julia Rechkunova","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-05T14:46:00Z","message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","backport:prev-minor","v8.16.0"],"title":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL","number":187244,"url":"https://github.com/elastic/kibana/pull/187244","mergeCommit":{"message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187244","number":187244,"mergeCommit":{"message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8"}}]}] BACKPORT--> Co-authored-by: Julia Rechkunova <[email protected]>
Summary
This PR updates the logic to show Unsaved changes badge when view mode changes in ES|QL mode.
Checklist