diff --git a/server/migrations/versions/5d6b9df9b045_make_room_platform_non_nullable_with_.py b/server/migrations/versions/5d6b9df9b045_make_room_platform_non_nullable_with_.py new file mode 100644 index 00000000..3b1e4e88 --- /dev/null +++ b/server/migrations/versions/5d6b9df9b045_make_room_platform_non_nullable_with_.py @@ -0,0 +1,30 @@ +"""Make room platform non-nullable with dynamic default + +Revision ID: 5d6b9df9b045 +Revises: 2b92a1b03caa +Create Date: 2025-11-21 13:22:25.756584 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "5d6b9df9b045" +down_revision: Union[str, None] = "2b92a1b03caa" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.execute("UPDATE room SET platform = 'whereby' WHERE platform IS NULL") + + with op.batch_alter_table("room", schema=None) as batch_op: + batch_op.alter_column("platform", existing_type=sa.String(), nullable=False) + + +def downgrade() -> None: + with op.batch_alter_table("room", schema=None) as batch_op: + batch_op.alter_column("platform", existing_type=sa.String(), nullable=True) diff --git a/server/reflector/db/meetings.py b/server/reflector/db/meetings.py index 9c290fa5..8a80e756 100644 --- a/server/reflector/db/meetings.py +++ b/server/reflector/db/meetings.py @@ -10,7 +10,6 @@ from reflector.db.rooms import Room from reflector.schemas.platform import WHEREBY_PLATFORM, Platform from reflector.utils import generate_uuid4 from reflector.utils.string import assert_equal -from reflector.video_platforms.factory import get_platform meetings = sa.Table( "meeting", @@ -140,7 +139,7 @@ class MeetingController: recording_trigger=room.recording_trigger, calendar_event_id=calendar_event_id, calendar_metadata=calendar_metadata, - platform=get_platform(room.platform), + platform=room.platform, ) query = meetings.insert().values(**meeting.model_dump()) await get_database().execute(query) diff --git a/server/reflector/db/rooms.py b/server/reflector/db/rooms.py index 1081ac38..fc6194e3 100644 --- a/server/reflector/db/rooms.py +++ b/server/reflector/db/rooms.py @@ -10,6 +10,7 @@ from sqlalchemy.sql import false, or_ from reflector.db import get_database, metadata from reflector.schemas.platform import Platform +from reflector.settings import settings from reflector.utils import generate_uuid4 rooms = sqlalchemy.Table( @@ -54,8 +55,7 @@ rooms = sqlalchemy.Table( sqlalchemy.Column( "platform", sqlalchemy.String, - nullable=True, - server_default=None, + nullable=False, ), sqlalchemy.Index("idx_room_is_shared", "is_shared"), sqlalchemy.Index("idx_room_ics_enabled", "ics_enabled"), @@ -84,7 +84,7 @@ class Room(BaseModel): ics_enabled: bool = False ics_last_sync: datetime | None = None ics_last_etag: str | None = None - platform: Platform | None = None + platform: Platform = Field(default_factory=lambda: settings.DEFAULT_VIDEO_PLATFORM) class RoomController: @@ -138,7 +138,7 @@ class RoomController: ics_url: str | None = None, ics_fetch_interval: int = 300, ics_enabled: bool = False, - platform: Platform | None = None, + platform: Platform = settings.DEFAULT_VIDEO_PLATFORM, ): """ Add a new room @@ -146,24 +146,26 @@ class RoomController: if webhook_url and not webhook_secret: webhook_secret = secrets.token_urlsafe(32) - room = Room( - name=name, - user_id=user_id, - zulip_auto_post=zulip_auto_post, - zulip_stream=zulip_stream, - zulip_topic=zulip_topic, - is_locked=is_locked, - room_mode=room_mode, - recording_type=recording_type, - recording_trigger=recording_trigger, - is_shared=is_shared, - webhook_url=webhook_url, - webhook_secret=webhook_secret, - ics_url=ics_url, - ics_fetch_interval=ics_fetch_interval, - ics_enabled=ics_enabled, - platform=platform, - ) + room_data = { + "name": name, + "user_id": user_id, + "zulip_auto_post": zulip_auto_post, + "zulip_stream": zulip_stream, + "zulip_topic": zulip_topic, + "is_locked": is_locked, + "room_mode": room_mode, + "recording_type": recording_type, + "recording_trigger": recording_trigger, + "is_shared": is_shared, + "webhook_url": webhook_url, + "webhook_secret": webhook_secret, + "ics_url": ics_url, + "ics_fetch_interval": ics_fetch_interval, + "ics_enabled": ics_enabled, + "platform": platform, + } + + room = Room(**room_data) query = rooms.insert().values(**room.model_dump()) try: await get_database().execute(query) diff --git a/server/reflector/video_platforms/factory.py b/server/reflector/video_platforms/factory.py index 172d45e7..7c60c379 100644 --- a/server/reflector/video_platforms/factory.py +++ b/server/reflector/video_platforms/factory.py @@ -1,5 +1,3 @@ -from typing import Optional - from reflector.settings import settings from reflector.storage import get_dailyco_storage, get_whereby_storage @@ -53,10 +51,3 @@ def get_platform_config(platform: Platform) -> VideoPlatformConfig: def create_platform_client(platform: Platform) -> VideoPlatformClient: config = get_platform_config(platform) return get_platform_client(platform, config) - - -def get_platform(room_platform: Optional[Platform] = None) -> Platform: - if room_platform: - return room_platform - - return settings.DEFAULT_VIDEO_PLATFORM diff --git a/server/reflector/views/rooms.py b/server/reflector/views/rooms.py index e786b0d9..baafaffe 100644 --- a/server/reflector/views/rooms.py +++ b/server/reflector/views/rooms.py @@ -19,10 +19,7 @@ from reflector.schemas.platform import Platform from reflector.services.ics_sync import ics_sync_service from reflector.settings import settings from reflector.utils.url import add_query_param -from reflector.video_platforms.factory import ( - create_platform_client, - get_platform, -) +from reflector.video_platforms.factory import create_platform_client from reflector.worker.webhook import test_webhook logger = logging.getLogger(__name__) @@ -190,9 +187,6 @@ async def rooms_list( ), ) - for room in paginated.items: - room.platform = get_platform(room.platform) - return paginated @@ -207,7 +201,6 @@ async def rooms_get( raise HTTPException(status_code=404, detail="Room not found") if not room.is_shared and (user_id is None or room.user_id != user_id): raise HTTPException(status_code=403, detail="Room access denied") - room.platform = get_platform(room.platform) return room @@ -229,8 +222,6 @@ async def rooms_get_by_name( room_dict["webhook_url"] = None room_dict["webhook_secret"] = None - room_dict["platform"] = get_platform(room.platform) - return RoomDetails(**room_dict) @@ -275,7 +266,6 @@ async def rooms_update( raise HTTPException(status_code=403, detail="Not authorized") values = info.dict(exclude_unset=True) await rooms_controller.update(room, values) - room.platform = get_platform(room.platform) return room @@ -323,7 +313,7 @@ async def rooms_create_meeting( if meeting is None: end_date = current_time + timedelta(hours=8) - platform = get_platform(room.platform) + platform = room.platform client = create_platform_client(platform) meeting_data = await client.create_meeting( @@ -513,9 +503,8 @@ async def rooms_list_active_meetings( room=room, current_time=current_time ) - effective_platform = get_platform(room.platform) for meeting in meetings: - meeting.platform = effective_platform + meeting.platform = room.platform if user_id != room.user_id: for meeting in meetings: diff --git a/server/reflector/worker/ics_sync.py b/server/reflector/worker/ics_sync.py index 6881dfa2..6e126309 100644 --- a/server/reflector/worker/ics_sync.py +++ b/server/reflector/worker/ics_sync.py @@ -10,7 +10,7 @@ from reflector.db.meetings import meetings_controller from reflector.db.rooms import Room, rooms_controller from reflector.redis_cache import RedisAsyncLock from reflector.services.ics_sync import SyncStatus, ics_sync_service -from reflector.video_platforms.factory import create_platform_client, get_platform +from reflector.video_platforms.factory import create_platform_client logger = structlog.wrap_logger(get_task_logger(__name__)) @@ -104,7 +104,7 @@ 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(get_platform(room.platform)) + client = create_platform_client(room.platform) meeting_data = await client.create_meeting( room.name, diff --git a/server/tests/test_video_platforms_factory.py b/server/tests/test_video_platforms_factory.py deleted file mode 100644 index 6c8c02c5..00000000 --- a/server/tests/test_video_platforms_factory.py +++ /dev/null @@ -1,58 +0,0 @@ -"""Tests for video_platforms.factory module.""" - -from unittest.mock import patch - -from reflector.video_platforms.factory import get_platform - - -class TestGetPlatformF: - """Test suite for get_platform function.""" - - @patch("reflector.video_platforms.factory.settings") - def test_with_room_platform(self, mock_settings): - """When room_platform provided, should return room_platform.""" - mock_settings.DEFAULT_VIDEO_PLATFORM = "whereby" - - # Should return the room's platform when provided - assert get_platform(room_platform="daily") == "daily" - assert get_platform(room_platform="whereby") == "whereby" - - @patch("reflector.video_platforms.factory.settings") - def test_without_room_platform_uses_default(self, mock_settings): - """When no room_platform, should return DEFAULT_VIDEO_PLATFORM.""" - mock_settings.DEFAULT_VIDEO_PLATFORM = "whereby" - - # Should return default when room_platform is None - assert get_platform(room_platform=None) == "whereby" - - @patch("reflector.video_platforms.factory.settings") - def test_with_daily_default(self, mock_settings): - """When DEFAULT_VIDEO_PLATFORM is 'daily', should return 'daily' when no room_platform.""" - mock_settings.DEFAULT_VIDEO_PLATFORM = "daily" - - # Should return default 'daily' when room_platform is None - assert get_platform(room_platform=None) == "daily" - - @patch("reflector.video_platforms.factory.settings") - def test_no_room_id_provided(self, mock_settings): - """Should work correctly even when room_id is not provided.""" - mock_settings.DEFAULT_VIDEO_PLATFORM = "whereby" - - # Should use room_platform when provided - assert get_platform(room_platform="daily") == "daily" - - # Should use default when room_platform not provided - assert get_platform(room_platform=None) == "whereby" - - @patch("reflector.video_platforms.factory.settings") - def test_room_platform_always_takes_precedence(self, mock_settings): - """room_platform should always be used when provided.""" - mock_settings.DEFAULT_VIDEO_PLATFORM = "whereby" - - # room_platform should take precedence over default - assert get_platform(room_platform="daily") == "daily" - assert get_platform(room_platform="whereby") == "whereby" - - # Different default shouldn't matter when room_platform provided - mock_settings.DEFAULT_VIDEO_PLATFORM = "daily" - assert get_platform(room_platform="whereby") == "whereby"