-
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] Fix legacy sort parameter provided by URL #134447
Conversation
…cloud_security_posture plugin (elastic#130630)
…-ref HEAD~1..HEAD --fix'
@elasticmachine merge upstream |
src/plugins/discover/public/application/main/services/discover_state.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
merge conflict between base and head |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @kertal |
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 👍
Summary
We switched to multisort a long time ago in #41918. Before this change users could just provide one sort parameter. This PR handles the case correctly when there's a
legacy
sort parameter incoming by the URL. It was broken for a long time, but was not noticed, since it was not possible tounsort
the classic table. With the change to classic table, q legacy sort parameter caused the table to be unsorted. This PR takes care of that, by migrating the legacy sort to multisort if needed.Here's a testable link (containing
sort:!(timestamp,desc))
as sort param, which is legacy.🥣 Deployment with fix
To compare it you can test with an installation where it doesn't work
🥣 Deployment without fix
it's not sorted:
Fixes #132892
Checklist