-
Notifications
You must be signed in to change notification settings - Fork 120
[Analytics Hub] Handle range selection granularity and interval #8341
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
…t the SelectionType interval
You can test the changes from this Pull Request by:
|
ealeksandrov
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.
LGTM with one small improvement suggestion! ![]()
| case .monthToDate, .lastMonth: | ||
| return 31 | ||
| case .quarterToDate, .lastQuarter: | ||
| return 12 |
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.
For amount of weeks in a quarter should we pick 13 to be more precise?
Year is 52 weeks (+1-2 days), so 52+/4 = 13+
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.
Makes perfect sense! Thanks! Fixed here: 1ca89c2
| } | ||
| } | ||
|
|
||
| /// The resopnse interval size that should be used to request stats from the given SelectedType |
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.
nit:
| /// The resopnse interval size that should be used to request stats from the given SelectedType | |
| /// The response interval size that should be used to request stats from the given SelectedType |
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.
Thanks! Fixed here: 1ca89c2
Closes #8189
Why
The stats request are currently using a fixed granularity of days with an interval size of
31. For better request performance and a correct chart render, we need to use the correct granularity and interval based on the selected time range.How
Adds to the
SelectedTypetwo computed properties:granularityandintervalSize, grouping the response based on the time range nature likedays,weeks,months,quartersandyears.How to Test
Update release notes:
RELEASE-NOTES.txtif necessary.