From 23d2bc283d4d02187b250d2055103e0374ee93d6 Mon Sep 17 00:00:00 2001 From: Igor Monadical Date: Wed, 21 Jan 2026 15:10:19 -0500 Subject: [PATCH] fix: ics non-sync bugfix (#823) * ics non-sync bugfix * fix tests --------- Co-authored-by: Igor Loskutov --- server/reflector/services/ics_sync.py | 60 +++++++++++++++++++-------- server/tests/test_ics_sync.py | 47 ++++++++++++++++++++- 2 files changed, 88 insertions(+), 19 deletions(-) diff --git a/server/reflector/services/ics_sync.py b/server/reflector/services/ics_sync.py index 2a4855cb..56f83495 100644 --- a/server/reflector/services/ics_sync.py +++ b/server/reflector/services/ics_sync.py @@ -319,21 +319,6 @@ class ICSSyncService: calendar = self.fetch_service.parse_ics(ics_content) content_hash = hashlib.md5(ics_content.encode()).hexdigest() - if room.ics_last_etag == content_hash: - logger.info("No changes in ICS for room", room_id=room.id) - room_url = f"{settings.UI_BASE_URL}/{room.name}" - events, total_events = self.fetch_service.extract_room_events( - calendar, room.name, room_url - ) - return { - "status": SyncStatus.UNCHANGED, - "hash": content_hash, - "events_found": len(events), - "total_events": total_events, - "events_created": 0, - "events_updated": 0, - "events_deleted": 0, - } # Extract matching events room_url = f"{settings.UI_BASE_URL}/{room.name}" @@ -371,6 +356,44 @@ class ICSSyncService: time_since_sync = datetime.now(timezone.utc) - room.ics_last_sync return time_since_sync.total_seconds() >= room.ics_fetch_interval + def _event_data_changed(self, existing: CalendarEvent, new_data: EventData) -> bool: + """Check if event data has changed by comparing relevant fields. + + IMPORTANT: When adding fields to CalendarEvent/EventData, update this method + and the _COMPARED_FIELDS set below for runtime validation. + """ + # Fields that come from ICS and should trigger updates when changed + _COMPARED_FIELDS = { + "title", + "description", + "start_time", + "end_time", + "location", + "attendees", + "ics_raw_data", + } + + # Runtime exhaustiveness check: ensure we're comparing all EventData fields + event_data_fields = set(EventData.__annotations__.keys()) - {"ics_uid"} + if event_data_fields != _COMPARED_FIELDS: + missing = event_data_fields - _COMPARED_FIELDS + extra = _COMPARED_FIELDS - event_data_fields + raise RuntimeError( + f"_event_data_changed() field mismatch: " + f"missing={missing}, extra={extra}. " + f"Update the comparison logic when adding/removing fields." + ) + + return ( + existing.title != new_data["title"] + or existing.description != new_data["description"] + or existing.start_time != new_data["start_time"] + or existing.end_time != new_data["end_time"] + or existing.location != new_data["location"] + or existing.attendees != new_data["attendees"] + or existing.ics_raw_data != new_data["ics_raw_data"] + ) + async def _sync_events_to_database( self, room_id: str, events: list[EventData] ) -> SyncStats: @@ -386,11 +409,14 @@ class ICSSyncService: ) if existing: - updated += 1 + # Only count as updated if data actually changed + if self._event_data_changed(existing, event_data): + updated += 1 + await calendar_events_controller.upsert(calendar_event) else: created += 1 + await calendar_events_controller.upsert(calendar_event) - await calendar_events_controller.upsert(calendar_event) current_ics_uids.append(event_data["ics_uid"]) # Soft delete events that are no longer in calendar diff --git a/server/tests/test_ics_sync.py b/server/tests/test_ics_sync.py index e448dd7d..cac10848 100644 --- a/server/tests/test_ics_sync.py +++ b/server/tests/test_ics_sync.py @@ -189,14 +189,17 @@ async def test_ics_sync_service_sync_room_calendar(): assert events[0].ics_uid == "sync-event-1" assert events[0].title == "Sync Test Meeting" - # Second sync with same content (should be unchanged) + # Second sync with same content (calendar unchanged, but sync always runs) # Refresh room to get updated etag and force sync by setting old sync time room = await rooms_controller.get_by_id(room.id) await rooms_controller.update( room, {"ics_last_sync": datetime.now(timezone.utc) - timedelta(minutes=10)} ) result = await sync_service.sync_room_calendar(room) - assert result["status"] == "unchanged" + assert result["status"] == "success" + assert result["events_created"] == 0 + assert result["events_updated"] == 0 + assert result["events_deleted"] == 0 # Third sync with updated event event["summary"] = "Updated Meeting Title" @@ -288,3 +291,43 @@ async def test_ics_sync_service_error_handling(): result = await sync_service.sync_room_calendar(room) assert result["status"] == "error" assert "Network error" in result["error"] + + +@pytest.mark.asyncio +async def test_event_data_changed_exhaustiveness(): + """Test that _event_data_changed compares all EventData fields (except ics_uid). + + This test ensures programmers don't forget to update the comparison logic + when adding new fields to EventData/CalendarEvent. + """ + from reflector.services.ics_sync import EventData + + sync_service = ICSSyncService() + + from reflector.db.calendar_events import CalendarEvent + + now = datetime.now(timezone.utc) + event_data: EventData = { + "ics_uid": "test-123", + "title": "Test", + "description": "Desc", + "location": "Loc", + "start_time": now, + "end_time": now + timedelta(hours=1), + "attendees": [], + "ics_raw_data": "raw", + } + + existing = CalendarEvent( + room_id="room1", + **event_data, + ) + + # Will raise RuntimeError if fields are missing from comparison + result = sync_service._event_data_changed(existing, event_data) + assert result is False + + modified_data = event_data.copy() + modified_data["title"] = "Changed Title" + result = sync_service._event_data_changed(existing, modified_data) + assert result is True