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

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

Open
Zocker1999NET opened this issue Sep 19, 2023 · 3 comments · Fixed by #335
Open

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

Zocker1999NET opened this issue Sep 19, 2023 · 3 comments · Fixed by #335

Comments

@Zocker1999NET
Copy link
Contributor

Zocker1999NET commented Sep 19, 2023

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
Copy link
Member

tobixen commented Sep 19, 2023

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
Copy link
Contributor Author

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
Copy link
Member

tobixen commented Sep 19, 2023

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.

@tobixen tobixen reopened this Sep 19, 2023
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 a pull request may close this issue.

2 participants