Rework RecurrenceId to use date/datetime and range
This is my suggestion for improving recurrence-id serializing/parsing #278. It now works with dates and supports RANGE as well (although it doesn't enumerate correctly yet in timespan).
A number of tests still need to be updated, some are just the snapshots but others should probably get this functionality confirmed before we change them.
Thank you for this library!
https://datatracker.ietf.org/doc/html/rfc5545#autoid-93
In short, the RFC states that Recurrence ID has a value of type DATE or DATE-TIME (same value type as the event's dtstart/dtend). This DATE/DATE-TIME must match an instance of the event within the recurrence, and it will override that one. Recurrence ID also has a few optional parameters: VALUE which can explicitly state whether it is DATE or DATE-TIME, the TZID timezone (similar to other DATE-TIME properties), and RANGE, which only has one valid value, THISANDFUTURE, and signals this override should also apply to all additional following instances of the event.
Here's an example giving all optional parameters
RECURRENCE-ID;TZID=America/New_York;VALUE=DATE-TIME;RANGE=THISANDFUTURE:20241217T140000
Prior to this PR, RecurrenceId could not set the parameters, only the value, and the input type was a string instead of date/datetime. While one could potentially format at least the date value correctly on their own, there was no way to set the timezone for that date or enable THISANDFUTURE range. Anecdotally, this caused a lot of confusion for me using the library about what sort of value I needed to provide, and was an issue because my use case depends on Recurrence ID heavily.
This PR reworks RecurrenceId into a Pydantic model with two fields: date which accepts either a date or datetime object, and this_and_future, which is a boolean and adds RANGE=THISANDFUTURE to the params. The model now both parses and encodes these as well.
This was proposed to get an idea of whether this was the right way to implement it, but it's incomplete and requires some additional changes:
- Support elsewhere in the library for accessing the date on
RecurrenceIdcorrectly, particularly ontimespan - Support for
THISANDFUTURErange - Update tests
- Update docs
- Improved errors/constraints
- Validate that
RecurrenceId's type matchesdtstarton the event - Validate that
RecurrenceId's date matches a time given by the recurrence rule. - Ensure that both
rruleandrecurrence-idare not both populated on the same event. (Saw an offhand comment about this but need stronger confirmation from the spec this is necessary)
- Validate that
Thank you for making this contribution. Can you update the PR description with a description of the rules, whats changing, and how it fixes any issues with implementing the RFC? (any other caveats, issues not fixed, etc) Thank you.
Sure, should be updated now.
Thanks for updating the description. I think there is more needed to make this work. Namely, the tests are not passing, so its breaking the API in some ways.
This needs to be compatible with existing APIs or it will be a breaking change. If we want to do a breaking change then we need to actually fix the full issues with how recurring event edits are handled. I think it needs to be compatible for now.
Yeah, I just did this as a proof of concept to get feedback so I haven't updated the tests or other parts of the library, they are broken right now. I'm not sure if it's possible to correct this without a breaking change though, even just changing the value's type from a string into a date/datetime would break anything currently passing in a string.
If you're fine with this direction and with making a breaking change release, I can finish this and implement the correct behavior for recurring events. If you'd like to prioritize compatibility instead, what would like me to do to make that happen?
The major problem that needs to be fixed is the store interface for making edits to recurring events is not exhasutive and has some gaps/bugs or incompatibilities with other calendar systems because of the way the recurring events are identified. If you are going to fix that problem then a breaking chage is ok. I don't want a breaking change just to change recurrence id without actually fixing anything.