Fix CalDAV connect / disconnect, list calendar refresh and external_connection_id (#1212)

* fix connect and disconnect caldav on frontend

* connect caldav calendars with their external_connection_id

* read caldav attributes directly from vevent

* removing unnecessary check

* guard vevent attributes with hasattr

* add test for CalDAV list_events

Author: davinotdavid

backend/src/appointment/controller/calendar.py

@@ -422,7 +422,7 @@ def test_connection(self) -> bool:
         # They need at least VEVENT support for appointment to work.
         return supports_vevent
 
-    def sync_calendars(self):
+    def sync_calendars(self, external_connection_id: int | None = None):
         error_occurred = False
 
         principal = self.client.principal()
@@ -443,7 +443,11 @@ def sync_calendars(self):
             # add calendar
             try:
                 repo.calendar.update_or_create(
-                    db=self.db, calendar=calendar, calendar_url=calendar.url, subscriber_id=self.subscriber_id
+                    db=self.db,
+                    calendar=calendar,
+                    calendar_url=calendar.url,
+                    subscriber_id=self.subscriber_id,
+                    external_connection_id=external_connection_id,
                 )
             except Exception as err:
                 logging.warning(f'[calendar.sync_calendars] Error occurred while creating calendar. Error: {str(err)}')
@@ -495,18 +499,24 @@ def list_events(self, start, end):
             if not vevent:
                 continue
 
-            event_components = vevent.components()
+            # Ignore events with missing dtstart
+            if not hasattr(vevent, 'dtstart') or not vevent.dtstart:
+                continue
+
+            # Check for either dtend or duration (need at least one to determine event end)
+            has_dtend = hasattr(vevent, 'dtend') and vevent.dtend
+            has_duration = hasattr(vevent, 'duration') and vevent.duration
 
-            # Ignore events with missing datetime data
-            if 'dtstart' not in event_components or (
-                'dtend' not in event_components and 'duration' not in event_components
-            ):
+            if not has_dtend and not has_duration:
                 continue
 
+            # Check for event summary or use default title
+            has_summary = hasattr(vevent, 'summary') and vevent.summary
+            title = vevent.summary.value if has_summary else l10n('event-summary-default')
+
             # Mark tentative events
             tentative = status == 'tentative'
 
-            title = vevent.summary.value if 'summary' in event_components else l10n('event-summary-default')
             start = vevent.dtstart.value
             # get_duration grabs either end or duration into a timedelta
             end = start + e.get_duration()
@@ -565,7 +575,12 @@ def delete_events(self, start):
         result = calendar.events()
         count = 0
         for e in result:
-            if str(e.vobject_instance.vevent.dtstart.value).startswith(start):
+            vevent = e.vobject_instance.vevent
+            if not vevent:
+                continue
+
+            has_dtstart = hasattr(vevent, 'dtstart') and vevent.dtstart
+            if has_dtstart and str(vevent.dtstart.value).startswith(start):
                 e.delete()
                 count += 1
 

backend/src/appointment/routes/caldav.py

@@ -118,13 +118,13 @@ def caldav_autodiscover_auth(
             token=connection.password,
         )
 
-        repo.external_connection.create(db, external_connection_schema)
+        external_connection = repo.external_connection.create(db, external_connection_schema)
     else:
-        repo.external_connection.update_token(
+        external_connection = repo.external_connection.update_token(
             db, connection.password, subscriber.id, models.ExternalConnectionType.caldav, caldav_id
         )
 
-    con.sync_calendars()
+    con.sync_calendars(external_connection_id=external_connection.id)
     return True
 
 

backend/test/factory/calendar_factory.py

@@ -16,6 +16,7 @@ def _make_caldav_calendar(
         connected=False,
         user=FAKER_RANDOM_VALUE,
         password=FAKER_RANDOM_VALUE,
+        external_connection_id=None,
     ):
         with with_db() as db:
             title = title if factory_has_value(title) else fake.name()
@@ -31,6 +32,7 @@ def _make_caldav_calendar(
                     password=password if factory_has_value(password) else fake.password(),
                 ),
                 subscriber_id,
+                external_connection_id=external_connection_id,
             )
 
     return _make_caldav_calendar

backend/test/integration/test_appointment.py

@@ -1,6 +1,7 @@
 import pytest
 import dateutil.parser
 from unittest.mock import patch, MagicMock
+from datetime import datetime
 
 from defines import DAY1, DAY3, auth_headers, TEST_USER_ID
 from appointment.database.repo import appointment as appointment_repo
@@ -53,6 +54,131 @@ def list_events(self, start, end):
         assert data[0]['start'] == generated_appointment.slots[0].start.isoformat()
         assert data[0]['end'] == dateutil.parser.parse(DAY3).isoformat()
 
+    def test_caldav_list_events_with_missing_properties(self, make_appointment, monkeypatch):
+        """Test that CalDavConnector.list_events handles events with missing properties gracefully"""
+        from appointment.controller.calendar import CalDavConnector
+
+        # Create mock events with various missing properties
+        class MockVEvent:
+            def __init__(self, has_summary=True, has_dtend=True, has_duration=False):
+                self.dtstart = MagicMock()
+                self.dtstart.value = datetime(2023, 12, 1, 10, 0, 0)
+
+                if has_summary:
+                    self.summary = MagicMock()
+                    self.summary.value = "Test Event"
+
+                if has_dtend:
+                    self.dtend = MagicMock()
+                    self.dtend.value = datetime(2023, 12, 1, 11, 0, 0)
+                elif has_duration:
+                    self.duration = MagicMock()
+                    self.duration.value = "PT1H"  # 1 hour
+
+        class MockVObjectInstance:
+            def __init__(self, has_summary=True, has_dtend=True, has_duration=False):
+                self.vevent = MockVEvent(has_summary, has_dtend, has_duration)
+
+        class MockEvent:
+            def __init__(self, has_summary=True, has_dtend=True, has_duration=False):
+                self.icalendar_component = {'status': 'confirmed'}
+                self.vobject_instance = MockVObjectInstance(has_summary, has_dtend, has_duration)
+
+            def get_duration(self):
+                from datetime import timedelta
+                return timedelta(hours=1)
+
+        # Create various test events
+        mock_events = [
+            MockEvent(has_summary=True, has_dtend=True),     # Normal event
+            MockEvent(has_summary=False, has_dtend=True),    # Missing summary
+            MockEvent(has_summary=True, has_dtend=False, has_duration=True),  # Has duration instead of dtend
+            MockEvent(has_summary=False, has_dtend=False, has_duration=True), # Missing summary and dtend
+        ]
+
+        # Add events that should be filtered out by our guards
+        class MockBadVEvent:
+            """VEvent with missing critical properties"""
+            def __init__(self, missing_dtstart=False, missing_both_end_props=False):
+                if not missing_dtstart:
+                    self.dtstart = MagicMock()
+                    self.dtstart.value = datetime(2023, 12, 1, 10, 0, 0)
+
+                if not missing_both_end_props:
+                    self.dtend = MagicMock()
+                    self.dtend.value = datetime(2023, 12, 1, 11, 0, 0)
+
+        class MockBadVObjectInstance:
+            def __init__(self, missing_dtstart=False, missing_both_end_props=False):
+                self.vevent = MockBadVEvent(missing_dtstart, missing_both_end_props)
+
+        class MockBadEvent:
+            def __init__(self, missing_dtstart=False, missing_both_end_props=False):
+                self.icalendar_component = {'status': 'confirmed'}
+                self.vobject_instance = MockBadVObjectInstance(missing_dtstart, missing_both_end_props)
+
+            def get_duration(self):
+                from datetime import timedelta
+                return timedelta(hours=1)
+
+        # Add events that should be filtered out
+        mock_events.extend([
+            MockBadEvent(missing_dtstart=True),           # Missing dtstart - should be filtered
+            MockBadEvent(missing_both_end_props=True),    # Missing both dtend and duration - should be filtered
+        ])
+
+        def mock_search(start, end, event=True, expand=True):
+            return mock_events
+
+        def mock_calendar(url):
+            calendar_mock = MagicMock()
+            calendar_mock.search = mock_search
+            return calendar_mock
+
+        def mock_client_calendar(url):
+            return mock_calendar(url)
+
+        # Set up the CalDavConnector with mocked components
+        connector = CalDavConnector(
+            db=None,
+            subscriber_id=1,
+            calendar_id=1,
+            redis_instance=None,
+            url="https://test.com/caldav",
+            user="test",
+            password="test"
+        )
+
+        # Mock the client.calendar method
+        connector.client = MagicMock()
+        connector.client.calendar = mock_client_calendar
+
+        # Mock the caching methods
+        connector.get_cached_events = MagicMock(return_value=None)
+        connector.put_cached_events = MagicMock()
+
+        # Test the method with problematic events
+        start_str = "2023-12-01"
+        end_str = "2023-12-02"
+
+        # This should not raise any exceptions despite missing properties
+        events = connector.list_events(start_str, end_str)
+
+        # Verify the results - should only process 4 valid events, filtering out 2 bad ones
+        assert len(events) == 4, "Should process 4 valid events and filter out 2 invalid ones"
+
+        # Check that events with missing summary get default title
+        events_with_default_title = [e for e in events if 'event-summary-default' in e.title]
+        assert len(events_with_default_title) == 2, "Events without summary should get default title"
+
+        # Check that all events have required fields
+        for event in events:
+            assert event.title is not None
+            assert event.start is not None
+            assert event.end is not None
+            assert isinstance(event.all_day, bool)
+            assert isinstance(event.tentative, bool)
+
     def test_get_remote_caldav_events_invalid_calendar(self, with_client, make_appointment):
         generated_appointment = make_appointment()
 

backend/test/integration/test_calendar.py

@@ -9,7 +9,7 @@
 
 from sqlalchemy import select
 
-from defines import auth_headers
+from defines import auth_headers, TEST_USER_ID
 
 
 def get_calendar_factory():
@@ -423,3 +423,93 @@ def test_update_existing_caldav_calendar_without_password(self, with_client, wit
         assert cal.url == os.getenv('CALDAV_TEST_CALENDAR_URL')
         assert cal.user == os.getenv('CALDAV_TEST_USER')
         assert cal.password == ''
+
+    def test_caldav_auth_links_external_connection_id(self, with_client, with_db, monkeypatch):
+        """Test that creating calendars through /caldav/auth correctly links external_connection_id"""
+
+        # Mock the CalDAV client and its methods
+        class MockCalendar:
+            def __init__(self, name, url):
+                self.name = name  
+                self.url = url
+
+        class MockPrincipal:
+            def calendars(self):
+                return [
+                    MockCalendar("Test Calendar 1", "https://test-server.com/calendar1/"),
+                    MockCalendar("Test Calendar 2", "https://test-server.com/calendar2/"),
+                ]
+
+        class MockClient:
+            def principal(self):
+                return MockPrincipal()
+
+        from appointment.controller.calendar import CalDavConnector
+
+        def mock_init(self, db, redis_instance, url, user, password, subscriber_id, calendar_id):
+            # Store the parameters we need
+            self.db = db
+            self.subscriber_id = subscriber_id
+            self.user = user
+            self.password = password
+            self.client = MockClient()
+
+        def mock_test_connection(self):
+            return True
+
+        def mock_is_supported(self, cal):
+            return True
+
+        def mock_bust_cached_events(self, all_calendars=False):
+            # Mock implementation - no-op for testing
+            pass
+
+        # Apply all the mocks
+        monkeypatch.setattr(CalDavConnector, '__init__', mock_init)
+        monkeypatch.setattr(CalDavConnector, 'test_connection', mock_test_connection)
+        monkeypatch.setattr(CalDavConnector, '_is_supported', mock_is_supported)
+        monkeypatch.setattr(CalDavConnector, 'bust_cached_events', mock_bust_cached_events)
+
+        # Call the /caldav/auth endpoint
+        response = with_client.post(
+            '/caldav/auth',
+            json={
+                'url': 'https://test-caldav-server.com',
+                'user': 'test_user',
+                'password': 'test_password',
+            },
+            headers=auth_headers,
+        )
+
+        assert response.status_code == 200, response.text
+
+        with with_db() as db:
+            # Check that an external connection was created
+            external_connections_query = select(models.ExternalConnections).where(
+                models.ExternalConnections.owner_id == TEST_USER_ID
+            )
+            external_connections = db.scalars(external_connections_query).all()
+            assert len(external_connections) == 1
+
+            external_connection = external_connections[0]
+            assert external_connection.type == models.ExternalConnectionType.caldav
+            assert external_connection.name == 'test_user'
+
+            # Check that calendars were created with the correct external_connection_id
+            calendars_query = select(models.Calendar).where(
+                models.Calendar.owner_id == TEST_USER_ID
+            )
+            calendars = db.scalars(calendars_query).all()
+            assert len(calendars) == 2  # We mocked 2 calendars
+
+            # Verify both calendars have the correct external_connection_id
+            for calendar in calendars:
+                assert calendar.external_connection_id == external_connection.id
+                assert calendar.provider == models.CalendarProvider.caldav
+                assert calendar.user == 'test_user'
+                assert calendar.password == 'test_password'
+
+            # Verify calendar titles match our mock data  
+            calendar_titles = [cal.title for cal in calendars]
+            assert "Test Calendar 1" in calendar_titles
+            assert "Test Calendar 2" in calendar_titles

frontend/src/views/DashboardView/index.vue

@@ -55,7 +55,7 @@ onMounted(async () => {
   }
 
   await refresh();
-  await calendarStore.getRemoteEvents(activeDate.value);
+  await calendarStore.getRemoteEvents(activeDate.value, true);
 });
 </script>