[data.query] Add getState() api to retrieve whole QueryState#132035
[data.query] Add getState() api to retrieve whole QueryState#132035Dosant merged 6 commits intoelastic:mainfrom
getState() api to retrieve whole QueryState#132035Conversation
getState() api to retrieve whole QueryState
There was a problem hiding this comment.
can we name this timeRange
There was a problem hiding this comment.
This would be problematic because QueryState is existing interface we use with URL syncing and time is how this key is preserved in URLs.
I think to make it timeRange we will have to create a separate interface and keep both around. WDYT?
There was a problem hiding this comment.
i think it makes sense to decouple the two, we'll be modifying this in the short future for universal search purposes and it sounds we'll be limited there if we keep the coupling
There was a problem hiding this comment.
but we can wait with that until we need to do some changes that would be incompatible, for the first step i think this should be fine. what do you think @flash1293
There was a problem hiding this comment.
Hmm, not sure why we want to decouple them - it seems like using the same interface for state syncing and communication between unified search and apps would be fine. Is there something I'm missing?
There was a problem hiding this comment.
The current QueryState interface reflects a state in the URL (partially _g and _a portion of it), so must be very careful with what we change there.
We agreed with Peter, that would be safer to decouple and be explicit about what interface describes a state in the URL and what is for internal data transfer.
There was a problem hiding this comment.
Good point, didn't consider that. Also, no objections for this being called time for now.
There was a problem hiding this comment.
I went with a "partial" decoupling so in case the states state deviates, we will catch it on typescript level and can adjust then 30e9c66
I introduced GlobalQueryStateFromUrl state which for now matches QueryState. Everywhere where QueryState was used in the URL context I replaced it with GlobalQueryStateFromUrl
ppisljar
left a comment
There was a problem hiding this comment.
i think its fine if on QueryState all the keys are optional (for example i think there is high chance that is esql we won't be sending in any filters or when you have non-time based index there is no time range)
d991700 to
ea51e2c
Compare
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-app-services (Team:AppServicesSv) |
|
ML changes LGTM |
ThomThomson
left a comment
There was a problem hiding this comment.
Presentation team type changes LGTM!
nreese
left a comment
There was a problem hiding this comment.
kibana-gis changes LGTM
code review
kertal
left a comment
There was a problem hiding this comment.
Code LGTM, just a change of type in Discover locator
|
@elasticmachine merge upstream |
flash1293
left a comment
There was a problem hiding this comment.
Code LGTM, only type change in VisEditors code
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Page load bundle
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
Summary
Close #129487
This is an API enhancements that exposes a single API for retrieving whole
QueryStatewithout a need to usefilterManagertimeFilterqueryStringManagerseparately. We are adding this for USaFe initiative.Also adds a
GlobalQueryStateFromUrlinterface to decouple_gquery param from the URL state interface from the more genericQueryStatewhich might change soon. See the discussion #132035 (comment)