refactor: define insights interval#9693
Conversation
mafredri
left a comment
There was a problem hiding this comment.
Nice work! I'm happy this was ~ as minimal a change as I had imagined.
|
|
||
| interval := time.Duration(arg.IntervalDays) * 24 * time.Hour | ||
|
|
||
| statsByInterval := []statByInterval{{arg.StartTime, arg.StartTime.Add(interval), make(map[uuid.UUID]struct{}), make(map[uuid.UUID]struct{})}} |
There was a problem hiding this comment.
I think this could still use .AddDate, the reason I point it out is caution with DST. E.g. does it behave the same as an interval when crossing the DST boundary?
I think Go assumes this for you, but I think it would be good to confirm 👍🏻
So the question is: Does the clock stay 00:00 when using Add(interval) and DST is part of time range? (A good test would be both 24time.Hour and 168time.Hour.)
There was a problem hiding this comment.
It is more like a Go question, and .Add(duration) doesn't preserve the time change (DST), so good catch! I will switch to .AddDate() to have consistent 00:00 hour windows.
func TestAddDateWithDST(t *testing.T) {
// Set the time zone to a location that observes DST
tz, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatal(err)
}
// Create a time in the specified time zone
date := time.Date(2023, time.March, 11, 0, 0, 0, 0, tz)
// Add a duration of one day to the date
// newDate := date.Add(48 * time.Hour)
newDate := date.AddDate(0, 0, 2)
// Since DST starts on March 12, 2023, in America/New_York, adding one day should account for DST
// and the resulting date should be 23 hours ahead
expectedDate := time.Date(2023, time.March, 13, 0, 0, 0, 0, tz)
// Compare the expected date with the actual result
if !newDate.Equal(expectedDate) {
t.Errorf("Expected: %s, Got: %s", expectedDate, newDate)
}
}
Related: #9495
This PR exposes
interval_daysfor "coderd" to customize the time range in template insights queries. In the follow up, I will continue with #9684.Note: As I renamed
GetTemplateDailyInsightstoGetTemplateInsightsByInterval, the formatter adjusted the order of functions accordingly, so this PR may seem to be relatively large.