Skip to content

RRule object is not initialized with dtstart #21

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

Closed
LionelMartin opened this issue Jul 28, 2013 · 7 comments
Closed

RRule object is not initialized with dtstart #21

LionelMartin opened this issue Jul 28, 2013 · 7 comments

Comments

@LionelMartin
Copy link

When accessing the rrule of an event, the dtstart of the rrule is not set, so I had to do this:

  ev.rrule.origOptions.dtstart = ev.start;
  ev.rrule = ev.rrule.clone();

Only then could I safely use the rrule

recurDate = ev.rrule.between(currentDate, dateTo);
@LionelMartin
Copy link
Author

@bastinat0r
Copy link
Contributor

Same problem here.
(LionelMartin: thanks for the hack)

@peterbraden
Copy link
Owner

I believe this was fixed by #48 - can you run your test and confirm?

I'd love to add the test to the suite if this solves it.

Angus71 pushed a commit to Angus71/ical.js that referenced this issue Feb 23, 2015
Works for me, but personally I don't like "hack" used in ical addTZ() fill fill optinal "RRule" with DTSTART value
@Angus71
Copy link

Angus71 commented Feb 23, 2015

I also stumbled across this one and made some changes, which work for me. From my point of view this is still a hack, as I don't like the changes I've made in ical.js/addTZ(..). Maybe somebody has a better idea, how to implement it.
The main problem I found, was that you can't depend on the position of the "DTSTART" if this comes before the "RRULE" then the #48 might work, but I had problems with it as the fix will not update the already created a "BYMOUNTH" and "BYDATE" in the rrule cache.
On the other hand if "DTSTART" follows RRULE #48 might never work, so the same fix done in "RRULE" handling needs also be done when DTSTART is parsed.
As said previously I consider this quite a hack, but as this problem is around nearly a year I though sharing might be helpful. If required/wanted I can created a pull request, but the code might need some additional testing.

@risacher
Copy link

risacher commented Apr 3, 2015

I ran across this problem also. The fix for #48 does not completely fix #21 - in that ev.rrule.between() does not seem to work properly... even though rrule.options.dtstart seems to be set, it's not being honored. (Perhaps ev.rrule.origOptions is being used instead?)

I independently developed my own fix, which is shown below. This code fixes the problems that I was experiencing, but potentially suffers the same deficiency that @Angus71 noted, namely that if "DTSTART" comes after the "RRULE", then it should not work. (and if RRULE includes a DTSTART field it will be overwritten, although this is easily fixed.)

ical.objectHandlers['RRULE'] = function(val, params, curr, stack, line){        
  var rule = line.replace("RRULE:", "");                                        
  rule += ";DTSTART="+curr.start.toISOString().replace(/[-:]/g,'');             
  rule = rule.replace(/\.[0-9]{3}/, "");                                        
  curr['rrule'] = rrule.fromString(rule);                                       
  return curr                                                                   
}             

@risacher
Copy link

risacher commented Apr 3, 2015

The DTSTART/RRULE sequencing issue noted by @Angus71 (and later me) should probably be fixed by moving the code that patches up the RRULE with a default start date into the objectHandler for 'END'. I'll try this tonight.

@ericlathrop
Copy link
Contributor

@risacher Any luck fixing this?

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

6 participants