teambition / rrule-go

Go library for working with recurrence rules for calendar dates.
MIT License
314 stars 58 forks source link

fix: daylight savings #66

Open Xyedo opened 5 months ago

Xyedo commented 5 months ago

fixing this issues: https://github.com/teambition/rrule-go/issues/63 and appending the testcase on https://github.com/teambition/rrule-go/pull/64:

By letting golang time package calculate the daylight savings by changing it to time.Duration and add it to the existing Date

leaanthony-sc commented 5 months ago

I've discovered an edge case where this does not work: If you have a DAILY frequency and it crosses a timeline, then it will fail.

Test added to rrule_test.go:

func TestDailyDST(t *testing.T) {
    sydney, _ := time.LoadLocation("Australia/Sydney")
    r, _ := NewRRule(ROption{
        Freq:    DAILY,
        Count:   3,
        Dtstart: time.Date(2023, 4, 1, 9, 0, 0, 0, sydney),
    })
    want := []time.Time{
        time.Date(2023, 4, 1, 9, 0, 0, 0, sydney),
        time.Date(2023, 4, 2, 9, 0, 0, 0, sydney),
        time.Date(2023, 4, 3, 9, 0, 0, 0, sydney),
    }
    value := r.All()
    if !timesEqual(value, want) {
        t.Errorf("get %v, want %v", value, want)
    }
}

Running it gives us the following error:

=== RUN   TestDailyDST
    rrule_test.go:1751: get [2023-04-01 09:00:00 +1100 AEDT 2023-04-02 08:00:00 +1000 AEST 2023-04-03 09:00:00 +1000 AEST], want [2023-04-01 09:00:00 +1100 AEDT 2023-04-02 09:00:00 +1000 AEST 2023-04-03 09:00:00 +1000 AEST]
--- FAIL: TestDailyDST (0.00s)

I believe for DAILY we need to recreate the date but only add the number of days.

Xyedo commented 5 months ago

I think for FREQ hourly, minutely, and secondly we need to add with time duration, so the duration is fixed, but with FREQ Daily, Weekly, and Monthly, we follow the time provided by the DTSTART and then add the Date.

leaanthony-sc commented 5 months ago

This change appears to work. Awesome job @Xyedo! 🙏