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

Handle missing vevent attributes #751

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

devmount
Copy link
Collaborator

Description of the Change

This change ignores all remote events, that don't have a dtstart or neither a dtend nor duration. Also the event title gets a default value, if summary is missing.

Benefits

Wider range of supported caldav servers.

Applicable Issues

Fixes #741

Copy link
Collaborator Author

@devmount devmount left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we actually test this? Our current tests (correct me if I'm wrong) always fake the list_events() function. Is there a way to acutally test the caldav connector?

backend/src/appointment/controller/calendar.py Outdated Show resolved Hide resolved
@devmount devmount self-assigned this Nov 18, 2024
@MelissaAutumn
Copy link
Member

How can we actually test this? Our current tests (correct me if I'm wrong) always fake the list_events() function. Is there a way to acutally test the caldav connector?

We can create some mock ical files, and have list_events return those files initialized into icalendar (iirc) instead. It's a little confusing because I think python-caldav handles the server requests and responses, but then it uses icalendar for the calendar/event objects...I think.

Copy link
Member

@MelissaAutumn MelissaAutumn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@devmount
Copy link
Collaborator Author

Good idea, I'll play around with that a bit and create some tests. Those should live in test_calendar.py rather than test_appointment.py, right?

@MelissaAutumn
Copy link
Member

Yep probably as a unit test if you want to test the functions directly. Let me know if you run into any issues there!

@devmount
Copy link
Collaborator Author

Postponed the test creation for now.

@devmount devmount merged commit 96d7cf3 into main Nov 19, 2024
4 checks passed
@devmount devmount deleted the bugs/741-handle-missing-caldav-event-attributes branch November 19, 2024 15:21
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 this pull request may close these issues.

Check before accessing event attributes on caldav
2 participants