Rule Set: All() returns events in local time even if DTStart is provided as an UTC time object

kristinn commented 5 years ago

Hello everybody.

I want to report a suspected bug. I'm able to provide a pull request with a fix if this is indeed a bug.

Bug description

The DTStart's timezone is not respected when requesting a list of upcoming events.

Code for reproducing the bad behavior

package main

import (
    "fmt"
    "github.com/teambition/rrule-go"
    "time"
)

func main() {
    ruleSet := &rrule.Set{}
    rule, err := rrule.StrToRRule("FREQ=DAILY;COUNT=10;WKST=MO;BYHOUR=10;BYMINUTE=0;BYSECOND=0")
    if err != nil {
        panic(err)
    }
    ruleSet.RRule(rule)
    ruleSet.DTStart(time.Date(2019, 3, 6, 0, 0, 0, 0, time.UTC))

    fmt.Printf("DTStart: %v\n", ruleSet.GetDTStart())
    fmt.Printf("Recurrence: %v\n", ruleSet.Recurrence())

    // ruleSet.All() returns events in my local timezone instead of UTC
    for _, event := range ruleSet.All() {
        fmt.Printf("Event: %v\n", event)
    }
}

Expected results

DTStart: 2019-03-06 00:00:00 +0000 UTC
Recurrence: [DTSTART:TZID=UTC:20190306T000000 RRULE:FREQ=DAILY;COUNT=10;BYHOUR=10;BYMINUTE=0;BYSECOND=0]
Event: 2019-03-06 10:00:00 +0000 UTC
Event: 2019-03-07 10:00:00 +0000 UTC
Event: 2019-03-08 10:00:00 +0000 UTC
Event: 2019-03-09 10:00:00 +0000 UTC
Event: 2019-03-10 10:00:00 +0000 UTC
Event: 2019-03-11 10:00:00 +0000 UTC
Event: 2019-03-12 10:00:00 +0000 UTC
Event: 2019-03-13 10:00:00 +0000 UTC
Event: 2019-03-14 10:00:00 +0000 UTC
Event: 2019-03-15 10:00:00 +0000 UTC

Actual results

DTStart: 2019-03-06 00:00:00 +0000 UTC
Recurrence: [DTSTART:TZID=UTC:20190306T000000 RRULE:FREQ=DAILY;COUNT=10;BYHOUR=10;BYMINUTE=0;BYSECOND=0]
Event: 2019-03-06 10:00:00 +0100 CET
Event: 2019-03-07 10:00:00 +0100 CET
Event: 2019-03-08 10:00:00 +0100 CET
Event: 2019-03-09 10:00:00 +0100 CET
Event: 2019-03-10 10:00:00 +0100 CET
Event: 2019-03-11 10:00:00 +0100 CET
Event: 2019-03-12 10:00:00 +0100 CET
Event: 2019-03-13 10:00:00 +0100 CET
Event: 2019-03-14 10:00:00 +0100 CET
Event: 2019-03-15 10:00:00 +0100 CET

rickywiens commented 5 years ago

Hi, I spent some time today figuring this out. I have a fix working locally but it's not super great.

I think this has to do with func NewRRule and the way it pre-calculates the timeset on creation, and then when DtStart is set, it does not effect those pre-calculated values.

I have a unit test locally to reproduce the issue:

func TestSetTimezonesDtStartSticky(t *testing.T) {

    set := &Set{}
    rule, err := StrToRRule("FREQ=DAILY;COUNT=10;WKST=MO;BYHOUR=10;BYMINUTE=0;BYSECOND=0")

    if err != nil {
        panic(err)
    }

    set.RRule(rule)
    dt := time.Date(2019, 3, 6, 0, 0, 0, 0, time.UTC)
    set.DTStart(dt)

    // set.All() returns events in my local timezone instead of UTC
    for _, event := range set.All() {
        if event.Location().String() != time.UTC.String() {
            t.Errorf("Timezone set incorrectly on recurrence after DtStart set: [%s]", event.String())
        }
    }
}

But the way I fixed it was to recreate all the rrules and exrules when the DtStart is set:

// DTStart sets dtstart property for all rules in set
func (set *Set) DTStart(dtstart time.Time) {

    var newrruleset []*RRule
    var newexruleset []*RRule

    set.dtstart = dtstart

    for _, r := range set.rrule {
        r.dtstart = dtstart
        r.OrigOptions.Dtstart = dtstart
        nr, err := NewRRule(r.OrigOptions)
        if err != nil {
            return
        }
        newrruleset = append(newrruleset, nr)
    }

    for _, xr := range set.exrule {
        xr.dtstart = dtstart
        xr.OrigOptions.Dtstart = dtstart
        nxr, err := NewRRule(xr.OrigOptions)
        if err != nil {
            return
        }
        newexruleset = append(newexruleset, nxr)
    }

    set.rrule = newrruleset
    set.exrule = newexruleset
}

I don't particularly like that fix, so if you have some ideas I'd love to hear them :).

kristinn commented 5 years ago

@rickywiens thank you for your quick response!

I will see what I can cook up and create a pull request if I think it's good enough. 😄

kristinn commented 5 years ago

@rickywiens @damoye I've created a pull request with a fix.

Thank you.

teambition / rrule-go