From f15e1dc7f77d2ac082db289bc19e9a3ffb40011b Mon Sep 17 00:00:00 2001 From: Mathieu Virbel Date: Mon, 8 Sep 2025 16:22:44 -0600 Subject: [PATCH] feat: improve ICS calendar sync UX and fix room URL matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace "Test Connection" button with "Force Sync" button (Edit Room only) - Show detailed sync results: total events downloaded vs room matches - Remove emoticons and auto-hide timeout for cleaner UX - Fix room URL matching to use UI_BASE_URL instead of BASE_URL - Replace FaSync icon with LuRefreshCw for consistency - Clear sync results when dialog closes or Force Sync pressed - Update tests to reflect UI_BASE_URL change and exact URL matching 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- server/reflector/services/ics_sync.py | 47 ++-- server/tests/test_ics_background_tasks.py | 2 +- server/tests/test_ics_sync.py | 27 +- .../(app)/rooms/_components/ICSSettings.tsx | 133 ++++----- www/app/(app)/rooms/page.tsx | 253 +++++++++--------- 5 files changed, 225 insertions(+), 237 deletions(-) diff --git a/server/reflector/services/ics_sync.py b/server/reflector/services/ics_sync.py index f27debc0..cebb308c 100644 --- a/server/reflector/services/ics_sync.py +++ b/server/reflector/services/ics_sync.py @@ -36,8 +36,11 @@ class SyncStats(TypedDict): events_deleted: int -class SyncResult(TypedDict, total=False): +class SyncResultBase(TypedDict): status: str # "success", "unchanged", "error", "skipped" + + +class SyncResult(SyncResultBase, total=False): hash: str | None events_found: int total_events: int @@ -73,41 +76,41 @@ class ICSFetchService: window_end = now + timedelta(hours=24) for component in calendar.walk(): - if component.name == "VEVENT": - # Skip cancelled events - status = component.get("STATUS", "").upper() - if status == "CANCELLED": - continue + if component.name != "VEVENT": + continue - # Count total non-cancelled events in the time window - event_data = self._parse_event(component) - if ( - event_data - and window_start <= event_data["start_time"] <= window_end - ): - total_events += 1 + status = component.get("STATUS", "").upper() + if status == "CANCELLED": + continue - # Check if event matches this room - if self._event_matches_room(component, room_name, room_url): - events.append(event_data) + # Count total non-cancelled events in the time window + event_data = self._parse_event(component) + print(room_url, event_data) + if event_data and window_start <= event_data["start_time"] <= window_end: + total_events += 1 + + # Check if event matches this room + if self._event_matches_room(component, room_name, room_url): + events.append(event_data) return events, total_events def _event_matches_room(self, event: Event, room_name: str, room_url: str) -> bool: + print("_____", room_url) location = str(event.get("LOCATION", "")) description = str(event.get("DESCRIPTION", "")) - # Only match full room URL (with or without protocol) + # Only match full room URL + # XXX leaved here as a patterns, to later be extended with tinyurl or such too patterns = [ - room_url, # Full URL with protocol - room_url.replace("https://", ""), # Without https protocol - room_url.replace("http://", ""), # Without http protocol + room_url, ] # Check location and description for patterns text_to_check = f"{location} {description}".lower() for pattern in patterns: + print(text_to_check, pattern.lower()) if pattern.lower() in text_to_check: return True @@ -225,7 +228,7 @@ class ICSSyncService: logger.info(f"No changes in ICS for room {room.id}") # Still parse to get event count calendar = self.fetch_service.parse_ics(ics_content) - room_url = f"{settings.BASE_URL}/room/{room.name}" + room_url = f"{settings.UI_BASE_URL}/{room.name}" events, total_events = self.fetch_service.extract_room_events( calendar, room.name, room_url ) @@ -243,7 +246,7 @@ class ICSSyncService: calendar = self.fetch_service.parse_ics(ics_content) # Build room URL - room_url = f"{settings.BASE_URL}/room/{room.name}" + room_url = f"{settings.UI_BASE_URL}/{room.name}" # Extract matching events events, total_events = self.fetch_service.extract_room_events( diff --git a/server/tests/test_ics_background_tasks.py b/server/tests/test_ics_background_tasks.py index f2ea7ea4..68d77ab8 100644 --- a/server/tests/test_ics_background_tasks.py +++ b/server/tests/test_ics_background_tasks.py @@ -36,7 +36,7 @@ async def test_sync_room_ics_task(): event.add("summary", "Task Test Meeting") from reflector.settings import settings - event.add("location", f"{settings.BASE_URL}/room/{room.name}") + event.add("location", f"{settings.UI_BASE_URL}/{room.name}") now = datetime.now(timezone.utc) event.add("dtstart", now + timedelta(hours=1)) event.add("dtend", now + timedelta(hours=2)) diff --git a/server/tests/test_ics_sync.py b/server/tests/test_ics_sync.py index e4bada05..e448dd7d 100644 --- a/server/tests/test_ics_sync.py +++ b/server/tests/test_ics_sync.py @@ -13,7 +13,7 @@ from reflector.services.ics_sync import ICSFetchService, ICSSyncService async def test_ics_fetch_service_event_matching(): service = ICSFetchService() room_name = "test-room" - room_url = "https://example.com/room/test-room" + room_url = "https://example.com/test-room" # Create test event event = Event() @@ -21,12 +21,12 @@ async def test_ics_fetch_service_event_matching(): event.add("summary", "Test Meeting") # Test matching with full URL in location - event.add("location", "https://example.com/room/test-room") + event.add("location", "https://example.com/test-room") assert service._event_matches_room(event, room_name, room_url) is True - # Test matching with URL without protocol - event["location"] = "example.com/room/test-room" - assert service._event_matches_room(event, room_name, room_url) is True + # Test non-matching with URL without protocol (exact matching only now) + event["location"] = "example.com/test-room" + assert service._event_matches_room(event, room_name, room_url) is False # Test matching in description event["location"] = "Conference Room A" @@ -39,7 +39,7 @@ async def test_ics_fetch_service_event_matching(): assert service._event_matches_room(event, room_name, room_url) is False # Test partial paths should NOT match anymore - event["location"] = "/room/test-room" + event["location"] = "/test-room" assert service._event_matches_room(event, room_name, room_url) is False event["location"] = f"Room: {room_name}" @@ -55,7 +55,7 @@ async def test_ics_fetch_service_parse_event(): event.add("uid", "test-456") event.add("summary", "Team Standup") event.add("description", "Daily team sync") - event.add("location", "https://example.com/room/standup") + event.add("location", "https://example.com/standup") now = datetime.now(timezone.utc) event.add("dtstart", now) @@ -73,7 +73,7 @@ async def test_ics_fetch_service_parse_event(): assert result["ics_uid"] == "test-456" assert result["title"] == "Team Standup" assert result["description"] == "Daily team sync" - assert result["location"] == "https://example.com/room/standup" + assert result["location"] == "https://example.com/standup" assert len(result["attendees"]) == 3 # 2 attendees + 1 organizer @@ -81,7 +81,7 @@ async def test_ics_fetch_service_parse_event(): async def test_ics_fetch_service_extract_room_events(): service = ICSFetchService() room_name = "meeting" - room_url = "https://example.com/room/meeting" + room_url = "https://example.com/meeting" # Create calendar with multiple events cal = Calendar() @@ -100,7 +100,7 @@ async def test_ics_fetch_service_extract_room_events(): event2 = Event() event2.add("uid", "no-match") event2.add("summary", "Other Meeting") - event2.add("location", "https://example.com/room/other") + event2.add("location", "https://example.com/other") event2.add("dtstart", now + timedelta(hours=4)) event2.add("dtend", now + timedelta(hours=5)) cal.add_component(event2) @@ -125,9 +125,10 @@ async def test_ics_fetch_service_extract_room_events(): cal.add_component(event4) # Extract events - events = service.extract_room_events(cal, room_name, room_url) + events, total_events = service.extract_room_events(cal, room_name, room_url) assert len(events) == 2 + assert total_events == 3 # 3 events in time window (excluding cancelled) assert events[0]["ics_uid"] == "match-1" assert events[1]["ics_uid"] == "match-2" @@ -155,10 +156,10 @@ async def test_ics_sync_service_sync_room_calendar(): event = Event() event.add("uid", "sync-event-1") event.add("summary", "Sync Test Meeting") - # Use the actual BASE_URL from settings + # Use the actual UI_BASE_URL from settings from reflector.settings import settings - event.add("location", f"{settings.BASE_URL}/room/{room.name}") + event.add("location", f"{settings.UI_BASE_URL}/{room.name}") now = datetime.now(timezone.utc) event.add("dtstart", now + timedelta(hours=1)) event.add("dtend", now + timedelta(hours=2)) diff --git a/www/app/(app)/rooms/_components/ICSSettings.tsx b/www/app/(app)/rooms/_components/ICSSettings.tsx index cccd3edc..9cadb15d 100644 --- a/www/app/(app)/rooms/_components/ICSSettings.tsx +++ b/www/app/(app)/rooms/_components/ICSSettings.tsx @@ -12,8 +12,9 @@ import { Spinner, Box, } from "@chakra-ui/react"; -import { useState } from "react"; -import { FaSync, FaCheckCircle, FaExclamationCircle } from "react-icons/fa"; +import { useState, useEffect } from "react"; +import { LuRefreshCw } from "react-icons/lu"; +import { FaCheckCircle, FaExclamationCircle } from "react-icons/fa"; import { useRoomIcsSync, useRoomIcsStatus } from "../../../lib/apiHooks"; interface ICSSettingsProps { @@ -26,6 +27,7 @@ interface ICSSettingsProps { icsLastEtag?: string; onChange: (settings: Partial) => void; isOwner?: boolean; + isEditing?: boolean; } export interface ICSSettingsData { @@ -52,12 +54,18 @@ export default function ICSSettings({ icsLastEtag, onChange, isOwner = true, + isEditing = false, }: ICSSettingsProps) { const [syncStatus, setSyncStatus] = useState< "idle" | "syncing" | "success" | "error" >("idle"); const [syncMessage, setSyncMessage] = useState(""); - const [testResult, setTestResult] = useState(""); + const [syncResult, setSyncResult] = useState<{ + eventsFound: number; + totalEvents: number; + eventsCreated: number; + eventsUpdated: number; + } | null>(null); // React Query hooks const syncMutation = useRoomIcsSync(); @@ -67,46 +75,21 @@ export default function ICSSettings({ items: fetchIntervalOptions, }); - const handleTestConnection = async () => { - if (!icsUrl || !roomName) return; - - setSyncStatus("syncing"); - setTestResult(""); - - try { - // First notify parent to update the room with the ICS URL - onChange({ - ics_url: icsUrl, - ics_enabled: true, - ics_fetch_interval: icsFetchInterval, - }); - - // Then trigger a sync - const result = await syncMutation.mutateAsync({ - params: { - path: { room_name: roomName }, - }, - }); - - if (result.status === "success") { - setSyncStatus("success"); - setTestResult( - `Successfully synced! Found ${result.events_found} events.`, - ); - } else { - setSyncStatus("error"); - setTestResult(result.error || "Sync failed"); - } - } catch (err: any) { - setSyncStatus("error"); - setTestResult(err.body?.detail || "Failed to test ICS connection"); + // Clear sync results when dialog closes + useEffect(() => { + if (!isEditing) { + setSyncStatus("idle"); + setSyncResult(null); + setSyncMessage(""); } - }; + }, [isEditing]); - const handleManualSync = async () => { - if (!roomName) return; + const handleForceSync = async () => { + if (!roomName || !isEditing) return; + // Clear previous results setSyncStatus("syncing"); + setSyncResult(null); setSyncMessage(""); try { @@ -116,26 +99,22 @@ export default function ICSSettings({ }, }); - if (result.status === "success") { + if (result.status === "success" || result.status === "unchanged") { setSyncStatus("success"); - setSyncMessage( - `Sync complete! Found ${result.events_found} events, ` + - `created ${result.events_created}, updated ${result.events_updated}.`, - ); + setSyncResult({ + eventsFound: result.events_found || 0, + totalEvents: result.total_events || 0, + eventsCreated: result.events_created || 0, + eventsUpdated: result.events_updated || 0, + }); } else { setSyncStatus("error"); setSyncMessage(result.error || "Sync failed"); } } catch (err: any) { setSyncStatus("error"); - setSyncMessage(err.body?.detail || "Failed to sync calendar"); + setSyncMessage(err.body?.detail || "Failed to force sync calendar"); } - - // Clear status after 5 seconds - setTimeout(() => { - setSyncStatus("idle"); - setSyncMessage(""); - }, 5000); }; if (!isOwner) { @@ -198,46 +177,48 @@ export default function ICSSettings({ - {icsUrl && ( + {icsUrl && isEditing && roomName && ( - - {roomName && icsLastSync && ( - - )} )} - {testResult && ( + {syncResult && syncStatus === "success" && ( - - {testResult} - + + + Sync completed + + + {syncResult.totalEvents} events downloaded,{" "} + {syncResult.eventsFound} match this room + + {(syncResult.eventsCreated > 0 || + syncResult.eventsUpdated > 0) && ( + + {syncResult.eventsCreated} created,{" "} + {syncResult.eventsUpdated} updated + + )} + )} diff --git a/www/app/(app)/rooms/page.tsx b/www/app/(app)/rooms/page.tsx index c79e308f..6b0b1f5f 100644 --- a/www/app/(app)/rooms/page.tsx +++ b/www/app/(app)/rooms/page.tsx @@ -448,6 +448,7 @@ export default function RoomsList() { General Calendar Share + Webhook @@ -613,9 +614,134 @@ export default function RoomsList() { Shared room + - {/* Webhook Configuration Section */} - + + { + setRoomInput({ + ...room, + icsUrl: + settings.ics_url !== undefined + ? settings.ics_url + : room.icsUrl, + icsEnabled: + settings.ics_enabled !== undefined + ? settings.ics_enabled + : room.icsEnabled, + icsFetchInterval: + settings.ics_fetch_interval !== undefined + ? settings.ics_fetch_interval + : room.icsFetchInterval, + }); + }} + isOwner={true} + isEditing={isEditing} + /> + + + + + { + const syntheticEvent = { + target: { + name: "zulipAutoPost", + type: "checkbox", + checked: e.checked, + }, + }; + handleRoomChange(syntheticEvent); + }} + > + + + + + + Automatically post transcription to Zulip + + + + + + Zulip stream + + setRoomInput({ + ...room, + zulipStream: e.value[0], + zulipTopic: "", + }) + } + collection={streamCollection} + disabled={!room.zulipAutoPost} + > + + + + + + + + + + + + {streamOptions.map((option) => ( + + {option.label} + + + ))} + + + + + + + Zulip topic + + setRoomInput({ ...room, zulipTopic: e.value[0] }) + } + collection={topicCollection} + disabled={!room.zulipAutoPost} + > + + + + + + + + + + + + {topicOptions.map((option) => ( + + {option.label} + + + ))} + + + + + + + + Webhook URL )} - - - { - setRoomInput({ - ...room, - icsUrl: - settings.ics_url !== undefined - ? settings.ics_url - : room.icsUrl, - icsEnabled: - settings.ics_enabled !== undefined - ? settings.ics_enabled - : room.icsEnabled, - icsFetchInterval: - settings.ics_fetch_interval !== undefined - ? settings.ics_fetch_interval - : room.icsFetchInterval, - }); - }} - isOwner={true} - /> - - - - - { - const syntheticEvent = { - target: { - name: "zulipAutoPost", - type: "checkbox", - checked: e.checked, - }, - }; - handleRoomChange(syntheticEvent); - }} - > - - - - - - Automatically post transcription to Zulip - - - - - - Zulip stream - - setRoomInput({ - ...room, - zulipStream: e.value[0], - zulipTopic: "", - }) - } - collection={streamCollection} - disabled={!room.zulipAutoPost} - > - - - - - - - - - - - - {streamOptions.map((option) => ( - - {option.label} - - - ))} - - - - - - - Zulip topic - - setRoomInput({ ...room, zulipTopic: e.value[0] }) - } - collection={topicCollection} - disabled={!room.zulipAutoPost} - > - - - - - - - - - - - - {topicOptions.map((option) => ( - - {option.label} - - - ))} - - - - -