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

Bugfix #91

Merged
merged 1 commit into from
Apr 17, 2015
Merged

Bugfix #91

merged 1 commit into from
Apr 17, 2015

Conversation

Andarist
Copy link
Contributor

Problem is that in options.wkst is kept a number and not a Weekday object. Therefore if wkst is set then we get wrong RFC string with 'WKST=0' when it should be 'WKST=MO' which makes parsedString = RRule.optionsToString(options) not parseable to RRule object again with RRule.fromString(parsedString) cause at line :1415 we have such assignment - options.wkst = RRule[value]; - value === 0 in that instead of 'MO' and problem is that RRule[0] is non existent. And that makes code at line :520 accessible and it raises an Exception (TypeError: Cannot read property 'weekday' of undefined) as nor Weekday nor number nor null is stored in opts.wkst.

Problem is that in options.wkst is kept a number and not a Weekday object. Therefore if wkst is set then we get wrong RFC string with 'WKST=0' when it should be 'WKST=MO' which makes parsedString = RRule.optionsToString(options) not parseable to RRule object again with RRule.fromString(parsedString) cause at line :1415 we have such assignment - options.wkst = RRule[value]; -  value === 0 in that instead of 'MO' and problem is that RRule[0] is non existent. And that makes code at line :520 accessible and it raises an Exception (TypeError: Cannot read property 'weekday' of undefined) as nor Weekday nor number nor null is stored in opts.wkst.
jkbrzt added a commit that referenced this pull request Apr 17, 2015
Fixed WKST conversion to string
@jkbrzt jkbrzt merged commit 7351021 into jkbrzt:master Apr 17, 2015
@jkbrzt
Copy link
Owner

jkbrzt commented Apr 17, 2015

Thanks!

jkbrzt added a commit that referenced this pull request Apr 29, 2015
@jkbrzt
Copy link
Owner

jkbrzt commented Apr 29, 2015

I've had to undo this change as it was throwing exceptions.

Could you please provide an example where wkst is a number instead of a Weekday?

@Andarist
Copy link
Contributor Author

var rrule = RRule.fromString('FREQ=WEEKLY;DTSTART=20110201T093000Z;INTERVAL=5;UNTIL=20130130T230000Z;BYDAY=MO,FR');

RRule.optionsToString(rrule.options); // and you get in the string WKST=0, and it ain't valid iCal option, you cannot parse it back to RRule object

@jkbrzt
Copy link
Owner

jkbrzt commented Apr 30, 2015

Please read the documentation on rrule.options and rrule.origOptions here:

var rrule = RRule.fromString('FREQ=WEEKLY;DTSTART=20110201T093000Z;INTERVAL=5;UNTIL=20130130T230000Z;BYDAY=MO,FR');

rrule.toString()
"FREQ=WEEKLY;DTSTART=20110201T093000Z;INTERVAL=5;UNTIL=20130130T230000Z;BYDAY=MO,FR"

rrule.origOptions.byweekday
[Weekday, Weekday]

rrule.origOptions.byweekday.toString()
"MO,FR"

rrule.origOptions.byweekday[0].toString()
"MO"

@jkbrzt
Copy link
Owner

jkbrzt commented Apr 30, 2015

The (documented) inconsistency regarding rrule.options.byweekday is an issue and is causing the problem you mention for example when you access it directly or do something like RRule.optionsToString(rrule.options). But it should be addressed on another level than your proposed pull request.

aramk added a commit to aramk/rrule that referenced this pull request May 3, 2015
# By Linquize (2) and others
# Via Jakub Roztočil (4) and Jakub Roztocil (2)
* 'master' of https://github.com/jakubroztocil/rrule:
  Undo jkbrzt#91 as it was breaking the demo app
  Allow Negative and 2-Digit Values in Demo
  Bugfix
  Fix typo
  Do not hang if interval is 0 or not a number
  Updating script location in Bower, fixes jkbrzt#61.
aramk added a commit to aramk/rrule that referenced this pull request May 3, 2015
* master:
  Renamed isValid() to isValidDate().
  Undo jkbrzt#91 as it was breaking the demo app
  Allow Negative and 2-Digit Values in Demo
  Bugfix
  Updating script location in Bower, fixes jkbrzt#61.

Conflicts:
	bower.json
	lib/rrule.js
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

Successfully merging this pull request may close these issues.

2 participants