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

Allow multiple recurrence rules in an event #77

Merged
merged 1 commit into from
Sep 16, 2016
Merged

Allow multiple recurrence rules in an event #77

merged 1 commit into from
Sep 16, 2016

Conversation

SHyx0rmZ
Copy link
Contributor

No description provided.

@markuspoerschke markuspoerschke merged commit da7987e into markuspoerschke:master Sep 16, 2016
@markuspoerschke
Copy link
Owner

Thanks for your PR!

@markuspoerschke
Copy link
Owner

Released with 0.11.0.

@markuspoerschke
Copy link
Owner

I checked the changes in this PR again. According to the RFC 5545 the RRULE property should not be specified more than once:

This property [RRULE property] can be specified in recurring "VEVENT",
"VTODO", and "VJOURNAL" calendar components as well as in the
"STANDARD" and "DAYLIGHT" sub-components of the "VTIMEZONE"
calendar component, but it SHOULD NOT be specified more than once.
The recurrence set generated with multiple "RRULE" properties is
undefined.

Therefore it should only be possible to define it once within the implementation of this library.

Currently the implementation is broken because the PropertyBag was changed to accept only one property with the same name. Fatal error: Uncaught Exception: Property with name 'RRULE' already exists in /var/www/html/vendor/eluceo/ical/src/PropertyBag.php:65.

@SHyx0rmZ Do you have an use case for adding more than one recurrence rule? Because I tend to revert the changes from this PR again so that only one recurrence rule per component is allowed.

@SHyx0rmZ
Copy link
Contributor Author

@markuspoerschke I use this to do evil things like representing a recurrence every two days and five minutes. Reverting the changes seems like the sane thing to do, I must've overlooked that bit in the RFC when I read it. My use case should be covered by me calculating a bigger interval for the smallest frequency I can find, or so I thought until I saw that the interval is only allowed to be a single digit just now. I'll probably just scrap the calendar export functionality altogether, as I don't know anyone who uses it. Conclusion: Trying to cram your Cron job schedules into a calendar isn't a very good idea.

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