diff --git a/server/reflector/db/meetings.py b/server/reflector/db/meetings.py index 02f407b2..44a23920 100644 --- a/server/reflector/db/meetings.py +++ b/server/reflector/db/meetings.py @@ -346,6 +346,27 @@ async def get_by_calendar_event( return None return Meeting(**result) + async def get_by_room_and_time_window( + self, room: Room, start_date: datetime, end_date: datetime + ) -> Meeting | None: + """Check if a meeting already exists for this room with the same time window.""" + query = ( + meetings.select() + .where( + sa.and_( + meetings.c.room_id == room.id, + meetings.c.start_date == start_date, + meetings.c.end_date == end_date, + meetings.c.is_active, + ) + ) + .limit(1) + ) + result = await get_database().fetch_one(query) + if not result: + return None + return Meeting(**result) + async def update_meeting(self, meeting_id: str, **kwargs): query = meetings.update().where(meetings.c.id == meeting_id).values(**kwargs) await get_database().execute(query) diff --git a/server/reflector/worker/ics_sync.py b/server/reflector/worker/ics_sync.py index 6e126309..f02a2ddd 100644 --- a/server/reflector/worker/ics_sync.py +++ b/server/reflector/worker/ics_sync.py @@ -5,7 +5,7 @@ from celery.utils.log import get_task_logger from reflector.asynctask import asynctask -from reflector.db.calendar_events import calendar_events_controller +from reflector.db.calendar_events import CalendarEvent, calendar_events_controller from reflector.db.meetings import meetings_controller from reflector.db.rooms import Room, rooms_controller from reflector.redis_cache import RedisAsyncLock @@ -83,10 +83,9 @@ def _should_sync(room) -> bool: return time_since_sync.total_seconds() >= room.ics_fetch_interval -MEETING_DEFAULT_DURATION = timedelta(hours=1) - - -async def create_upcoming_meetings_for_event(event, create_window, room: Room): +async def create_upcoming_meetings_for_event( + event: CalendarEvent, create_window: datetime, room: Room +): if event.start_time <= create_window: return existing_meeting = await meetings_controller.get_by_calendar_event(event.id, room) @@ -94,6 +93,21 @@ async def create_upcoming_meetings_for_event(event, create_window, room: Room): if existing_meeting: return + # Prevent duplicate meetings from aggregated calendar feeds + # (e.g. same event appears with different UIDs from Cal.com and Google Calendar) + end_date = event.end_time + existing_by_time = await meetings_controller.get_by_room_and_time_window( + room, event.start_time, end_date + ) + if existing_by_time: + logger.info( + "Skipping duplicate calendar event - meeting already exists for this time window", + room_id=room.id, + event_id=event.id, + existing_meeting_id=existing_by_time.id, + ) + return + logger.info( "Pre-creating meeting for calendar event", room_id=room.id, @@ -102,8 +116,6 @@ async def create_upcoming_meetings_for_event(event, create_window, room: Room): ) try: - end_date = event.end_time or (event.start_time + MEETING_DEFAULT_DURATION) - client = create_platform_client(room.platform) meeting_data = await client.create_meeting( diff --git a/server/tests/test_ics_dedup.py b/server/tests/test_ics_dedup.py new file mode 100644 index 00000000..db9e2088 --- /dev/null +++ b/server/tests/test_ics_dedup.py @@ -0,0 +1,190 @@ +from datetime import datetime, timedelta, timezone +from unittest.mock import AsyncMock, patch + +import pytest + +from reflector.db import get_database +from reflector.db.calendar_events import CalendarEvent, calendar_events_controller +from reflector.db.meetings import meetings +from reflector.db.rooms import rooms_controller +from reflector.worker.ics_sync import create_upcoming_meetings_for_event + + +@pytest.mark.asyncio +async def test_duplicate_calendar_event_does_not_create_duplicate_meeting(): + """When an aggregated ICS feed contains the same event with different UIDs + (e.g. Cal.com UID + Google Calendar UUID), only one meeting should be created.""" + + room = await rooms_controller.add( + name="dedup-test-room", + user_id="test-user", + zulip_auto_post=False, + zulip_stream="", + zulip_topic="", + is_locked=False, + room_mode="normal", + recording_type="cloud", + recording_trigger="automatic-2nd-participant", + is_shared=False, + ics_url="https://calendar.example.com/dedup.ics", + ics_enabled=True, + ) + + now = datetime.now(timezone.utc) + start_time = now + timedelta(hours=1) + end_time = now + timedelta(hours=2) + + # Create first calendar event (Cal.com UID) + event1 = await calendar_events_controller.upsert( + CalendarEvent( + room_id=room.id, + ics_uid="abc123@Cal.com", + title="Team Standup", + start_time=start_time, + end_time=end_time, + ) + ) + + # create_window must be before start_time for the function to proceed + create_window = now - timedelta(minutes=6) + + # Create meeting for event1 + with patch( + "reflector.worker.ics_sync.create_platform_client" + ) as mock_platform_factory: + mock_client = AsyncMock() + + async def mock_create_meeting_1(room_name_prefix, *, end_date, room): + return AsyncMock( + meeting_id="meeting-1", + room_name="dedup-test-room-abc", + room_url="https://mock.video/dedup-test-room-abc", + host_room_url="https://mock.video/dedup-test-room-abc?host=true", + ) + + mock_client.create_meeting = mock_create_meeting_1 + mock_client.upload_logo = AsyncMock() + mock_platform_factory.return_value = mock_client + + await create_upcoming_meetings_for_event(event1, create_window, room) + + # Verify meeting was created + results = await get_database().fetch_all( + meetings.select().where(meetings.c.room_id == room.id) + ) + assert len(results) == 1, f"Expected 1 meeting, got {len(results)}" + + # Create second calendar event with different UID but same time window (Google Calendar UUID) + event2 = await calendar_events_controller.upsert( + CalendarEvent( + room_id=room.id, + ics_uid="550e8400-e29b-41d4-a716-446655440000", + title="Team Standup", + start_time=start_time, + end_time=end_time, + ) + ) + + # Try to create meeting for event2 - should be skipped due to dedup + with patch( + "reflector.worker.ics_sync.create_platform_client" + ) as mock_platform_factory: + mock_client = AsyncMock() + create_meeting_called = False + + async def mock_create_meeting_2(room_name_prefix, *, end_date, room): + nonlocal create_meeting_called + create_meeting_called = True + + mock_client.create_meeting = mock_create_meeting_2 + mock_client.upload_logo = AsyncMock() + mock_platform_factory.return_value = mock_client + + await create_upcoming_meetings_for_event(event2, create_window, room) + + # Platform client should NOT have been called for the duplicate + assert ( + not create_meeting_called + ), "create_meeting should not be called for duplicate" + + # Verify still only 1 meeting + results = await get_database().fetch_all( + meetings.select().where(meetings.c.room_id == room.id) + ) + assert len(results) == 1, f"Expected 1 meeting after dedup, got {len(results)}" + + +@pytest.mark.asyncio +async def test_different_time_windows_create_separate_meetings(): + """Events at different times should create separate meetings, even if titles match.""" + + room = await rooms_controller.add( + name="dedup-diff-time-room", + user_id="test-user", + zulip_auto_post=False, + zulip_stream="", + zulip_topic="", + is_locked=False, + room_mode="normal", + recording_type="cloud", + recording_trigger="automatic-2nd-participant", + is_shared=False, + ics_url="https://calendar.example.com/dedup2.ics", + ics_enabled=True, + ) + + now = datetime.now(timezone.utc) + create_window = now - timedelta(minutes=6) + + # Event 1: 1-2pm + event1 = await calendar_events_controller.upsert( + CalendarEvent( + room_id=room.id, + ics_uid="event-morning@Cal.com", + title="Team Standup", + start_time=now + timedelta(hours=1), + end_time=now + timedelta(hours=2), + ) + ) + + # Event 2: 3-4pm (different time) + event2 = await calendar_events_controller.upsert( + CalendarEvent( + room_id=room.id, + ics_uid="event-afternoon@Cal.com", + title="Team Standup", + start_time=now + timedelta(hours=3), + end_time=now + timedelta(hours=4), + ) + ) + + with patch( + "reflector.worker.ics_sync.create_platform_client" + ) as mock_platform_factory: + mock_client = AsyncMock() + + call_count = 0 + + async def mock_create_meeting(room_name_prefix, *, end_date, room): + nonlocal call_count + call_count += 1 + return AsyncMock( + meeting_id=f"meeting-{call_count}", + room_name=f"dedup-diff-time-room-{call_count}", + room_url=f"https://mock.video/dedup-diff-time-room-{call_count}", + host_room_url=f"https://mock.video/dedup-diff-time-room-{call_count}?host=true", + ) + + mock_client.create_meeting = mock_create_meeting + mock_client.upload_logo = AsyncMock() + mock_platform_factory.return_value = mock_client + + await create_upcoming_meetings_for_event(event1, create_window, room) + await create_upcoming_meetings_for_event(event2, create_window, room) + + results = await get_database().fetch_all( + meetings.select().where(meetings.c.room_id == room.id) + ) + assert ( + len(results) == 2 + ), f"Expected 2 meetings for different times, got {len(results)}" diff --git a/www/app/[roomName]/components/DailyRoom.tsx b/www/app/[roomName]/components/DailyRoom.tsx index d1c00254..c64b6d18 100644 --- a/www/app/[roomName]/components/DailyRoom.tsx +++ b/www/app/[roomName]/components/DailyRoom.tsx @@ -8,7 +8,7 @@ import { useRef, useState, } from "react"; -import { Box, Spinner, Center, Text } from "@chakra-ui/react"; +import { Box, Spinner, Center, Text, Button, VStack } from "@chakra-ui/react"; import { useRouter, useParams } from "next/navigation"; import DailyIframe, { DailyCall, @@ -16,10 +16,13 @@ import DailyIframe, { DailyCustomTrayButton, DailyCustomTrayButtons, DailyEventObjectCustomButtonClick, + DailyEventObjectFatalError, + DailyFatalErrorType, DailyFactoryOptions, DailyParticipantsObject, } from "@daily-co/daily-js"; import type { components } from "../../reflector-api"; +import { printApiError, ApiError } from "../../api/_error"; import { useAuth } from "../../lib/AuthProvider"; import { useConsentDialog } from "../../lib/consent"; import { @@ -45,6 +48,63 @@ const RAW_TRACKS_NAMESPACE = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; const RECORDING_START_DELAY_MS = 2000; const RECORDING_START_MAX_RETRIES = 5; +const FATAL_ERROR_MESSAGES: Partial< + Record +> = { + "connection-error": { + message: "Connection lost. Please check your network.", + rejoinable: true, + }, + "exp-room": { message: "The meeting time has ended." }, + "exp-token": { message: "Your session has expired.", rejoinable: true }, + ejected: { message: "You were removed from this meeting." }, + "meeting-full": { message: "This meeting is full." }, + "not-allowed": { message: "You are not allowed to join this meeting." }, + "nbf-room": { message: "This meeting hasn't started yet." }, + "nbf-token": { message: "This meeting hasn't started yet." }, + "no-room": { message: "This room does not exist." }, + "end-of-life": { message: "This meeting room is no longer available." }, +}; + +function FatalErrorScreen({ + error, + roomName, +}: { + error: FatalError; + roomName: string; +}) { + const router = useRouter(); + const info = + error.type !== "unknown" ? FATAL_ERROR_MESSAGES[error.type] : undefined; + const message = info?.message ?? `Something went wrong: ${error.message}`; + const rejoinable = info?.rejoinable ?? false; + + return ( +
+ + {message} + {rejoinable ? ( + <> + + + + ) : ( + + )} + +
+ ); +} + type Meeting = components["schemas"]["Meeting"]; type Room = components["schemas"]["RoomDetails"]; @@ -82,6 +142,8 @@ const USE_FRAME_INIT_STATE = { joined: false as boolean, } as const; +type FatalError = { type: DailyFatalErrorType | "unknown"; message: string }; + // Daily js and not Daily react used right now because daily-js allows for prebuild interface vs. -react is customizable but has no nice defaults const useFrame = ( container: HTMLDivElement | null, @@ -89,6 +151,7 @@ const useFrame = ( onLeftMeeting: () => void; onCustomButtonClick: (ev: DailyEventObjectCustomButtonClick) => void; onJoinMeeting: () => void; + onError: (ev: DailyEventObjectFatalError) => void; }, ) => { const [{ frame, joined }, setState] = useState(USE_FRAME_INIT_STATE); @@ -134,6 +197,7 @@ const useFrame = ( if (!frame) return; frame.on("left-meeting", cbs.onLeftMeeting); frame.on("custom-button-click", cbs.onCustomButtonClick); + frame.on("error", cbs.onError); const joinCb = () => { if (!frame) { console.error("frame is null in joined-meeting callback"); @@ -145,6 +209,7 @@ const useFrame = ( return () => { frame.off("left-meeting", cbs.onLeftMeeting); frame.off("custom-button-click", cbs.onCustomButtonClick); + frame.off("error", cbs.onError); frame.off("joined-meeting", joinCb); }; }, [frame, cbs]); @@ -188,6 +253,7 @@ export default function DailyRoom({ meeting, room }: DailyRoomProps) { const joinMutation = useRoomJoinMeeting(); const startRecordingMutation = useMeetingStartRecording(); const [joinedMeeting, setJoinedMeeting] = useState(null); + const [fatalError, setFatalError] = useState(null); // Generate deterministic instanceIds so all participants use SAME IDs const cloudInstanceId = parseNonEmptyString(meeting.id); @@ -234,8 +300,18 @@ export default function DailyRoom({ meeting, room }: DailyRoomProps) { const roomUrl = joinedMeeting?.room_url; const handleLeave = useCallback(() => { + // If a fatal error occurred, don't redirect — let the error UI show + if (fatalError) return; router.push("/browse"); - }, [router]); + }, [router, fatalError]); + + const handleError = useCallback((ev: DailyEventObjectFatalError) => { + const error: FatalError = { + type: ev.error?.type ?? "unknown", + message: ev.errorMsg, + }; + setFatalError(error); + }, []); const handleCustomButtonClick = useCallback( (ev: DailyEventObjectCustomButtonClick) => { @@ -324,6 +400,7 @@ export default function DailyRoom({ meeting, room }: DailyRoomProps) { onLeftMeeting: handleLeave, onCustomButtonClick: handleCustomButtonClick, onJoinMeeting: handleFrameJoinMeeting, + onError: handleError, }); useEffect(() => { @@ -380,13 +457,27 @@ export default function DailyRoom({ meeting, room }: DailyRoomProps) { } if (joinMutation.isError) { + const apiDetail = printApiError( + joinMutation.error as /*ref 095959E6-01CC-4CF0-B3A9-F65F12F046D3*/ ApiError, + ); return (
- Failed to join meeting. Please try again. + + + {apiDetail ?? "Failed to join meeting. Please try again."} + + +
); } + if (fatalError) { + return ; + } + if (!roomUrl) { return null; } diff --git a/www/app/lib/apiHooks.ts b/www/app/lib/apiHooks.ts index 788dfac6..58378f3e 100644 --- a/www/app/lib/apiHooks.ts +++ b/www/app/lib/apiHooks.ts @@ -9,6 +9,7 @@ import { MeetingId } from "./types"; import { NonEmptyString } from "./utils"; /* + * ref 095959E6-01CC-4CF0-B3A9-F65F12F046D3 * XXX error types returned from the hooks are not always correct; declared types are ValidationError but real type could be string or any other * this is either a limitation or incorrect usage of Python json schema generator * or, limitation or incorrect usage of .d type generator from json schema