-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(explore): set default chart interval to highest granularity #104283
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
Conversation
useChartInterval() will now default to the highest granularity available using ChartIntervalUnspecifiedStrategy.USE_SMALLEST.
4bd87c6 to
79b4c91
Compare
| end: '2024-01-01T01:00:00.000Z', | ||
| range: '1h', | ||
| environment: ['production'], | ||
| interval: '5m', |
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.
Given that logs were already setting ChartIntervalUnspecifiedStrategy.USE_SMALLEST I'm confused as to why I needed to update this test.
Would like to look a little closer before landing.
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.
useSaveAsItems, when invoking useSaveQuery uses the interval returned by userChartInterval() rather than the interval passed in as options: https://github.com/getsentry/sentry/blob/master/static/app/views/explore/hooks/useSaveQuery.tsx#L59
I believe this to be intentional so that when you save you're saving whatever is currently set in the chart on the page.
Testing locally saving queries seemed to work as expected.
gggritso
left a comment
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.
Very nice, thanks!
useChartInterval() will now default to the highest granularity available using ChartIntervalUnspecifiedStrategy.USE_SMALLEST.