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

First iteration not generated if local time is before iteration time #369

Open
RonyDimebag opened this issue Oct 1, 2019 · 9 comments
Open

Comments

@RonyDimebag
Copy link

RonyDimebag commented Oct 1, 2019

Hi,
I am generating instances from a string RRule and noticed that the first iteration will not be produced if local time (HH:mm) is earlier than the event's time, regardless of the date and that the instance is in the future.
I am using UTC and all the dates are produced from unix timestamp values.

var dateStart = new Date(1580810460 * 1000); // 1580810460 = Tuesday, 04-Feb-20 10:01:00 UTC
var end = new Date(1583618400 * 1000); // 1583618400 = Saturday, 07-Mar-20 22:00:00 UTC
var nrule = rrule.rrulestr('FREQ=WEEKLY;BYDAY=TU;UNTIL=20200301T000000Z');
nrule.options.dtstart = dateStart;
var set = nrule.between(new Date(dateStart * 1000), new Date(end * 1000), true);

In the example above, if current UTC time is before 10:01, the first supposed instance of this rule that should occur on 04-Feb-20 will not be generated (although it's in the future), while if current UTC time is after 10:01 it will be generated successfully.

I tried this on both 2.2.0 and latest 2.6.0, both yielded same result.
I'm running on latest chrome / windows 10, my local time is GMT+02:00.

Thanks a lot!

@pitops
Copy link

pitops commented Dec 12, 2019

@RonyDimebag i am running into a similar issue. Do you have any solution to this?

@RonyDimebag
Copy link
Author

RonyDimebag commented Dec 12, 2019

@pitops unfortunately not.

The only idea I had for a solution was to uniquely check for such an exception and deal with it privately (manually add an instance), but it's too messy and I don't know what other exception I will run into.

Sadly such bug makes this great library practically unusable.

@pitops
Copy link

pitops commented Dec 12, 2019

@RonyDimebag
Thats unfortunate indeed. Yeah, I might have to look into alternatives or monkey patch this great library i guess..

@RonyDimebag
Copy link
Author

@pitops if you succeed please do let me know :)

@pitops
Copy link

pitops commented Dec 12, 2019

@RonyDimebag sure!

@pitops
Copy link

pitops commented Dec 14, 2019

@RonyDimebag I think i found the issue. According to this https://stackoverflow.com/questions/54517101/rrule-not-setting-correct-time-if-dtstart-is-set

it seems that doing nrule.options.dtstart = dateStart messes everything up (i was doing the same).

Instead create a new RRule and add the dtstart there and it should work fine :) - works in my case at least!

@RonyDimebag
Copy link
Author

Hey @pitops Wow that's great! You mean put the dstart as part of the actual rrule string?
I'm not working on this right now but i will sure check it in a couple of weeks.

Thanks

@pitops
Copy link

pitops commented Dec 16, 2019

Hey @RonyDimebag, so basically when you make a new rrule using the new RRule({}) syntax, add the dtstart there.

example

new RRule({
	...other options
	dtstart: <date>
})

If you start with a rrule string then you need to do

	const rrule = RRule.fromText(<string>)
	const rruleWithDTSTART = new RRule({
		...rrule.options
		dtstart: <date>
	})

Note that spreading all rrule.options seems to behave strange for me

hope that helps - good luck

@davidgoli
Copy link
Collaborator

davidgoli commented Dec 16, 2019

Hi, this should be better documented, but you should consider the options object to be immutable. Don't expect that changes you make to it after constructing the RRule will have any effect. @pitops solution is the recommended approach.

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