python-caldav / caldav

Apache License 2.0
313 stars 91 forks source link

set_relation reltype can be ignored because of type(uid)==icalendar.vText #334

Open Zocker1999NET opened 10 months ago

Zocker1999NET commented 10 months ago

Description

When using set_relation on a component (e.g. todo), the reltype might be ignored. This is because the library icalendar ignores properties if the value of a property is already of an internal type (see code in question). This happens if the UID is directly taken from the icalendar object and is of type vText.

Testing

cal: caldav.Calendar  # assume valid calendar
task = cal.add_todo(summary="First Task")
dependent = cal.add_todo(summary="Dependent")
dependent.set_relation(other=task, reltype="DEPENDS-ON", set_reverse=False)
print(dependent.icalendar_instance.to_ical())

Check output that reltype got lost.

Possible Mitigations

Converting the uid to a string explicitly before calling self.icalendar_component.add in set_relation works for me. This may be a possible workaround until icalendar potentially fixes this unexpected behavior. I can a add a PR for that, if requested.

I also submitted this issue to icalendar (see collective/icalendar#557). However, I see that changing this behavior in icalendar might introduce issues for other dependent projects, so I do not expect them to fix this issue soon.

tobixen commented 10 months ago

I've also been struck by this, but thought I had fixed it already. Yes, converting vText to string whenever appropriate seems to be the solution. Send a merge/pull-request (it would be awesome with a pull-request including test code), or perhaps I'll get around to do research on it myself later this week.

Zocker1999NET commented 10 months ago

I added a PR, #335, but without the tests. I can try add them later this or next week, as I have not that much free time now.

tobixen commented 10 months ago

My employer is currently having a customer with short deadlines and decent budget, unfortunately during the same days as I'm meeting up with family ... so I'm also overloaded for the moment :-)

I'll reopen the issue as tests are missing.