Rework RecurrenceId to use date/datetime and range

[pmdevita]

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 RecurrenceId correctly, particularly on timespan
• Support for THISANDFUTURE range
• Update tests
• Update docs
• Improved errors/constraints
  • Validate that RecurrenceId's type matches dtstart on the event
  • Validate that RecurrenceId's date matches a time given by the recurrence rule.
  • Ensure that both rrule and recurrence-id are not both populated on the same event. (Saw an offhand comment about this but need stronger confirmation from the spec this is necessary)

[allenporter]

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.

[pmdevita]

Sure, should be updated now.

[allenporter]

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.

[pmdevita]

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?

[allenporter]

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.

[pmdevita]

This has now been updated to work with existing tests. It looks like stutrek did a lot of work earlier to do store editing so I'm piggybacking off of that, though I think I need to add some more tests now.

One question, my original draft had the boolean for RANGE/this_and_future and I realized later I completely missed the enum, I assume the answer would be yes but do you want me to swap to only using the enum?

[allenporter]

Thanks for working on this! I did a detailed review of the proposed changes in this PR and wanted to share some key findings and concerns regarding matching, backwards compatibility, and the RFC 5545 spec:

1. Wall-Clock Time Mutation (Timezone Matching Bug)

In the matching and conversion logic (e.g., in from_value and _match_item), timezone normalization uses .astimezone():

if timezone is not None:
    if isinstance(date, datetime.datetime):
        date = date.astimezone(timezone)

If the input is a naive datetime representing the local start time of a recurrence instance (e.g., '20250511T090000'), calling .astimezone(timezone) treats it as being in the system local timezone and converts it. If the system is in UTC, it converts 09:00:00 UTC to 05:00:00-04:00 New York, shifting the wall-clock time and causing matching to fail. We should use .replace(tzinfo=...) instead to keep the wall time.

2. Breaking API Changes

Changing RecurrenceId to a Pydantic BaseModel breaks backward compatibility:

• Existing code expects event.recurrence_id to be a str.
• When serializing the calendar to JSON/dict via model_dump(), recurrence_id now serializes as a dictionary (e.g., {'date': ..., 'this_and_future': ...}) rather than a string, which will break database schemas and integrations.

3. Inline Parameter Parsing

The PR's string_to_date() method still fails on strings like 'TZID=America/New_York:20250511T020000' because it passes the raw string to the standard encoders, which don't strip parameter prefixes.

4. RFC 5545 Conformance

To fully satisfy Section 3.8.4.4 of RFC 5545:

• RECURRENCE-ID must have the same value type (DATE vs DATE-TIME) as the event's DTSTART.
• RECURRENCE-ID must be specified as floating local time if and only if DTSTART is specified as floating local time.
  The PR doesn't yet enforce these validations.

[allenporter]

Thank you again for this contribution — I want to give proper credit here because this PR did a lot of the hard thinking that shaped where we ended up.

Your analysis of RFC 5545 §3.8.4.4 was spot-on, and the core design decisions in this PR — surfacing TZID and RANGE as first-class fields on RecurrenceId, building a from_value() factory that accepts multiple input types, and removing the separate recurrence_range parameter from the store API in favour of embedding it on the recurrence id itself — are all the right moves. The test updates you made across test_store.py, test_timeline.py, and the snapshot files were also genuinely useful as a reference for what correct behaviour should look like.

Where I hesitated to merge was primarily around backward compatibility: changing RecurrenceId from str to a BaseModel is a breaking change for anyone comparing event.recurrence_id == "20220724T120000" or relying on model_dump() producing a string, and I didn't want to ship a break that didn't simultaneously close all the gaps. There was also the astimezone() vs replace(tzinfo=...) distinction in from_value() that would have caused silent matching failures for users in certain timezones.

What we did in #628 (just merged) is essentially Phase 1 built on your groundwork: we kept RecurrenceId as a str subclass for now but attached .tzinfo and .range metadata attributes using __new__, and wired up the TZID parsing from ParsedProperty.params and the inline string format. The store matching fix uses replace(tzinfo=...) as you identified.

The Phase 2 work — plumbing RANGE=THISANDFUTURE through store edit() and delete() the way you designed it — is still open, and your PR is the clearest prior art for how to do it. If you're still interested in contributing that piece, a rebase on top of the current main would be very welcome. The compatibility constraint is easier to satisfy now that TZID is already handled.