Skip to content
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

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

Closed
kristinn opened this issue Mar 6, 2019 · 3 comments
Assignees

Comments

@kristinn
Copy link
Contributor

kristinn commented Mar 6, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

kristinn commented Mar 7, 2019

@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
Copy link
Contributor Author

kristinn commented Mar 7, 2019

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

Thank you.

@zensh zensh closed this as completed in #26 Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants