-
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
[Lens] fix error for minInterval>computedInterval for XYChart #61931
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
466bab3
to
766d07f
Compare
@mbondyra I'm trying to test this PR with an example that is definitely broken: what steps have you been using that are known to be broken? |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
@wylieconlon if you create a date histogram, including the date range consisting Daylight Saving Time, it will break for sure (eg.from |
Hint for that: It must include a DST switch in the timezone you're using :-) So I think the above given time range would include one for European time zones, but not for most US time zones. |
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 on Chrome Linux. Tested one layer with DST switch in it, and tested multiple layers where one layer has only one bar and the other multiple.
Both failed before that, and work with the fix. Code LGTM
Also a hint: you can test the second scenario, by editing the DSL of a filter directly to match something like:
{
"query": {
"bool": {
"should": [
{
"match_phrase": {
"FlightNum": "00PKULZ"
}
},
{
"match": {
"_index": "kibana_sample_data_logs"
}
}
]
}
}
}
Where one part of the filter will only match ONE document in that one index and the other just on the index name of the 2nd layer.
* master: [HomeApp] Set breadcrumbs when coming back from add data dir (elastic#62186) [Lens] fix error for minInterval>computedInterval for XYChart (elastic#61931) ci: remove AppArch label from ProBot path-labeler (elastic#62211) [Uptime] Optimize get latest monitor API (elastic#61820) [Maps] Separate layer wizards for Clusters and heatmap (elastic#60870) Remove polling delay (elastic#62099) accessibility tests for dashboard panel ( OSS) (elastic#62055) rename README.md to readme, avoiding issues with case change [SIEM] [Detection Engine] Fixes all rules sorting (elastic#62039) [SIEM] CASES Bugs BC2 (elastic#62170) Revert "Endpoint: Add ts-node dev dependency (elastic#61884)" (elastic#62197) Closes elastic#60173 by turning off client caching for the main service map API call (elastic#62111) [SIEM] Restores the _External alert count_ widget's subtitle (elastic#62094) [Maps] Update ems client dependency to 7.8.0 (elastic#62181) [Metrics Alerts] Fix action variables, default message, and EU… (elastic#62061) Update CODEOWNERS with ES-UI apps, including grok debugger. (elastic#62045) Update ILM node attributes blacklist. (elastic#62093)
Summary
Fixes throwing an error for minInterval>computedInterval for XYChart - when user chooses timerange with Daylight Saving Time in between, the minInterval causes throwing an error in elastic-charts library. Fix: We don't need to pass minInterval to any other case except for single bar chart (that was the reason the bug was introduced here: (#61452) so I've just limited the case. Confirmed with @markov00 .
In addition, when testing, I checked the following scenarios for multiple layers:
Checklist
Delete any items that are not applicable to this PR.
For maintainers