From b9d483b29dccb8c311d2652b1da9f0f5c1de5b22 Mon Sep 17 00:00:00 2001 From: Mathieu Virbel Date: Thu, 11 Sep 2025 11:52:34 -0600 Subject: [PATCH] Remove grace period logic and improve meeting deactivation - Removed grace_period_minutes and last_participant_left_at fields - Simplified deactivation logic based on actual usage patterns: * Active sessions: Keep meeting active regardless of scheduled time * Calendar meetings: Wait until scheduled end if unused, deactivate immediately once used and empty * On-the-fly meetings: Deactivate immediately when empty - Created migration to drop unused database columns - Updated tests to remove grace period test cases --- ...dc035ff72fd5_remove_grace_period_fields.py | 43 +++++++ server/reflector/db/meetings.py | 4 - server/reflector/views/rooms.py | 4 - server/reflector/views/whereby.py | 6 - server/reflector/worker/process.py | 81 ++++++------- server/tests/test_multiple_active_meetings.py | 112 ------------------ www/app/reflector-api.d.ts | 7 -- 7 files changed, 78 insertions(+), 179 deletions(-) create mode 100644 server/migrations/versions/dc035ff72fd5_remove_grace_period_fields.py diff --git a/server/migrations/versions/dc035ff72fd5_remove_grace_period_fields.py b/server/migrations/versions/dc035ff72fd5_remove_grace_period_fields.py new file mode 100644 index 00000000..c38a0227 --- /dev/null +++ b/server/migrations/versions/dc035ff72fd5_remove_grace_period_fields.py @@ -0,0 +1,43 @@ +"""remove_grace_period_fields + +Revision ID: dc035ff72fd5 +Revises: d8e204bbf615 +Create Date: 2025-09-11 10:36:45.197588 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "dc035ff72fd5" +down_revision: Union[str, None] = "d8e204bbf615" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Remove grace period columns from meeting table + op.drop_column("meeting", "last_participant_left_at") + op.drop_column("meeting", "grace_period_minutes") + + +def downgrade() -> None: + # Add back grace period columns to meeting table + op.add_column( + "meeting", + sa.Column( + "last_participant_left_at", sa.DateTime(timezone=True), nullable=True + ), + ) + op.add_column( + "meeting", + sa.Column( + "grace_period_minutes", + sa.Integer(), + server_default=sa.text("15"), + nullable=True, + ), + ) diff --git a/server/reflector/db/meetings.py b/server/reflector/db/meetings.py index f9fd9efd..261a6db2 100644 --- a/server/reflector/db/meetings.py +++ b/server/reflector/db/meetings.py @@ -51,8 +51,6 @@ meetings = sa.Table( ), ), sa.Column("calendar_metadata", JSONB), - sa.Column("last_participant_left_at", sa.DateTime(timezone=True)), - sa.Column("grace_period_minutes", sa.Integer, server_default=sa.text("15")), sa.Index("idx_meeting_room_id", "room_id"), sa.Index("idx_meeting_calendar_event", "calendar_event_id"), ) @@ -100,8 +98,6 @@ class Meeting(BaseModel): is_active: bool = True calendar_event_id: str | None = None calendar_metadata: dict[str, Any] | None = None - last_participant_left_at: datetime | None = None - grace_period_minutes: int = 15 class MeetingController: diff --git a/server/reflector/views/rooms.py b/server/reflector/views/rooms.py index f3976baf..fc7bfbd8 100644 --- a/server/reflector/views/rooms.py +++ b/server/reflector/views/rooms.py @@ -77,8 +77,6 @@ class Meeting(BaseModel): is_active: bool = True calendar_event_id: str | None = None calendar_metadata: dict[str, Any] | None = None - last_participant_left_at: datetime | None = None - grace_period_minutes: int = 15 class CreateRoom(BaseModel): @@ -475,7 +473,6 @@ async def rooms_list_active_meetings( room_name: str, user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)], ): - """List all active meetings for a room (supports multiple active meetings)""" user_id = user["sub"] if user else None room = await rooms_controller.get_by_name(room_name) @@ -501,7 +498,6 @@ async def rooms_join_meeting( meeting_id: str, user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)], ): - """Join a specific meeting by ID""" user_id = user["sub"] if user else None room = await rooms_controller.get_by_name(room_name) diff --git a/server/reflector/views/whereby.py b/server/reflector/views/whereby.py index 7a6af45e..d12b0a9f 100644 --- a/server/reflector/views/whereby.py +++ b/server/reflector/views/whereby.py @@ -69,12 +69,6 @@ async def whereby_webhook(event: WherebyWebhookEvent, request: Request): if event.type in ["room.client.joined", "room.client.left"]: update_data = {"num_clients": event.data["numClients"]} - - # Clear grace period if participant joined - if event.type == "room.client.joined" and event.data["numClients"] > 0: - if meeting.last_participant_left_at: - update_data["last_participant_left_at"] = None - await meetings_controller.update_meeting(meeting.id, **update_data) return {"status": "ok"} diff --git a/server/reflector/worker/process.py b/server/reflector/worker/process.py index daeafba5..a17dd081 100644 --- a/server/reflector/worker/process.py +++ b/server/reflector/worker/process.py @@ -1,6 +1,6 @@ import json import os -from datetime import datetime, timedelta, timezone +from datetime import datetime, timezone from urllib.parse import unquote import av @@ -149,7 +149,15 @@ async def process_recording(bucket_name: str, object_key: str): async def process_meetings(): """ Checks which meetings are still active and deactivates those that have ended. - Supports multiple active meetings per room and grace period logic. + + Deactivation logic: + - Active sessions: Keep meeting active regardless of scheduled time + - No active sessions: + * Calendar meetings: + - If previously used (had sessions): Deactivate immediately + - If never used: Keep active until scheduled end time, then deactivate + * On-the-fly meetings: Deactivate immediately (created when someone joins, + so no sessions means everyone left) """ logger.info("Processing meetings") meetings = await meetings_controller.get_all_active() @@ -161,56 +169,37 @@ async def process_meetings(): if end_date.tzinfo is None: end_date = end_date.replace(tzinfo=timezone.utc) - # Check if meeting has passed its scheduled end time - if end_date <= current_time: - # For calendar meetings, force close 30 minutes after scheduled end + response = await get_room_sessions(meeting.room_name) + room_sessions = response.get("results", []) + has_active_sessions = room_sessions and any( + rs["endedAt"] is None for rs in room_sessions + ) + has_had_sessions = bool(room_sessions) + + if has_active_sessions: + logger.debug("Meeting %s still has active sessions", meeting.id) + else: if meeting.calendar_event_id: - if current_time > end_date + timedelta(minutes=30): + if has_had_sessions: should_deactivate = True logger.info( - "Meeting %s forced closed 30 min after calendar end", meeting.id - ) - else: - # Unscheduled meetings follow normal closure rules - should_deactivate = True - - # Check Whereby room sessions only if not already deactivating - if not should_deactivate and end_date > current_time: - response = await get_room_sessions(meeting.room_name) - room_sessions = response.get("results", []) - has_active_sessions = room_sessions and any( - rs["endedAt"] is None for rs in room_sessions - ) - - if not has_active_sessions: - # No active sessions - check grace period - if meeting.num_clients == 0: - if meeting.last_participant_left_at: - # Check if grace period has expired - grace_period = timedelta(minutes=meeting.grace_period_minutes) - if ( - current_time - > meeting.last_participant_left_at + grace_period - ): - should_deactivate = True - logger.info("Meeting %s grace period expired", meeting.id) - else: - # First time all participants left, record the time - await meetings_controller.update_meeting( - meeting.id, last_participant_left_at=current_time - ) - logger.info( - "Meeting %s marked empty at %s", meeting.id, current_time - ) - else: - # Has active sessions - clear grace period if set - if meeting.last_participant_left_at: - await meetings_controller.update_meeting( - meeting.id, last_participant_left_at=None + "Calendar meeting %s ended - all participants left", meeting.id ) + elif current_time > end_date: + should_deactivate = True logger.info( - "Meeting %s reactivated - participant rejoined", meeting.id + "Calendar meeting %s deactivated - scheduled time ended with no participants", + meeting.id, ) + else: + logger.debug( + "Calendar meeting %s waiting for participants until %s", + meeting.id, + end_date, + ) + else: + should_deactivate = True + logger.info("On-the-fly meeting %s has no active sessions", meeting.id) if should_deactivate: await meetings_controller.update_meeting(meeting.id, is_active=False) diff --git a/server/tests/test_multiple_active_meetings.py b/server/tests/test_multiple_active_meetings.py index 680790f6..cc59f52d 100644 --- a/server/tests/test_multiple_active_meetings.py +++ b/server/tests/test_multiple_active_meetings.py @@ -117,69 +117,6 @@ async def test_get_active_by_calendar_event(): assert found_meeting.calendar_event_id == event.id -@pytest.mark.asyncio -async def test_grace_period_logic(): - """Test that meetings have a grace period after last participant leaves.""" - # Create a room - room = await rooms_controller.add( - name="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, - ) - - current_time = datetime.now(timezone.utc) - end_time = current_time + timedelta(hours=2) - - # Create meeting - meeting = await meetings_controller.create( - id="meeting-grace", - room_name="test-meeting-grace", - room_url="https://whereby.com/test-grace", - host_room_url="https://whereby.com/test-grace-host", - start_date=current_time, - end_date=end_time, - user_id="test-user", - room=room, - ) - - # Test grace period logic by simulating different states - - # Simulate first time all participants left - await meetings_controller.update_meeting( - meeting.id, num_clients=0, last_participant_left_at=current_time - ) - - # Within grace period (10 min) - should still be active - await meetings_controller.update_meeting( - meeting.id, last_participant_left_at=current_time - timedelta(minutes=10) - ) - - updated_meeting = await meetings_controller.get_by_id(meeting.id) - assert updated_meeting.is_active is True # Still active during grace period - - # Simulate grace period expired (20 min) and deactivate - await meetings_controller.update_meeting( - meeting.id, last_participant_left_at=current_time - timedelta(minutes=20) - ) - - # Manually test the grace period logic that would be in process_meetings - updated_meeting = await meetings_controller.get_by_id(meeting.id) - if updated_meeting.last_participant_left_at: - grace_period = timedelta(minutes=updated_meeting.grace_period_minutes) - if current_time > updated_meeting.last_participant_left_at + grace_period: - await meetings_controller.update_meeting(meeting.id, is_active=False) - - updated_meeting = await meetings_controller.get_by_id(meeting.id) - assert updated_meeting.is_active is False # Now deactivated - - @pytest.mark.asyncio async def test_calendar_meeting_force_close_after_30_min(): """Test that calendar meetings force close 30 minutes after scheduled end.""" @@ -232,52 +169,3 @@ async def test_calendar_meeting_force_close_after_30_min(): updated_meeting = await meetings_controller.get_by_id(meeting.id) assert updated_meeting.is_active is False # Force closed after 30 min - - -@pytest.mark.asyncio -async def test_participant_rejoin_clears_grace_period(): - """Test that participant rejoining clears the grace period.""" - # Create a room - room = await rooms_controller.add( - name="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, - ) - - current_time = datetime.now(timezone.utc) - end_time = current_time + timedelta(hours=2) - - # Create meeting with grace period already set - meeting = await meetings_controller.create( - id="meeting-rejoin", - room_name="test-meeting-rejoin", - room_url="https://whereby.com/test-rejoin", - host_room_url="https://whereby.com/test-rejoin-host", - start_date=current_time, - end_date=end_time, - user_id="test-user", - room=room, - ) - - # Set last_participant_left_at to simulate grace period - await meetings_controller.update_meeting( - meeting.id, - last_participant_left_at=current_time - timedelta(minutes=5), - num_clients=0, - ) - - # Simulate participant rejoining - clear grace period - await meetings_controller.update_meeting( - meeting.id, last_participant_left_at=None, num_clients=1 - ) - - updated_meeting = await meetings_controller.get_by_id(meeting.id) - assert updated_meeting.last_participant_left_at is None # Grace period cleared - assert updated_meeting.is_active is True # Still active diff --git a/www/app/reflector-api.d.ts b/www/app/reflector-api.d.ts index 0b404692..e898a75e 100644 --- a/www/app/reflector-api.d.ts +++ b/www/app/reflector-api.d.ts @@ -1072,13 +1072,6 @@ export interface components { calendar_metadata?: { [key: string]: unknown; } | null; - /** Last Participant Left At */ - last_participant_left_at?: string | null; - /** - * Grace Period Minutes - * @default 15 - */ - grace_period_minutes: number; }; /** MeetingConsentRequest */ MeetingConsentRequest: {