python-caldav / caldav

Apache License 2.0
321 stars 95 forks source link

Improvements, test code and documentation needed for editing and selecting recurrence instances #398

Open tobixen opened 4 months ago

tobixen commented 4 months ago

Ref https://github.com/python-caldav/caldav/issues/35#issuecomment-2125631113 and below

tobixen commented 4 months ago

Some thoughts ...

Below, those definitions apply:

An Event-object can contain quite some different things (binary number representing suggested new check-methods below):

There needs to be methods on the Event to easily check what kind of data the Event contains (perhaps is_single, contains_recurring, contains_recurrence_instance. That's three booleans, represented by the binary in the list above. Note that 111 and 010 are an invalid combinations.

When encountering recurring events, the search-method will act differently depending on weather the flags split_expanded and expanded are set. Both are True by default (if a date range is given in the search). In the "split expand"-mode, recurrence instances are yielded as separate Event-objects (a list will be returned containing 100 and 101 type of Events). When expand is set, but split_expanded is not set, a set of recurrence instances may be yielded (001 - with 101 and 100 also being valid dependent on what kind of data exists on the server).

What if split_expanded is set and expand is not set? It still ought to return every recurrence instance in separate Event-objects, but I don't think I had it on the radar while the code was written, so we need test code for this case. Does the current expand-code support special recurrence instances? I believe it's needed to write test code for this.

split_expanded is specific for the search-method, but all methods returning Events should have this functionality built-in. The flag should be renamed, split_recurrences seems like a good idea - and of course, search should still support the deprecated split_expanded-flag until 3.0 is released.

A library client that wants to edit a recurrence instance should be able to do it regardless on weather the Event-object is of type 101 or 011. For sure 011 works today, 101 needs to be tested (it may be that different servers behaves differently, then it would probably be appropriate to have client logic converting the 101 into an 011.

Same goes when a library client wants to create a new special recurrence instance. It should be doable by editing an autogenerated 101 and saving it, or by adding it to the icalendar data of a 011.

Deleting a recurrence instance (see #35) is not very well defined, if running Event.delete on a 101, it should possibly raise an error rather than doing something potentially harmful. (Today I think it will delete the whole recurrence-set, that's definitively not what one wants). At the other hand, a traceback is likely to blow up on some end-user sooner or later, probably setting the status to CANCELLED would be the better idea.

Today we have a property icalendar_component which is very useful for the 1xx-cases, but will do the completely wrong thing for 0xx.. The usage of icalendar_component is recommended in the documentation. I'm not sure what to do with this. Possibly to let a list of 1xx be the default return type of any methods returning an Event and to be very clear in the documentation about the issues with icalendar_component. This change may possibly be introduced carefully from version 2.0 by deprecating the current API and writing a new API.